[libvirt] [PATCH v1.2.[1-5]-maint] qemu: blockcopy: Don't remove existing disk mirror info

[Eric Blake]

On 06/26/2014 07:51 AM, Eric Blake wrote:
> From: Peter Krempa <pkrempa at redhat.com>
> 
> When creating a new disk mirror the new struct is stored in a separate
> variable until everything went well. The removed hunk would actually
> remove existing mirror information for example when the api would be run
> if a mirror still exists.
> 
> (cherry picked from commit 02b364e186d487f54ed410c01af042f23e812d42)
> 
> This fixes a regression introduced in commit ff5f30b.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> 
> Conflicts:
> 	src/qemu/qemu_driver.c - no refactoring of commit 7b7bf001
> ---
> 
> As Peter's patch resolves a regression, I'd like to backport it to
> the maint branches; however, that means redoing the patch.
> 
>  src/qemu/qemu_driver.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 59185c6..591864f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -15203,6 +15203,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
>      struct stat st;
>      bool need_unlink = false;
>      char *mirror = NULL;
> +    int mirrorFormat;

Uninitialized...

>      virQEMUDriverConfigPtr cfg = NULL;
> 
>      /* Preliminaries: find the disk we are editing, sanity checks */
> @@ -15290,10 +15291,10 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
>              goto endjob;
>          VIR_FORCE_CLOSE(fd);
>          if (!format)
> -            disk->mirrorFormat = disk->src.format;
> +            mirrorFormat = disk->src.format;

but here, if the user did not request reusing a file but DID request
raw, mirrorFormat is still uninitialized...

>      } else if (format) {
> -        disk->mirrorFormat = virStorageFileFormatTypeFromString(format);
> -        if (disk->mirrorFormat <= 0) {
> +        mirrorFormat = virStorageFileFormatTypeFromString(format);
> +        if (mirrorFormat <= 0) {
>              virReportError(VIR_ERR_INVALID_ARG, _("unrecognized format '%s'"),
>                             format);
>              goto endjob;
> @@ -15303,11 +15304,11 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
>           * also passed the RAW flag (and format is non-NULL), or it is
>           * safe for us to probe the format from the file that we will
>           * be using.  */
> -        disk->mirrorFormat = virStorageFileProbeFormat(dest, cfg->user,
> -                                                       cfg->group);
> +        mirrorFormat = virStorageFileProbeFormat(dest, cfg->user,
> +                                                 cfg->group);
>      }
> -    if (!format && disk->mirrorFormat > 0)
> -        format = virStorageFileFormatTypeToString(disk->mirrorFormat);
> +    if (!format && mirrorFormat > 0)
> +        format = virStorageFileFormatTypeToString(mirrorFormat);

and here we are using the uninitialized value :(

It looks like this has been broken ever since v1.0.0 (when blockcopy was
first introduced), so it is not a regression that we have been
mishandling 'virsh blockcopy $dom vda /path/to/file --raw', but
disk->mirrorFormat was at least initialized to 0 compared to my patch
using an uninitialized value.  Peter will be proposing a patch soon,
which must make it into 1.2.6.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140701/0ae16de9/attachment-0001.sig>