Re: [Qemu-devel] [PATCH v8 for 2.1 4/4] block: add QAPI command to allow live backing file change

[Eric Blake <eblake@redhat.com>]

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

On 07/01/2014 01:52 AM, Stefan Hajnoczi wrote:
> From: Jeff Cody <jcody@redhat.com>
> 
> This allows a user to make a live change to the backing file recorded in
> an open image.
> 
> The image file to modify can be specified 2 ways:
> 
> 1) image filename
> 2) image node-name
> 

This part of the commit message is now stale, but that's okay...

> Note: this does not cause the backing file itself to be reopened; it
> merely changes the backing filename in the image file structure, and
> in internal BDS structures.
> 
> It is the responsibility of the user to pass a filename string that
> can be resolved when the image chain is reopened, and the filename
> string is not validated.
> 
> A good analogy for this command is that it is a live version of
> 'qemu-img rebase -u', with respect to changing the backing file string.
> 
> [Jeff is offline so I respun this patch in his absence.  Dropped image
> filename since using node-name is preferred and this is a new command.
> No need to introduce the limitations of finding images by filename.
> --Stefan]

...given that you are preserving history while documenting changes.

> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

This is a big enough change that I re-reviewed the patch.  The reduced
capability of the command is not an issue for how libvirt plans to use
it (mainly as a witness that libvirt has control over backing file
strings); and it is still desirable from libvirt's point of view to
introduce the command.

Thanks for fixing this up.

> ---
>  blockdev.c           | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi/block-core.json | 26 +++++++++++++++++
>  qmp-commands.hx      | 39 ++++++++++++++++++++++++++
>  3 files changed, 144 insertions(+)
> 


> +SQMP
> +change-backing-file
> +-------------------
> +Since: 2.1
> +
> +Change the backing file in the image file metadata.  This does not cause
> +QEMU to reopen the image file to reparse the backing filename (it may,
> +however, perform a reopen to change permissions from r/o -> r/w -> r/o,
> +if needed). The new backing file string is written into the image file
> +metadata, and the QEMU internal strings are updated.
> +
> +Arguments:
> +
> +- "image-node-name":    The name of the block driver state node of the
> +                        image to modify.  The "device" is argument is used to
> +                        verify "image-node-name" is in the chain described by
> +                        "device".
> +                        (json-string, optional)

No longer optional.  If you still have time to fix it, great; if not,
it's minor enough.  My R-b still stands.
-- 
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: 604 bytes --]