qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: kwolf@redhat.com, fsimonce@redhat.com, qemu-devel@nongnu.org,
	stefanha@linux.vnet.ibm.com, lcapitulino@redhat.com
Subject: Re: [Qemu-devel] [PATCH v4 10/10] Add the drive-reopen command
Date: Tue, 13 Mar 2012 14:48:10 -0600	[thread overview]
Message-ID: <4F5FB28A.4090605@redhat.com> (raw)
In-Reply-To: <1331056563-7503-11-git-send-email-pbonzini@redhat.com>

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

On 03/06/2012 10:56 AM, Paolo Bonzini wrote:
> From: Federico Simoncelli <fsimonce@redhat.com>
> 
> Signed-off-by: Federico Simoncelli <fsimonce@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

>  ##
> +# @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   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 --]

  reply	other threads:[~2012-03-13 20:48 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-06 17:55 [Qemu-devel] [PATCH v4 00/10] Mirrored block writes Paolo Bonzini
2012-03-06 17:55 ` [Qemu-devel] [PATCH v4 01/10] use QSIMPLEQ_FOREACH_SAFE when freeing list elements Paolo Bonzini
2012-03-06 17:55 ` [Qemu-devel] [PATCH v4 02/10] fix format name for backing file Paolo Bonzini
2012-03-06 17:55 ` [Qemu-devel] [PATCH v4 03/10] qapi: complete implementation of unions Paolo Bonzini
2012-03-06 17:55 ` [Qemu-devel] [PATCH v4 04/10] rename blockdev-group-snapshot-sync Paolo Bonzini
2012-03-06 17:55 ` [Qemu-devel] [PATCH v4 05/10] add mode field to blockdev-snapshot-sync transaction item Paolo Bonzini
2012-03-06 17:55 ` [Qemu-devel] [PATCH v4 06/10] qmp: convert blockdev-snapshot-sync to a wrapper around transactions Paolo Bonzini
2012-03-06 17:56 ` [Qemu-devel] [PATCH v4 07/10] Add blkmirror block driver Paolo Bonzini
2012-03-06 17:56 ` [Qemu-devel] [PATCH v4 08/10] add mirroring to transaction Paolo Bonzini
2012-03-06 17:56 ` [Qemu-devel] [PATCH v4 09/10] add drive-mirror command and HMP equivalent Paolo Bonzini
2012-03-06 17:56 ` [Qemu-devel] [PATCH v4 10/10] Add the drive-reopen command Paolo Bonzini
2012-03-13 20:48   ` Eric Blake [this message]
2012-03-14  0:14     ` Federico Simoncelli
2012-03-14  9:34       ` Kevin Wolf
2012-03-14 13:11         ` Eric Blake
2012-03-14 14:30           ` Kevin Wolf
2012-03-14 13:29         ` Federico Simoncelli
2012-03-14  9:17     ` Kevin Wolf
2012-03-14  9:19       ` Paolo Bonzini
2012-03-14  9:35         ` Kevin Wolf
2012-03-09 15:36 ` [Qemu-devel] [PATCH v4 00/10] Mirrored block writes Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F5FB28A.4090605@redhat.com \
    --to=eblake@redhat.com \
    --cc=fsimonce@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).