Re: [Qemu-devel] [PATCH 5/5] block: extend block-commit to accept a string for the backing file

[Eric Blake]

On 05/14/2014 09:20 PM, Jeff Cody wrote:
> On some image chains, QEMU may not always be able to resolve the
> filenames properly, when updating the backing file of an image
> after a block commit.

Do we want to allow qemu to error out in situations where it might get
things wrong (such as if a gluster protocol is backed by a local
filename) - and mandate that the user supply a backing string in those
situations?  But this patch is a strict improvement even if you don't go
that far, because it gives libvirt the flexibility to shift the burden
of name generation up the stack rather than sticking qemu with that task.

Hmm - how will this be discoverable by libvirt? Maybe when libvirt is
doing the 'qemu -m none' probing, it can hotplug a device pointing to
/dev/null (libvirt _already_ does that to test if add-fd works), and
intentionally omit a node name.  If libvirt then queries the device, and
sees that the __qemu##000NNNN node-name was auto-assigned, then it can
be assumed that this qemu is new enough to provide node-names for ALL
operations (but that means this series is incomplete unless we add
node-name support to all remaining block commands, such as block-stream,
drive-mirror, and drive-backup).  This part is where I wonder if patch
1/5 should be rebased to be last in the series.

> 
> For instance, certain relative pathnames may fail, or drives may
> have been specified originally by file descriptor (e.g. /dev/fd/???),

I'd document this as /dev/fdset/???, which is the magic string QMP uses
with its add-fd command (/dev/fd/??? is platform-specific whether it
will work, /dev/fdset/??? is guaranteed to work in all builds of qemu).

> or a relative protocol pathname may have been used.
> 
> In these instances, QEMU may lack the information to be able to make
> the correct choice, but the user or management layer most likely does
> have that knowledge.
> 
> With this extension to the block-commit api, the user is able to change
> the backing file of the overlay image as part of the block-commit
> operation.
> 
> This allows the change to be 'safe', in the sense that if the attempt
> to write the overlay image metadata fails, then the block-commit
> operation returns failure, without disrupting the guest.
> 
> If the commit top is the active layer, then specifying the backing
> file string will be treated as an error (there is no overlay image
> to modify in that case).
> 
> If a backing file string is not specified in the command, the backing
> file string to use is determined in the same manner as it was
> previously.

In short, this new command option allows the equivalent of 'qemu-img
rebase -u' on a live image.  Definitely a needed functionality.

> 
> Signed-off-by: Jeff Cody <address@hidden>
> ---
>  block.c                   |  8 ++++++--
>  block/commit.c            |  9 ++++++---
>  blockdev.c                |  8 +++++++-
>  include/block/block.h     |  3 ++-
>  include/block/block_int.h |  3 ++-
>  qapi-schema.json          | 18 ++++++++++++++++--
>  qmp-commands.hx           | 14 +++++++++++++-
>  7 files changed, 52 insertions(+), 11 deletions(-)
> 

>      } else {
>          commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs,
> -                    &local_err);
> +                     backing_file, &local_err);

See the rest of the thread about using 'has_backing_file ? backing_file
: NULL' here,


> +# @backing-file: #optional The backing file string to write into the overlay
> +#                          image of 'top'.  If 'top' is the active layer,
> +#                          specifying a backing file string is an error. This
> +#                          backing file string is only written into the the
> +#                          image file metadata - internal structures inside
> +#                          QEMU are not updated,  and the string is not 
> validated.
> +#                          If not specified, QEMU will automatically 
> determine
> +#                          the backing file string to use.  Care should be 
> taken

Maybe "If not specified, QEMU will automatically determine a backing
file string to use, or error out if there is no obvious choice", to
allow us flexibility in erroring out on corner cases such as mixing
gluster with local files.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature