Re: [Qemu-devel] [PATCH 5/8] block: add drive-mirror command

[Eric Blake]

On 04/13/2012 10:23 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---

> +    flags = bs->open_flags | BDRV_O_RDWR;
> +    source = bs->backing_hd;

> +    /* create new image w/backing file */
> +    if (full && mode != NEW_IMAGE_MODE_EXISTING) {
> +        assert(format && drv);
> +        bdrv_get_geometry(bs, &size);
> +        size *= 512;
> +        ret = bdrv_img_create(target, format,
> +                              NULL, NULL, NULL, size, flags);

This side of the branch forces the size (which makes sense, since there
is no backing file to probe it from),...

> +    } else {
> +        switch (mode) {
> +        case NEW_IMAGE_MODE_EXISTING:
> +            ret = 0;
> +            break;
> +        case NEW_IMAGE_MODE_ABSOLUTE_PATHS:
> +            ret = bdrv_img_create(target, format,
> +                                  source->filename,
> +                                  source->drv->format_name,
> +                                  NULL, -1, flags);

...but this side passes -1 (which I assume means to probe the backing
file for its size, and reuse that size).  But is this always right, or
shouldn't this side of the branch _also_ be calling bdrv_get_geometry
and passing in an explicit size?  Should we be using BDRV_O_NO_BACKING
to avoid potential problems in temporarily reopening a file that we
already have open due to source?

Am I correct that bdrv_img_create will fail if I attempt to use a driver
that doesn't support backing files (think raw) but include a request to
set the backing file?  (Put another way, I'm wondering whether libvirt
can trust qemu to do error detection, or whether libvirt must filter out:
virDomainBlockRebase(, _COPY | _SHALLOW | _COPY_RAW)
as reasonable on an existing raw image, but as an error if the existing
image already uses a backing file.

Just to make sure: if I pass in an existing file, then it is expected
that the data in that image is either identical to the data of the
backing file (if full==false) or is conceptually uninitialized (if
fall=true).  We document that there might be rare cases of wanting to
alter contents - I'm assuming that one such case would be to pass in an
existing file with a larger size than the source, such that when we then
do disk-reopen, we've achieved a poor-man's block_resize.  Does the code
react well to the existing destination having a different size than the
source?

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

signature.asc
Description: OpenPGP digital signature