[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 3/6] qemu-img: Implement commit like QMP
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v4 3/6] qemu-img: Implement commit like QMP |
Date: |
Wed, 16 Apr 2014 16:40:46 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 12.04.2014 um 20:57 hat Max Reitz geschrieben:
> qemu-img should use QMP commands whenever possible in order to ensure
> feature completeness of both online and offline image operations. As
> qemu-img itself has no access to QMP (since this would basically require
> just everything being linked into qemu-img), imitate QMP's
> implementation of block-commit by using commit_active_start() and then
> waiting for the block job to finish.
>
> This new implementation does not empty the snapshot image, as opposed to
> the old implementation using bdrv_commit(). However, as QMP's
> block-commit apparently never did this and as qcow2 (which is probably
> qemu's standard image format) does not even implement the required
> function (bdrv_make_empty()), it does not seem necessary.
>
> Signed-off-by: Max Reitz <address@hidden>
> ---
> block/Makefile.objs | 2 +-
> qemu-img.c | 86
> +++++++++++++++++++++++++++++++++++++++++------------
> 2 files changed, 68 insertions(+), 20 deletions(-)
>
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index fd88c03..2c37e80 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -9,6 +9,7 @@ block-obj-y += snapshot.o qapi.o
> block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
> block-obj-$(CONFIG_POSIX) += raw-posix.o
> block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
> +block-obj-y += mirror.o
>
> ifeq ($(CONFIG_POSIX),y)
> block-obj-y += nbd.o nbd-client.o sheepdog.o
> @@ -22,7 +23,6 @@ endif
>
> common-obj-y += stream.o
> common-obj-y += commit.o
> -common-obj-y += mirror.o
> common-obj-y += backup.o
>
> iscsi.o-cflags := $(LIBISCSI_CFLAGS)
> diff --git a/qemu-img.c b/qemu-img.c
> index 8455994..9fe6384 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -30,6 +30,7 @@
> #include "qemu/osdep.h"
> #include "sysemu/sysemu.h"
> #include "block/block_int.h"
> +#include "block/blockjob.h"
> #include "block/qapi.h"
> #include <getopt.h>
>
> @@ -682,12 +683,49 @@ fail:
> return ret;
> }
>
> +struct CommonBlockJobCBInfo {
> + Error **errp;
> + bool done;
Looks unused (set, but never read).
> +};
> +
> +static void common_block_job_cb(void *opaque, int ret)
> +{
> + struct CommonBlockJobCBInfo *cbi = opaque;
> +
> + if (ret < 0) {
> + error_setg_errno(cbi->errp, -ret, "Block job failed");
> + }
In practice, I guess this will give us rather bad error messages.
Perhaps we need to replace 'int ret' with 'Error *errp' for block job
callbacks in a followup.
> +
> + cbi->done = true;
> +}
> +
> +static void run_block_job(BlockJob *job, struct CommonBlockJobCBInfo *cbi)
> +{
> + BlockJobInfo *info;
> +
> + do {
> + qemu_aio_wait();
> +
> + info = block_job_query(job);
Where does info get freed?
> +
> + if (!info->busy && info->offset < info->len) {
> + block_job_resume(job);
> + }
> + } while (info->offset < info->len);
> +
> + block_job_complete_sync(job, cbi->errp);
> +}
> +
> +/* Same as in block.c */
> +#define COMMIT_BUF_SECTORS 2048
[...]
> + commit_active_start(bs, base_bs, 0, COMMIT_BUF_SECTORS <<
> BDRV_SECTOR_BITS,
> + BLOCKDEV_ON_ERROR_REPORT, common_block_job_cb, &cbi,
> + &local_err);
Though bdrv_commit() uses it for a different purpose: There it's the
buffer size that is used for committing. A single request can never be
larger than this value, but depending on bdrv_is_allocated() it can be
smaller. So the granularity for the decision whether to copy data is
still the granularity of bdrv_is_allocated(), i.e. one cluster.
For the mirror block job, the decision is taken on the granularity that
you specify. This should be the same as for bdrv_commit(), i.e. the
default that you get when you specify 0. mirror_start_job() also has a
buf_size parameter, which is however not exposed by
commit_active_start(). This is where COMMIT_BUF_SECTORS would be right.
Kevin
- [Qemu-devel] [PATCH v4 0/6] qemu-img: Implement commit like QMP, Max Reitz, 2014/04/12
- [Qemu-devel] [PATCH v4 3/6] qemu-img: Implement commit like QMP, Max Reitz, 2014/04/12
- Re: [Qemu-devel] [PATCH v4 3/6] qemu-img: Implement commit like QMP,
Kevin Wolf <=
- [Qemu-devel] [PATCH v4 4/6] qemu-img: Enable progress output for commit, Max Reitz, 2014/04/12
- Re: [Qemu-devel] [PATCH v4 4/6] qemu-img: Enable progress output for commit, Kevin Wolf, 2014/04/16
- Re: [Qemu-devel] [PATCH v4 4/6] qemu-img: Enable progress output for commit, Max Reitz, 2014/04/16
- Re: [Qemu-devel] [PATCH v4 4/6] qemu-img: Enable progress output for commit, Max Reitz, 2014/04/16
- Re: [Qemu-devel] [PATCH v4 4/6] qemu-img: Enable progress output for commit, Kevin Wolf, 2014/04/17
- Re: [Qemu-devel] [PATCH v4 4/6] qemu-img: Enable progress output for commit, Fam Zheng, 2014/04/17
- Re: [Qemu-devel] [PATCH v4 4/6] qemu-img: Enable progress output for commit, Eric Blake, 2014/04/17
- Re: [Qemu-devel] [PATCH v4 4/6] qemu-img: Enable progress output for commit, Max Reitz, 2014/04/17
[Qemu-devel] [PATCH v4 5/6] qemu-img: Specify backing file for commit, Max Reitz, 2014/04/12