[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 11/11] qmp: add command 'blockdev-backup'
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v2 11/11] qmp: add command 'blockdev-backup' |
Date: |
Tue, 23 Jul 2013 12:10:21 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Fri, Jul 19, 2013 at 06:16:55PM +0800, Wenchao Xia wrote:
> δΊ 2013-7-18 12:41, Fam Zheng ει:
> >On Wed, 07/17 06:44, Eric Blake wrote:
> >>On 07/17/2013 03:42 AM, Fam Zheng wrote:
> >>>Similar to drive-backup, but this command uses a device id as target
> >>>instead of creating/opening an image file.
> >>>
> >>>Signed-off-by: Fam Zheng <address@hidden>
> >>>---
> >>> blockdev.c | 71
> >>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>> qapi-schema.json | 49 ++++++++++++++++++++++++++++++++++++++
> >>> qmp-commands.hx | 22 ++++++++++++++++++
> >>> 3 files changed, 142 insertions(+)
> >>>
> >>
> >>>+++ b/qapi-schema.json
> >>>@@ -1665,6 +1665,40 @@
> >>> '*on-target-error': 'BlockdevOnError' } }
> >>>
> >>> ##
> >>>+# @BlockdevBackup
> >>>+#
> >>
> >>>+{ 'type': 'BlockdevBackup',
> >>>+ 'data': { 'device': 'str', 'target': 'str',
> >>>+ 'sync': 'MirrorSyncMode',
> >>>+ '*speed': 'int',
> >>>+ '*on-source-error': 'BlockdevOnError',
> >>>+ '*on-target-error': 'BlockdevOnError' } }
> >>
> >>Seems okay. But what is missing is the addition of this type into the
> >>union used for 'transaction' - shouldn't it be possible to mix this with
> >>other transaction capabilities?
> >>
> >
> >Should be possible, as users may want point-in-time snapshot of multiple
> >disks. If this API looks OK, I will include it into transaction in the
> >next version.
> >
> Instead of add this new API, how about extend Driver-backup API? This
> patch is basically doing similar things with driver-backup, extension
> of old API will save trouble to do same things driver-backup already
> does, such as support qmp_transaction.
The rationale was that drive-* commands should be high-level and can
create image files. blockdev-* commands should be low-level and work on
an existing -drive.
The reason for keeping two separate commands is to avoid adding in
parameters that work at different levels (filename vs drive name).
In terms of API design I think drive- + blockdev- really is cleaner.
Kevin can explain in more detail if I got something wrong.
Stefan
- Re: [Qemu-devel] [PATCH v2 10/11] block: add option 'backing' to -drive options, (continued)
- Re: [Qemu-devel] [PATCH v2 10/11] block: add option 'backing' to -drive options, Paolo Bonzini, 2013/07/17
- Re: [Qemu-devel] [PATCH v2 10/11] block: add option 'backing' to -drive options, Kevin Wolf, 2013/07/17
- Re: [Qemu-devel] [PATCH v2 10/11] block: add option 'backing' to -drive options, Paolo Bonzini, 2013/07/17
- Re: [Qemu-devel] [PATCH v2 10/11] block: add option 'backing' to -drive options, Kevin Wolf, 2013/07/17
- Re: [Qemu-devel] [PATCH v2 10/11] block: add option 'backing' to -drive options, Paolo Bonzini, 2013/07/17
- Re: [Qemu-devel] [PATCH v2 10/11] block: add option 'backing' to -drive options, Fam Zheng, 2013/07/22
[Qemu-devel] [PATCH v2 11/11] qmp: add command 'blockdev-backup', Fam Zheng, 2013/07/17
Re: [Qemu-devel] [PATCH v2 00/11] Point-in-time snapshot exporting over NBD, Wenchao Xia, 2013/07/19