Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4328 bytes --]

On 07/24/2012 05:04 AM, Paolo Bonzini wrote:
> This adds the monitor commands that start the mirroring job.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  blockdev.c       |  133 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  hmp-commands.hx  |   21 +++++++++
>  hmp.c            |   28 ++++++++++++
>  hmp.h            |    1 +
>  qapi-schema.json |   35 ++++++++++++++
>  qmp-commands.hx  |   42 +++++++++++++++++
>  trace-events     |    2 +-
>  7 files changed, 258 insertions(+), 4 deletions(-)

This command only allows mirroring from the top or the complete disk,
but no intermediate points.  That is, given:
base <- sn1 <- sn2
I can mirror sn2 in isolation, or the entire chain including base, but I
cannot mirror exactly sn1+sn2.  Instead, I would have to do a mirror of
sn2 then follow up with a partial block-stream to rebase that mirror on
top of base.  I guess this is okay, but it feels a bit limited to have
to do it in two steps instead of one.

[Hmm, your later patches introduce the ability to have a persistent
mapping file; perhaps this can be exploited to have the user pre-create
a mapping file that shows the portions allocated by sn1+sn2 as the
starting point, but I'm not there in the patch series yet to know if
this is the case.]

> +++ b/qapi-schema.json
> @@ -1367,6 +1367,41 @@
>    'returns': 'str' }
>  
>  ##
> +# @drive-mirror
> +#
> +# Start mirroring a block device's writes to a new destination.
> +#
> +# @device:  the name of the device whose writes should be mirrored.
> +#
> +# @target: the target of the new image. If the file exists, or if it
> +#          is a device, the existing file/device will be used as the new
> +#          destination.  If it does not exist, a new file will be created.
> +#
> +# @format: #optional the format of the new destination, default is to
> +#          probe is @mode is 'existing', else the format of the source

s/probe is/probe if/

> +#
> +# @mode: #optional whether and how QEMU should create a new image, default is
> +#        'absolute-paths'.
> +#
> +# @speed:  #optional the maximum speed, in bytes per second
> +#
> +# @sync: what parts of the disk image should be copied to the destination
> +#        (all the disk, only the sectors allocated in the topmost image, or
> +#        only new I/O).

Is there any reason 'sync' is listed last here, but before 'mode' in the
JSON?

> +#
> +# Returns: nothing on success
> +#          If @device is not a valid block device, DeviceNotFound
> +#          If @target can't be opened, OpenFileFailed
> +#          If @format is invalid, InvalidBlockFormat
> +#
> +# Since 1.2
> +##
> +{ 'command': 'drive-mirror',
> +  'data': { 'device': 'str', 'target': 'str', '*format': 'str',
> +            'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
> +            '*speed': 'int' } }
> +

> +drive-mirror
> +------------
> +
> +Start mirroring a block device's writes to a new destination. target
> +specifies the target of the new image. If the file exists, or if it is
> +a device, it will be used as the new destination for writes. If does not
> +exist, a new file will be created. format specifies the format of the
> +mirror image, default is to probe if mode='existing', else the format
> +of the source.
> +
> +Arguments:
> +
> +- "device": device name to operate on (json-string)
> +- "target": name of new image file (json-string)
> +- "format": format of new image (json-string, optional)
> +- "mode": how an image file should be created into the target
> +  file/device (NewImageMode, optional, default 'absolute-paths')

Should we call out all the possible 'NewImageMode' strings here,...

> +- "speed": maximum speed of the streaming job, in bytes per second
> +  (json-int)

Mention that this is optional.

> +- "sync": what parts of the disk image should be copied to the destination;
> +  possibilities include "full" for all the disk, "top" for only the sectors
> +  allocated in the topmost image, or "none" to only replicate new I/O
> +  (MirrorSyncMode).

...given that we did the same for all the possible MirrorSyncMode strings?

-- 
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: 620 bytes --]