From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38196) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0zDy-00084U-PX for qemu-devel@nongnu.org; Fri, 24 Feb 2012 12:46:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S0zDx-0002jU-8R for qemu-devel@nongnu.org; Fri, 24 Feb 2012 12:46:30 -0500 Received: from mx1.redhat.com ([209.132.183.28]:10194) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0zDx-0002jN-0I for qemu-devel@nongnu.org; Fri, 24 Feb 2012 12:46:29 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q1OHkRpx031603 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 24 Feb 2012 12:46:27 -0500 Message-ID: <4F47CCF2.1090007@redhat.com> Date: Fri, 24 Feb 2012 10:46:26 -0700 From: Eric Blake MIME-Version: 1.0 References: <1329930815-7995-1-git-send-email-fsimonce@redhat.com> <1330102144-14491-2-git-send-email-fsimonce@redhat.com> In-Reply-To: <1330102144-14491-2-git-send-email-fsimonce@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enig03413FC97D3C373DB5A10162" Subject: Re: [Qemu-devel] [PATCH 2/2 v2] Add the blockdev-reopen and blockdev-migrate commands List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Federico Simoncelli Cc: kwolf@redhat.com, armbru@redhat.com, mtosatti@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, pbonzini@redhat.com This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig03413FC97D3C373DB5A10162 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 02/24/2012 09:49 AM, Federico Simoncelli wrote: > Signed-off-by: Federico Simoncelli Pretty sparse on the commit message. > +++ b/hmp-commands.hx > @@ -886,6 +886,44 @@ Snapshot device, using snapshot file as target if = provided > ETEXI > =20 > { > + .name =3D "drive_reopen", > + .args_type =3D "device:B,new-image-file:s?,format:s?", > + .params =3D "device [new-image-file] [format]", > + .help =3D "Assigns a new image file to a device.\n\t\t\t= " > + "The image will be opened using the format\n\t\t= \t" > + "specified or 'qcow2' by default.\n\t\t\t" > + "This command is deprecated.", > + .mhandler.cmd =3D hmp_drive_reopen, Libvirt will just use the QMP version, so I'm not reviewing the HMP version very closely. > +++ b/qapi-schema.json > @@ -1136,6 +1136,60 @@ > 'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str' = } } > =20 > ## > +# @drive-reopen > +# > +# Assigns a new image file to a device. > +# > +# @device: the name of the device for which we are chainging the image= file. s/chainging/changing/ > +# > +# @new-image-file: the target of the new image. If the file doesn't ex= ists the > +# command will fail. I think you need to be more explicit that @new-image-file MUST have identical contents as the current image file, for this to be useful, and that qemu does not validate whether the new image met those conditions. Possible ways to achieve this: 1. Freeze all guest I/O, so that you know the current image file is stable, copy the contents to new-image-file, drive-reopen, then unfreeze guest I/O 2. Create an empty qcow2 file with backing image of the current image file. drive-reopen will then see the new image file as containing the same contents as the previous image file. 3. Create the new image file via mirroring (blockdev-migrate with incremental and new-image-file), and reopen to the mirror > +# > +# @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 > +# > +# Notes: This command is deprecated. Generally, a deprecation note should list the preferred replacement; but we don't have a replacement, so I don't see this note adding any information. > +# > +# Since 1.1 > +## > + > +{ 'command': 'drive-reopen', > + 'data': { 'device': 'str', 'new-image-file': 'str', '*format': 'str'= } } > + > +## > +# @blockdev-migrate > +# > +# Migrates the device to a new destination. > +# > +# @device: the name of the block device to migrate. > +# > +# @incremental: if true only the new writes are sent to the destinatio= n. > +# This method is particularly useful if used in conjunct= ion > +# with new-image-file allowing the current image to be > +# transferred to the destination by an external manager.= > +# > +# @destination: the destination of the block migration. Where do you list what format the destination is? Shouldn't this have an optional format, defaulting to qcow2? Does qemu create the destination file, or must it already be existing? > +# > +# @new-image-file: #optional an existing image file that will replace > +# the current one in the device. Where do you list what format the new-image-file is? Shouldn't this have an optional format, defaulting to qcow2? Does qemu create the new-image-file, or can one already be existing? I know that this patch is only implementing the case where incremental is true and new-image-file is provided; but I'm not quite sure what semantics are intended if incremental is false. Is that still a case where this sets up mirroring (writes go to two images) but additionally the contents from the current image are (asynchronously) streamed into the destination? I guess I'm not sure what the intended semantics of the modes that we aren't implementing, or why we don't just go with a simpler command that only exposes the semantics that we are implementing.= > +# > +# Returns: nothing on success > +# If @device is not a valid block device, DeviceNotFound > +# If @mode is not a valid migration mode, InvalidParameterVal= ue > +# If @destination can't be opened, OpenFileFailed > +# If @new-image-file can't be opened, OpenFileFailed > +# > +# Since 1.1 > +## > +{ 'command': 'blockdev-migrate', > + 'data': { 'device': 'str', 'incremental' : 'bool', > + 'destination': 'str', '*new-image-file': 'str' } } > + > +## > # @human-monitor-command: > # > # Execute a command on the human monitor and return the output. --=20 Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enig03413FC97D3C373DB5A10162 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBCAAGBQJPR8zyAAoJEKeha0olJ0NqZ2MIAJ6EIYkc0hZop/9WMYe1A+Wf JKOn1mUTi1EA1Mk7tWDvWp8RxTjD+DD8vf6B4C30VVCnZnem6s2eN/lfqZvwwOQq ZEz5HX73rm8zldIY/n1kVwNY0fLn8zIY7EUT+VTRMT+SAmfINDT1PFxYeyiLhbKF v4jaC/A4igspA42KeFUMm781qAZLWhJGw5NnVsvwsLW1ciS6ffkHRM+AfVKepVpL a4nPEOAOyJzFANkFIZ8vvxU8MCA1c7RIWURfNwq3R4lPpXEHX5NWJ5L1ND0Z0Cjo qRz5sGUSWiDL0Th4BFhL5hurUTiJN5pUovlaWopBftxhqIjkPmwTUHFr7quLx/M= =ceex -----END PGP SIGNATURE----- --------------enig03413FC97D3C373DB5A10162--