Re: [Qemu-devel] [PATCH v4 10/10] Add the drive-reopen command

[Eric Blake]

On 03/06/2012 10:56 AM, Paolo Bonzini wrote:
> From: Federico Simoncelli <address@hidden>
> 
> Signed-off-by: Federico Simoncelli <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>

>  ##
> +# @drive-reopen
> +#
> +# Assigns a new image file to a device.
> +#
> +# @device: the name of the device for which we are changing the image file.
> +#
> +# @new-image-file: the target of the new image. If the file doesn't exists 
> the
> +#                  command will fail.
> +#
> +# @format: #optional the format of the new image, default is 'qcow2'.
> +#
> +# Returns: nothing on success
> +#          If @device is not a valid block device, DeviceNotFound
> +#          If @new-image-file can't be opened, OpenFileFailed
> +#          If @format is invalid, InvalidBlockFormat
> +#
> +# Since 1.1
> +##
> +{ 'command': 'drive-reopen',
> +  'data': { 'device': 'str', 'new-image-file': 'str', '*format': 'str' } }

I still think we need a 'drive-reopen' action included in 'transaction',
as an 11/10 on this series.  For disk migration, it is true that you can
migrate one disk at a time, and therefore only need to reopen one disk
at a time, to get the guarantee that for a single disk image, the
current state of that image will be guaranteed to be consistent using
only one storage domain.

But since the API allows the creation of two mirrors in one command, I'm
worried that someone will try to start a mirror on two disks at once,
but then be stuck doing two separate 'drive-reopen' commands.  If the
first succeeds but the second fails, you have now stranded the qemu
process across two storage domains, which is exactly what we were trying
to avoid in the first place by inventing transactions.  That is, even if
all disks are individually consistent in a single domain, the act of
migrating then reopening one disk at a time means you will have a window
where disk 1 and disk 2 are opened on different storage domains.

Besides, I'm planning on implementing libvirt support for the
'drive-reopen' command by adding a flag to virDomainSnapshotDelete
(basically, the presence of the flag states that for all mirrored disks
in a given snapshot, libvirt will then issue a drive-reopen that pivots
qemu over to the mirror, and finally delete the snapshot now that
mirroring is no longer needed).  With separate commands, if drive-reopen
on disk 1 succeeds, then drive-reopen on disk 2 fails, I can attempt a
rollback by doing another drive-reopen on disk 1; but the rollback will
be incomplete since I have lost the ability to reopen the mirroring.  I
would much rather issue a 'transaction' with multiple reopen commands,
and knowing that either all disks were reopened and the mirrors
discarded, or that none were reopened and the mirroring remains intact.

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature