* [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups
@ 2016-06-10 16:05 Kevin Wolf
2016-06-10 16:05 ` [Qemu-devel] [PATCH 1/6] block: Introduce bdrv_preadv() Kevin Wolf
` (8 more replies)
0 siblings, 9 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-06-10 16:05 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, qemu-devel, famz, stefanha
This series contains a few cleanups with respect to the vmstate I/O functions.
Apart from making the interface more consistent (writes were already vectored,
but not reads), this makes use of the new byte-based .bdrv_co_preadv/pwritev
callbacks in qcow2 to get rid of a few hacks, including bs->zero_beyond_eof.
Kevin Wolf (6):
block: Introduce bdrv_preadv()
block: Make .bdrv_load_vmstate() vectored
block: Allow .bdrv_load/save_vmstate() to return 0/-errno
block: Make bdrv_load/save_vmstate coroutine_fns
qcow2: Let vmstate call qcow2_co_preadv/pwrite directly
block: Remove bs->zero_beyond_eof
block.c | 2 -
block/io.c | 173 +++++++++++++++++++++++++++++++---------------
block/qcow2.c | 28 ++------
block/sheepdog.c | 13 +++-
include/block/block.h | 2 +
include/block/block_int.h | 13 ++--
6 files changed, 143 insertions(+), 88 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 1/6] block: Introduce bdrv_preadv()
2016-06-10 16:05 [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Kevin Wolf
@ 2016-06-10 16:05 ` Kevin Wolf
2016-06-10 20:50 ` Eric Blake
2016-06-10 16:05 ` [Qemu-devel] [PATCH 2/6] block: Make .bdrv_load_vmstate() vectored Kevin Wolf
` (7 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Kevin Wolf @ 2016-06-10 16:05 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, qemu-devel, famz, stefanha
We already have a byte-based bdrv_pwritev(), but the read counterpart
was still missing. This commit adds it.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 20 +++++++++++++-------
include/block/block.h | 1 +
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/block/io.c b/block/io.c
index b3d6228..11510cf 100644
--- a/block/io.c
+++ b/block/io.c
@@ -700,6 +700,18 @@ int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags)
}
}
+int bdrv_preadv(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov)
+{
+ int ret;
+
+ ret = bdrv_prwv_co(bs, offset, qiov, false, 0);
+ if (ret < 0) {
+ return ret;
+ }
+
+ return qiov->size;
+}
+
int bdrv_pread(BlockDriverState *bs, int64_t offset, void *buf, int bytes)
{
QEMUIOVector qiov;
@@ -707,19 +719,13 @@ int bdrv_pread(BlockDriverState *bs, int64_t offset, void *buf, int bytes)
.iov_base = (void *)buf,
.iov_len = bytes,
};
- int ret;
if (bytes < 0) {
return -EINVAL;
}
qemu_iovec_init_external(&qiov, &iov, 1);
- ret = bdrv_prwv_co(bs, offset, &qiov, false, 0);
- if (ret < 0) {
- return ret;
- }
-
- return bytes;
+ return bdrv_preadv(bs, offset, &qiov);
}
int bdrv_pwritev(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov)
diff --git a/include/block/block.h b/include/block/block.h
index fb0078f..aca7f23 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -232,6 +232,7 @@ int bdrv_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags);
int bdrv_pread(BlockDriverState *bs, int64_t offset,
void *buf, int count);
+int bdrv_preadv(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov);
int bdrv_pwrite(BlockDriverState *bs, int64_t offset,
const void *buf, int count);
int bdrv_pwritev(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 2/6] block: Make .bdrv_load_vmstate() vectored
2016-06-10 16:05 [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Kevin Wolf
2016-06-10 16:05 ` [Qemu-devel] [PATCH 1/6] block: Introduce bdrv_preadv() Kevin Wolf
@ 2016-06-10 16:05 ` Kevin Wolf
2016-06-10 21:25 ` Eric Blake
2016-06-10 16:05 ` [Qemu-devel] [PATCH 3/6] block: Allow .bdrv_load/save_vmstate() to return 0/-errno Kevin Wolf
` (6 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Kevin Wolf @ 2016-06-10 16:05 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, qemu-devel, famz, stefanha
This brings it in line with .bdrv_save_vmstate().
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 26 +++++++++++++++++++++-----
block/qcow2.c | 6 +++---
block/sheepdog.c | 13 ++++++++++---
include/block/block.h | 1 +
include/block/block_int.h | 4 ++--
5 files changed, 37 insertions(+), 13 deletions(-)
diff --git a/block/io.c b/block/io.c
index 11510cf..602c7d3 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1862,13 +1862,29 @@ int bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
int64_t pos, int size)
{
+ QEMUIOVector qiov;
+ struct iovec iov = {
+ .iov_base = buf,
+ .iov_len = size,
+ };
+ int ret;
+
+ qemu_iovec_init_external(&qiov, &iov, 1);
+ return bdrv_readv_vmstate(bs, &qiov, pos);
+}
+
+int bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
+{
BlockDriver *drv = bs->drv;
- if (!drv)
+
+ if (!drv) {
return -ENOMEDIUM;
- if (drv->bdrv_load_vmstate)
- return drv->bdrv_load_vmstate(bs, buf, pos, size);
- if (bs->file)
- return bdrv_load_vmstate(bs->file->bs, buf, pos, size);
+ } else if (drv->bdrv_load_vmstate) {
+ return drv->bdrv_load_vmstate(bs, qiov, pos);
+ } else if (bs->file) {
+ return bdrv_readv_vmstate(bs->file->bs, qiov, pos);
+ }
+
return -ENOTSUP;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index cb55e2d..72ae2bf 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2920,8 +2920,8 @@ static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
return ret;
}
-static int qcow2_load_vmstate(BlockDriverState *bs, uint8_t *buf,
- int64_t pos, int size)
+static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
+ int64_t pos)
{
BDRVQcow2State *s = bs->opaque;
bool zero_beyond_eof = bs->zero_beyond_eof;
@@ -2929,7 +2929,7 @@ static int qcow2_load_vmstate(BlockDriverState *bs, uint8_t *buf,
BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_LOAD);
bs->zero_beyond_eof = false;
- ret = bdrv_pread(bs, qcow2_vm_state_offset(s) + pos, buf, size);
+ ret = bdrv_preadv(bs, qcow2_vm_state_offset(s) + pos, qiov);
bs->zero_beyond_eof = zero_beyond_eof;
return ret;
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 23fbace..ef5d044 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2784,12 +2784,19 @@ static int sd_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
return ret;
}
-static int sd_load_vmstate(BlockDriverState *bs, uint8_t *data,
- int64_t pos, int size)
+static int sd_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
+ int64_t pos)
{
BDRVSheepdogState *s = bs->opaque;
+ void *buf;
+ int ret;
- return do_load_save_vmstate(s, data, pos, size, 1);
+ buf = qemu_blockalign(bs, qiov->size);
+ ret = do_load_save_vmstate(s, buf, pos, qiov->size, 1);
+ qemu_iovec_from_buf(qiov, 0, buf, qiov->size);
+ qemu_vfree(buf);
+
+ return ret;
}
diff --git a/include/block/block.h b/include/block/block.h
index aca7f23..a158575 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -428,6 +428,7 @@ void path_combine(char *dest, int dest_size,
const char *base_path,
const char *filename);
+int bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos);
int bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos);
int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
int64_t pos, int size);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 8a4963c..f9a32cc 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -226,8 +226,8 @@ struct BlockDriver {
int (*bdrv_save_vmstate)(BlockDriverState *bs, QEMUIOVector *qiov,
int64_t pos);
- int (*bdrv_load_vmstate)(BlockDriverState *bs, uint8_t *buf,
- int64_t pos, int size);
+ int (*bdrv_load_vmstate)(BlockDriverState *bs, QEMUIOVector *qiov,
+ int64_t pos);
int (*bdrv_change_backing_file)(BlockDriverState *bs,
const char *backing_file, const char *backing_fmt);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 3/6] block: Allow .bdrv_load/save_vmstate() to return 0/-errno
2016-06-10 16:05 [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Kevin Wolf
2016-06-10 16:05 ` [Qemu-devel] [PATCH 1/6] block: Introduce bdrv_preadv() Kevin Wolf
2016-06-10 16:05 ` [Qemu-devel] [PATCH 2/6] block: Make .bdrv_load_vmstate() vectored Kevin Wolf
@ 2016-06-10 16:05 ` Kevin Wolf
2016-06-10 21:29 ` Eric Blake
2016-06-10 16:05 ` [Qemu-devel] [PATCH 4/6] block: Make bdrv_load/save_vmstate coroutine_fns Kevin Wolf
` (5 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Kevin Wolf @ 2016-06-10 16:05 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, qemu-devel, famz, stefanha
The return value of .bdrv_load/save_vmstate() can be any non-negative
number in case of success now. It used to be bytes/-errno.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/block/io.c b/block/io.c
index 602c7d3..bca244c 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1839,9 +1839,16 @@ int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
.iov_base = (void *) buf,
.iov_len = size,
};
+ int ret;
qemu_iovec_init_external(&qiov, &iov, 1);
- return bdrv_writev_vmstate(bs, &qiov, pos);
+
+ ret = bdrv_writev_vmstate(bs, &qiov, pos);
+ if (ret < 0) {
+ return ret;
+ }
+
+ return size;
}
int bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
@@ -1870,7 +1877,12 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
int ret;
qemu_iovec_init_external(&qiov, &iov, 1);
- return bdrv_readv_vmstate(bs, &qiov, pos);
+ ret = bdrv_readv_vmstate(bs, &qiov, pos);
+ if (ret < 0) {
+ return ret;
+ }
+
+ return size;
}
int bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 4/6] block: Make bdrv_load/save_vmstate coroutine_fns
2016-06-10 16:05 [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Kevin Wolf
` (2 preceding siblings ...)
2016-06-10 16:05 ` [Qemu-devel] [PATCH 3/6] block: Allow .bdrv_load/save_vmstate() to return 0/-errno Kevin Wolf
@ 2016-06-10 16:05 ` Kevin Wolf
2016-06-10 22:33 ` Eric Blake
2016-06-16 8:49 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2016-06-10 16:05 ` [Qemu-devel] [PATCH 5/6] qcow2: Let vmstate call qcow2_co_preadv/pwrite directly Kevin Wolf
` (4 subsequent siblings)
8 siblings, 2 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-06-10 16:05 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, qemu-devel, famz, stefanha
This allows drivers to share code between normal I/O and vmstate
accesses.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 80 ++++++++++++++++++++++++++++++++++-------------
include/block/block_int.h | 10 +++---
2 files changed, 64 insertions(+), 26 deletions(-)
diff --git a/block/io.c b/block/io.c
index bca244c..adf2726 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1831,6 +1831,62 @@ int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
return drv->bdrv_write_compressed(bs, sector_num, buf, nb_sectors);
}
+typedef struct BdrvVmstateCo {
+ BlockDriverState *bs;
+ QEMUIOVector *qiov;
+ int64_t pos;
+ bool is_read;
+ int ret;
+} BdrvVmstateCo;
+
+static int coroutine_fn
+bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
+ bool is_read)
+{
+ BlockDriver *drv = bs->drv;
+
+ if (!drv) {
+ return -ENOMEDIUM;
+ } else if (drv->bdrv_load_vmstate) {
+ return is_read ? drv->bdrv_load_vmstate(bs, qiov, pos)
+ : drv->bdrv_save_vmstate(bs, qiov, pos);
+ } else if (bs->file) {
+ return bdrv_co_rw_vmstate(bs->file->bs, qiov, pos, is_read);
+ }
+
+ return -ENOTSUP;
+}
+
+static void bdrv_co_rw_vmstate_entry(void *opaque)
+{
+ BdrvVmstateCo *co = opaque;
+ co->ret = bdrv_co_rw_vmstate(co->bs, co->qiov, co->pos, co->is_read);
+}
+
+static inline int
+bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
+ bool is_read)
+{
+ if (qemu_in_coroutine()) {
+ return bdrv_co_rw_vmstate(bs, qiov, pos, is_read);
+ } else {
+ BdrvVmstateCo data = {
+ .bs = bs,
+ .qiov = qiov,
+ .pos = pos,
+ .is_read = is_read,
+ .ret = -EINPROGRESS,
+ };
+ Coroutine *co = qemu_coroutine_create(bdrv_co_rw_vmstate_entry);
+
+ qemu_coroutine_enter(co, &data);
+ while (data.ret == -EINPROGRESS) {
+ aio_poll(bdrv_get_aio_context(bs), true);
+ }
+ return data.ret;
+ }
+}
+
int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
int64_t pos, int size)
{
@@ -1853,17 +1909,7 @@ int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
int bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
{
- BlockDriver *drv = bs->drv;
-
- if (!drv) {
- return -ENOMEDIUM;
- } else if (drv->bdrv_save_vmstate) {
- return drv->bdrv_save_vmstate(bs, qiov, pos);
- } else if (bs->file) {
- return bdrv_writev_vmstate(bs->file->bs, qiov, pos);
- }
-
- return -ENOTSUP;
+ return bdrv_rw_vmstate(bs, qiov, pos, false);
}
int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
@@ -1887,17 +1933,7 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
int bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
{
- BlockDriver *drv = bs->drv;
-
- if (!drv) {
- return -ENOMEDIUM;
- } else if (drv->bdrv_load_vmstate) {
- return drv->bdrv_load_vmstate(bs, qiov, pos);
- } else if (bs->file) {
- return bdrv_readv_vmstate(bs->file->bs, qiov, pos);
- }
-
- return -ENOTSUP;
+ return bdrv_rw_vmstate(bs, qiov, pos, true);
}
/**************************************************************/
diff --git a/include/block/block_int.h b/include/block/block_int.h
index f9a32cc..1fe0811 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -224,10 +224,12 @@ struct BlockDriver {
int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi);
ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs);
- int (*bdrv_save_vmstate)(BlockDriverState *bs, QEMUIOVector *qiov,
- int64_t pos);
- int (*bdrv_load_vmstate)(BlockDriverState *bs, QEMUIOVector *qiov,
- int64_t pos);
+ int coroutine_fn (*bdrv_save_vmstate)(BlockDriverState *bs,
+ QEMUIOVector *qiov,
+ int64_t pos);
+ int coroutine_fn (*bdrv_load_vmstate)(BlockDriverState *bs,
+ QEMUIOVector *qiov,
+ int64_t pos);
int (*bdrv_change_backing_file)(BlockDriverState *bs,
const char *backing_file, const char *backing_fmt);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 5/6] qcow2: Let vmstate call qcow2_co_preadv/pwrite directly
2016-06-10 16:05 [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Kevin Wolf
` (3 preceding siblings ...)
2016-06-10 16:05 ` [Qemu-devel] [PATCH 4/6] block: Make bdrv_load/save_vmstate coroutine_fns Kevin Wolf
@ 2016-06-10 16:05 ` Kevin Wolf
2016-06-10 22:39 ` Eric Blake
2016-06-10 16:05 ` [Qemu-devel] [PATCH 6/6] block: Remove bs->zero_beyond_eof Kevin Wolf
` (3 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Kevin Wolf @ 2016-06-10 16:05 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, qemu-devel, famz, stefanha
We don't really want to go through the block layer in order to read from
or write to the vmstate in a qcow2 image. Doing so required a few ugly
hacks like saving and restoring the old image size (because writing to
vmstate offsets would increase the image size) or disabling the "reads
after EOF = zeroes" logic. When calling the right functions directly,
these hacks aren't necessary any more.
Note that .bdrv_vmstate_load/save() return 0 instead of the number of
bytes in case of success now.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2.c | 24 ++++--------------------
1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 72ae2bf..c40baca 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2903,36 +2903,20 @@ static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
int64_t pos)
{
BDRVQcow2State *s = bs->opaque;
- int64_t total_sectors = bs->total_sectors;
- bool zero_beyond_eof = bs->zero_beyond_eof;
- int ret;
BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_SAVE);
- bs->zero_beyond_eof = false;
- ret = bdrv_pwritev(bs, qcow2_vm_state_offset(s) + pos, qiov);
- bs->zero_beyond_eof = zero_beyond_eof;
-
- /* bdrv_co_do_writev will have increased the total_sectors value to include
- * the VM state - the VM state is however not an actual part of the block
- * device, therefore, we need to restore the old value. */
- bs->total_sectors = total_sectors;
-
- return ret;
+ return bs->drv->bdrv_co_pwritev(bs, qcow2_vm_state_offset(s) + pos,
+ qiov->size, qiov, 0);
}
static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
int64_t pos)
{
BDRVQcow2State *s = bs->opaque;
- bool zero_beyond_eof = bs->zero_beyond_eof;
- int ret;
BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_LOAD);
- bs->zero_beyond_eof = false;
- ret = bdrv_preadv(bs, qcow2_vm_state_offset(s) + pos, qiov);
- bs->zero_beyond_eof = zero_beyond_eof;
-
- return ret;
+ return bs->drv->bdrv_co_preadv(bs, qcow2_vm_state_offset(s) + pos,
+ qiov->size, qiov, 0);
}
/*
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 6/6] block: Remove bs->zero_beyond_eof
2016-06-10 16:05 [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Kevin Wolf
` (4 preceding siblings ...)
2016-06-10 16:05 ` [Qemu-devel] [PATCH 5/6] qcow2: Let vmstate call qcow2_co_preadv/pwrite directly Kevin Wolf
@ 2016-06-10 16:05 ` Kevin Wolf
2016-06-10 22:41 ` Eric Blake
2016-06-12 2:59 ` [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Fam Zheng
` (2 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Kevin Wolf @ 2016-06-10 16:05 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, eblake, mreitz, qemu-devel, famz, stefanha
It is always true for open images now.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 2 --
block/io.c | 51 +++++++++++++++++++++--------------------------
include/block/block_int.h | 3 ---
3 files changed, 23 insertions(+), 33 deletions(-)
diff --git a/block.c b/block.c
index 3d850a2..b350794 100644
--- a/block.c
+++ b/block.c
@@ -938,7 +938,6 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
}
bs->request_alignment = drv->bdrv_co_preadv ? 1 : 512;
- bs->zero_beyond_eof = true;
bs->read_only = !(bs->open_flags & BDRV_O_RDWR);
if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
@@ -2192,7 +2191,6 @@ static void bdrv_close(BlockDriverState *bs)
bs->encrypted = 0;
bs->valid_key = 0;
bs->sg = 0;
- bs->zero_beyond_eof = false;
QDECREF(bs->options);
QDECREF(bs->explicit_options);
bs->options = NULL;
diff --git a/block/io.c b/block/io.c
index adf2726..d504443 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1000,40 +1000,35 @@ static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs,
}
/* Forward the request to the BlockDriver */
- if (!bs->zero_beyond_eof) {
- ret = bdrv_driver_preadv(bs, offset, bytes, qiov, 0);
- } else {
- /* Read zeros after EOF */
- int64_t total_bytes, max_bytes;
+ int64_t total_bytes, max_bytes;
- total_bytes = bdrv_getlength(bs);
- if (total_bytes < 0) {
- ret = total_bytes;
- goto out;
- }
+ total_bytes = bdrv_getlength(bs);
+ if (total_bytes < 0) {
+ ret = total_bytes;
+ goto out;
+ }
- max_bytes = ROUND_UP(MAX(0, total_bytes - offset), align);
- if (bytes < max_bytes) {
- ret = bdrv_driver_preadv(bs, offset, bytes, qiov, 0);
- } else if (max_bytes > 0) {
- QEMUIOVector local_qiov;
+ max_bytes = ROUND_UP(MAX(0, total_bytes - offset), align);
+ if (bytes < max_bytes) {
+ ret = bdrv_driver_preadv(bs, offset, bytes, qiov, 0);
+ } else if (max_bytes > 0) {
+ QEMUIOVector local_qiov;
- qemu_iovec_init(&local_qiov, qiov->niov);
- qemu_iovec_concat(&local_qiov, qiov, 0, max_bytes);
+ qemu_iovec_init(&local_qiov, qiov->niov);
+ qemu_iovec_concat(&local_qiov, qiov, 0, max_bytes);
- ret = bdrv_driver_preadv(bs, offset, max_bytes, &local_qiov, 0);
+ ret = bdrv_driver_preadv(bs, offset, max_bytes, &local_qiov, 0);
- qemu_iovec_destroy(&local_qiov);
- } else {
- ret = 0;
- }
+ qemu_iovec_destroy(&local_qiov);
+ } else {
+ ret = 0;
+ }
- /* Reading beyond end of file is supposed to produce zeroes */
- if (ret == 0 && total_bytes < offset + bytes) {
- uint64_t zero_offset = MAX(0, total_bytes - offset);
- uint64_t zero_bytes = offset + bytes - zero_offset;
- qemu_iovec_memset(qiov, zero_offset, 0, zero_bytes);
- }
+ /* Reading beyond end of file is supposed to produce zeroes */
+ if (ret == 0 && total_bytes < offset + bytes) {
+ uint64_t zero_offset = MAX(0, total_bytes - offset);
+ uint64_t zero_bytes = offset + bytes - zero_offset;
+ qemu_iovec_memset(qiov, zero_offset, 0, zero_bytes);
}
out:
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1fe0811..16c43e2 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -451,9 +451,6 @@ struct BlockDriverState {
/* I/O Limits */
BlockLimits bl;
- /* Whether produces zeros when read beyond eof */
- bool zero_beyond_eof;
-
/* Alignment requirement for offset/length of I/O requests */
unsigned int request_alignment;
/* Flags honored during pwrite (so far: BDRV_REQ_FUA) */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] block: Introduce bdrv_preadv()
2016-06-10 16:05 ` [Qemu-devel] [PATCH 1/6] block: Introduce bdrv_preadv() Kevin Wolf
@ 2016-06-10 20:50 ` Eric Blake
0 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-06-10 20:50 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel, famz, stefanha
[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]
On 06/10/2016 10:05 AM, Kevin Wolf wrote:
> We already have a byte-based bdrv_pwritev(), but the read counterpart
> was still missing. This commit adds it.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/io.c | 20 +++++++++++++-------
> include/block/block.h | 1 +
> 2 files changed, 14 insertions(+), 7 deletions(-)
>
Worth adding a flags argument while at it? But bdrv_pwritev() lacks one,
so for symmetry reasons, I'm okay if you don't bother.
> +int bdrv_preadv(BlockDriverState *bs, int64_t offset, QEMUIOVector *qiov)
> +{
> + int ret;
> +
> + ret = bdrv_prwv_co(bs, offset, qiov, false, 0);
> + if (ret < 0) {
> + return ret;
> + }
> +
> + return qiov->size;
This implies we never have a short read, it's an all-or-none error or
success. Matches what we've done elsewhere, so I guess it's right.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/6] block: Make .bdrv_load_vmstate() vectored
2016-06-10 16:05 ` [Qemu-devel] [PATCH 2/6] block: Make .bdrv_load_vmstate() vectored Kevin Wolf
@ 2016-06-10 21:25 ` Eric Blake
0 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-06-10 21:25 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel, famz, stefanha
[-- Attachment #1: Type: text/plain, Size: 1950 bytes --]
On 06/10/2016 10:05 AM, Kevin Wolf wrote:
> This brings it in line with .bdrv_save_vmstate().
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/io.c | 26 +++++++++++++++++++++-----
> block/qcow2.c | 6 +++---
> block/sheepdog.c | 13 ++++++++++---
> include/block/block.h | 1 +
> include/block/block_int.h | 4 ++--
> 5 files changed, 37 insertions(+), 13 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index 11510cf..602c7d3 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1862,13 +1862,29 @@ int bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
> int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
> int64_t pos, int size)
> {
> + QEMUIOVector qiov;
> + struct iovec iov = {
> + .iov_base = buf,
> + .iov_len = size,
> + };
> + int ret;
Dead variable.
> +
> + qemu_iovec_init_external(&qiov, &iov, 1);
> + return bdrv_readv_vmstate(bs, &qiov, pos);
> +}
> +
> +int bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
> +{
> BlockDriver *drv = bs->drv;
> - if (!drv)
> +
> + if (!drv) {
> return -ENOMEDIUM;
> - if (drv->bdrv_load_vmstate)
> - return drv->bdrv_load_vmstate(bs, buf, pos, size);
> - if (bs->file)
> - return bdrv_load_vmstate(bs->file->bs, buf, pos, size);
> + } else if (drv->bdrv_load_vmstate) {
> + return drv->bdrv_load_vmstate(bs, qiov, pos);
> + } else if (bs->file) {
> + return bdrv_readv_vmstate(bs->file->bs, qiov, pos);
> + }
Don't know that I would have used 'else if' after a return, but it's not
wrong, so no need to change.
With the dead 'ret' gone,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 3/6] block: Allow .bdrv_load/save_vmstate() to return 0/-errno
2016-06-10 16:05 ` [Qemu-devel] [PATCH 3/6] block: Allow .bdrv_load/save_vmstate() to return 0/-errno Kevin Wolf
@ 2016-06-10 21:29 ` Eric Blake
0 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-06-10 21:29 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel, famz, stefanha
[-- Attachment #1: Type: text/plain, Size: 1816 bytes --]
On 06/10/2016 10:05 AM, Kevin Wolf wrote:
> The return value of .bdrv_load/save_vmstate() can be any non-negative
> number in case of success now. It used to be bytes/-errno.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/io.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index 602c7d3..bca244c 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1839,9 +1839,16 @@ int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
> .iov_base = (void *) buf,
> .iov_len = size,
> };
> + int ret;
>
> qemu_iovec_init_external(&qiov, &iov, 1);
> - return bdrv_writev_vmstate(bs, &qiov, pos);
> +
> + ret = bdrv_writev_vmstate(bs, &qiov, pos);
> + if (ret < 0) {
> + return ret;
> + }
> +
> + return size;
> }
>
> int bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
> @@ -1870,7 +1877,12 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
> int ret;
Aha, my complaint in v2 about it being dead means you need to reinstate
it here.
>
> qemu_iovec_init_external(&qiov, &iov, 1);
> - return bdrv_readv_vmstate(bs, &qiov, pos);
> + ret = bdrv_readv_vmstate(bs, &qiov, pos);
> + if (ret < 0) {
> + return ret;
> + }
> +
> + return size;
> }
>
> int bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
>
Matches the semantics we have elsewhere (I'm not sure if 'size' is the
best choice if we ever need to support short read/write, but doesn't
seem to hurt).
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 4/6] block: Make bdrv_load/save_vmstate coroutine_fns
2016-06-10 16:05 ` [Qemu-devel] [PATCH 4/6] block: Make bdrv_load/save_vmstate coroutine_fns Kevin Wolf
@ 2016-06-10 22:33 ` Eric Blake
2016-06-16 8:49 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
1 sibling, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-06-10 22:33 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel, famz, stefanha
[-- Attachment #1: Type: text/plain, Size: 525 bytes --]
On 06/10/2016 10:05 AM, Kevin Wolf wrote:
> This allows drivers to share code between normal I/O and vmstate
> accesses.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/io.c | 80 ++++++++++++++++++++++++++++++++++-------------
> include/block/block_int.h | 10 +++---
> 2 files changed, 64 insertions(+), 26 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] qcow2: Let vmstate call qcow2_co_preadv/pwrite directly
2016-06-10 16:05 ` [Qemu-devel] [PATCH 5/6] qcow2: Let vmstate call qcow2_co_preadv/pwrite directly Kevin Wolf
@ 2016-06-10 22:39 ` Eric Blake
2016-06-12 2:58 ` Fam Zheng
0 siblings, 1 reply; 19+ messages in thread
From: Eric Blake @ 2016-06-10 22:39 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel, famz, stefanha
[-- Attachment #1: Type: text/plain, Size: 2641 bytes --]
On 06/10/2016 10:05 AM, Kevin Wolf wrote:
> We don't really want to go through the block layer in order to read from
> or write to the vmstate in a qcow2 image. Doing so required a few ugly
> hacks like saving and restoring the old image size (because writing to
> vmstate offsets would increase the image size) or disabling the "reads
> after EOF = zeroes" logic. When calling the right functions directly,
> these hacks aren't necessary any more.
>
> Note that .bdrv_vmstate_load/save() return 0 instead of the number of
> bytes in case of success now.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/qcow2.c | 24 ++++--------------------
> 1 file changed, 4 insertions(+), 20 deletions(-)
>
> +++ b/block/qcow2.c
> @@ -2903,36 +2903,20 @@ static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
> int64_t pos)
> {
> BDRVQcow2State *s = bs->opaque;
> - int64_t total_sectors = bs->total_sectors;
> - bool zero_beyond_eof = bs->zero_beyond_eof;
> - int ret;
>
> BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_SAVE);
> - bs->zero_beyond_eof = false;
> - ret = bdrv_pwritev(bs, qcow2_vm_state_offset(s) + pos, qiov);
> - bs->zero_beyond_eof = zero_beyond_eof;
> -
> - /* bdrv_co_do_writev will have increased the total_sectors value to include
> - * the VM state - the VM state is however not an actual part of the block
> - * device, therefore, we need to restore the old value. */
> - bs->total_sectors = total_sectors;
> -
> - return ret;
> + return bs->drv->bdrv_co_pwritev(bs, qcow2_vm_state_offset(s) + pos,
> + qiov->size, qiov, 0);
> }
bs->drv->bdrv_co_pwritev() is an optional interface; not all the drivers
have it yet. Should you be asserting that it exists, and/or returning
an error if it does not?
>
> static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
> int64_t pos)
> {
> BDRVQcow2State *s = bs->opaque;
> - bool zero_beyond_eof = bs->zero_beyond_eof;
> - int ret;
>
> BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_LOAD);
> - bs->zero_beyond_eof = false;
> - ret = bdrv_preadv(bs, qcow2_vm_state_offset(s) + pos, qiov);
> - bs->zero_beyond_eof = zero_beyond_eof;
> -
> - return ret;
> + return bs->drv->bdrv_co_preadv(bs, qcow2_vm_state_offset(s) + pos,
> + qiov->size, qiov, 0);
Ditto.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] block: Remove bs->zero_beyond_eof
2016-06-10 16:05 ` [Qemu-devel] [PATCH 6/6] block: Remove bs->zero_beyond_eof Kevin Wolf
@ 2016-06-10 22:41 ` Eric Blake
0 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-06-10 22:41 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, qemu-devel, famz, stefanha
[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]
On 06/10/2016 10:05 AM, Kevin Wolf wrote:
> It is always true for open images now.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 2 --
> block/io.c | 51 +++++++++++++++++++++--------------------------
> include/block/block_int.h | 3 ---
> 3 files changed, 23 insertions(+), 33 deletions(-)
>
> +++ b/block/io.c
> @@ -1000,40 +1000,35 @@ static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs,
> }
>
> /* Forward the request to the BlockDriver */
> - if (!bs->zero_beyond_eof) {
> - ret = bdrv_driver_preadv(bs, offset, bytes, qiov, 0);
> - } else {
> - /* Read zeros after EOF */
> - int64_t total_bytes, max_bytes;
> + int64_t total_bytes, max_bytes;
This declaration is now in the middle of the function. Shouldn't you
hoist it to the beginning?
That's minor enough to fix on pull request, so:
Reviewed-by: Eric Blake <eblake@redhat.com>
I'll rebase my pending byte-based BlockLimits series on top of this.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] qcow2: Let vmstate call qcow2_co_preadv/pwrite directly
2016-06-10 22:39 ` Eric Blake
@ 2016-06-12 2:58 ` Fam Zheng
2016-06-13 12:36 ` Eric Blake
0 siblings, 1 reply; 19+ messages in thread
From: Fam Zheng @ 2016-06-12 2:58 UTC (permalink / raw)
To: Eric Blake; +Cc: Kevin Wolf, qemu-block, mreitz, qemu-devel, stefanha
On Fri, 06/10 16:39, Eric Blake wrote:
> > +++ b/block/qcow2.c
> > @@ -2903,36 +2903,20 @@ static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
> > int64_t pos)
> > {
> > BDRVQcow2State *s = bs->opaque;
> > - int64_t total_sectors = bs->total_sectors;
> > - bool zero_beyond_eof = bs->zero_beyond_eof;
> > - int ret;
> >
> > BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_SAVE);
> > - bs->zero_beyond_eof = false;
> > - ret = bdrv_pwritev(bs, qcow2_vm_state_offset(s) + pos, qiov);
> > - bs->zero_beyond_eof = zero_beyond_eof;
> > -
> > - /* bdrv_co_do_writev will have increased the total_sectors value to include
> > - * the VM state - the VM state is however not an actual part of the block
> > - * device, therefore, we need to restore the old value. */
> > - bs->total_sectors = total_sectors;
> > -
> > - return ret;
> > + return bs->drv->bdrv_co_pwritev(bs, qcow2_vm_state_offset(s) + pos,
> > + qiov->size, qiov, 0);
> > }
>
> bs->drv->bdrv_co_pwritev() is an optional interface; not all the drivers
> have it yet. Should you be asserting that it exists, and/or returning
> an error if it does not?
bs->drv is always qcow2 driver here so this is correct.
>
> >
> > static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
> > int64_t pos)
> > {
> > BDRVQcow2State *s = bs->opaque;
> > - bool zero_beyond_eof = bs->zero_beyond_eof;
> > - int ret;
> >
> > BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_LOAD);
> > - bs->zero_beyond_eof = false;
> > - ret = bdrv_preadv(bs, qcow2_vm_state_offset(s) + pos, qiov);
> > - bs->zero_beyond_eof = zero_beyond_eof;
> > -
> > - return ret;
> > + return bs->drv->bdrv_co_preadv(bs, qcow2_vm_state_offset(s) + pos,
> > + qiov->size, qiov, 0);
>
> Ditto.
>
Fam
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups
2016-06-10 16:05 [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Kevin Wolf
` (5 preceding siblings ...)
2016-06-10 16:05 ` [Qemu-devel] [PATCH 6/6] block: Remove bs->zero_beyond_eof Kevin Wolf
@ 2016-06-12 2:59 ` Fam Zheng
2016-06-16 8:54 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2016-06-16 9:33 ` [Qemu-devel] " Kevin Wolf
8 siblings, 0 replies; 19+ messages in thread
From: Fam Zheng @ 2016-06-12 2:59 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, eblake, mreitz, qemu-devel, stefanha
On Fri, 06/10 18:05, Kevin Wolf wrote:
> This series contains a few cleanups with respect to the vmstate I/O functions.
> Apart from making the interface more consistent (writes were already vectored,
> but not reads), this makes use of the new byte-based .bdrv_co_preadv/pwritev
> callbacks in qcow2 to get rid of a few hacks, including bs->zero_beyond_eof.
Looks sane to me,
Reviewed-by: Fam Zheng <famz@redhat.com>
(Kevin, this is based on your "qcow2: Implement .bdrv_co_preadv/pwritev"
series, right?)
Fam
>
> Kevin Wolf (6):
> block: Introduce bdrv_preadv()
> block: Make .bdrv_load_vmstate() vectored
> block: Allow .bdrv_load/save_vmstate() to return 0/-errno
> block: Make bdrv_load/save_vmstate coroutine_fns
> qcow2: Let vmstate call qcow2_co_preadv/pwrite directly
> block: Remove bs->zero_beyond_eof
>
> block.c | 2 -
> block/io.c | 173 +++++++++++++++++++++++++++++++---------------
> block/qcow2.c | 28 ++------
> block/sheepdog.c | 13 +++-
> include/block/block.h | 2 +
> include/block/block_int.h | 13 ++--
> 6 files changed, 143 insertions(+), 88 deletions(-)
>
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] qcow2: Let vmstate call qcow2_co_preadv/pwrite directly
2016-06-12 2:58 ` Fam Zheng
@ 2016-06-13 12:36 ` Eric Blake
0 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2016-06-13 12:36 UTC (permalink / raw)
To: Fam Zheng; +Cc: Kevin Wolf, qemu-block, mreitz, qemu-devel, stefanha
[-- Attachment #1: Type: text/plain, Size: 637 bytes --]
On 06/11/2016 08:58 PM, Fam Zheng wrote:
>>> - return ret;
>>> + return bs->drv->bdrv_co_pwritev(bs, qcow2_vm_state_offset(s) + pos,
>>> + qiov->size, qiov, 0);
>>> }
>>
>> bs->drv->bdrv_co_pwritev() is an optional interface; not all the drivers
>> have it yet. Should you be asserting that it exists, and/or returning
>> an error if it does not?
>
> bs->drv is always qcow2 driver here so this is correct.
In that case,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 4/6] block: Make bdrv_load/save_vmstate coroutine_fns
2016-06-10 16:05 ` [Qemu-devel] [PATCH 4/6] block: Make bdrv_load/save_vmstate coroutine_fns Kevin Wolf
2016-06-10 22:33 ` Eric Blake
@ 2016-06-16 8:49 ` Stefan Hajnoczi
1 sibling, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2016-06-16 8:49 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, famz, qemu-devel, mreitz, stefanha
[-- Attachment #1: Type: text/plain, Size: 707 bytes --]
On Fri, Jun 10, 2016 at 06:05:20PM +0200, Kevin Wolf wrote:
> +static int coroutine_fn
> +bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
> + bool is_read)
> +{
> + BlockDriver *drv = bs->drv;
> +
> + if (!drv) {
> + return -ENOMEDIUM;
> + } else if (drv->bdrv_load_vmstate) {
> + return is_read ? drv->bdrv_load_vmstate(bs, qiov, pos)
> + : drv->bdrv_save_vmstate(bs, qiov, pos);
> + } else if (bs->file) {
> + return bdrv_co_rw_vmstate(bs->file->bs, qiov, pos, is_read);
> + }
> +
> + return -ENOTSUP;
> +}
> +
> +static void bdrv_co_rw_vmstate_entry(void *opaque)
This should also be coroutine_fn.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups
2016-06-10 16:05 [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Kevin Wolf
` (6 preceding siblings ...)
2016-06-12 2:59 ` [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Fam Zheng
@ 2016-06-16 8:54 ` Stefan Hajnoczi
2016-06-16 9:33 ` [Qemu-devel] " Kevin Wolf
8 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2016-06-16 8:54 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, famz, qemu-devel, mreitz, stefanha
[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]
On Fri, Jun 10, 2016 at 06:05:16PM +0200, Kevin Wolf wrote:
> This series contains a few cleanups with respect to the vmstate I/O functions.
> Apart from making the interface more consistent (writes were already vectored,
> but not reads), this makes use of the new byte-based .bdrv_co_preadv/pwritev
> callbacks in qcow2 to get rid of a few hacks, including bs->zero_beyond_eof.
>
> Kevin Wolf (6):
> block: Introduce bdrv_preadv()
> block: Make .bdrv_load_vmstate() vectored
> block: Allow .bdrv_load/save_vmstate() to return 0/-errno
> block: Make bdrv_load/save_vmstate coroutine_fns
> qcow2: Let vmstate call qcow2_co_preadv/pwrite directly
> block: Remove bs->zero_beyond_eof
>
> block.c | 2 -
> block/io.c | 173 +++++++++++++++++++++++++++++++---------------
> block/qcow2.c | 28 ++------
> block/sheepdog.c | 13 +++-
> include/block/block.h | 2 +
> include/block/block_int.h | 13 ++--
> 6 files changed, 143 insertions(+), 88 deletions(-)
>
> --
> 1.8.3.1
>
>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups
2016-06-10 16:05 [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Kevin Wolf
` (7 preceding siblings ...)
2016-06-16 8:54 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
@ 2016-06-16 9:33 ` Kevin Wolf
8 siblings, 0 replies; 19+ messages in thread
From: Kevin Wolf @ 2016-06-16 9:33 UTC (permalink / raw)
To: qemu-block; +Cc: eblake, mreitz, qemu-devel, famz, stefanha
Am 10.06.2016 um 18:05 hat Kevin Wolf geschrieben:
> This series contains a few cleanups with respect to the vmstate I/O functions.
> Apart from making the interface more consistent (writes were already vectored,
> but not reads), this makes use of the new byte-based .bdrv_co_preadv/pwritev
> callbacks in qcow2 to get rid of a few hacks, including bs->zero_beyond_eof.
Applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-06-16 9:33 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-10 16:05 [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Kevin Wolf
2016-06-10 16:05 ` [Qemu-devel] [PATCH 1/6] block: Introduce bdrv_preadv() Kevin Wolf
2016-06-10 20:50 ` Eric Blake
2016-06-10 16:05 ` [Qemu-devel] [PATCH 2/6] block: Make .bdrv_load_vmstate() vectored Kevin Wolf
2016-06-10 21:25 ` Eric Blake
2016-06-10 16:05 ` [Qemu-devel] [PATCH 3/6] block: Allow .bdrv_load/save_vmstate() to return 0/-errno Kevin Wolf
2016-06-10 21:29 ` Eric Blake
2016-06-10 16:05 ` [Qemu-devel] [PATCH 4/6] block: Make bdrv_load/save_vmstate coroutine_fns Kevin Wolf
2016-06-10 22:33 ` Eric Blake
2016-06-16 8:49 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2016-06-10 16:05 ` [Qemu-devel] [PATCH 5/6] qcow2: Let vmstate call qcow2_co_preadv/pwrite directly Kevin Wolf
2016-06-10 22:39 ` Eric Blake
2016-06-12 2:58 ` Fam Zheng
2016-06-13 12:36 ` Eric Blake
2016-06-10 16:05 ` [Qemu-devel] [PATCH 6/6] block: Remove bs->zero_beyond_eof Kevin Wolf
2016-06-10 22:41 ` Eric Blake
2016-06-12 2:59 ` [Qemu-devel] [PATCH 0/6] block: bdrv_load/save_vmstate() cleanups Fam Zheng
2016-06-16 8:54 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2016-06-16 9:33 ` [Qemu-devel] " Kevin Wolf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.