On 06/26/2014 07:51 AM, Eric Blake wrote:
> From: Peter Krempa <pkrempa@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
>
> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list