[libvirt] [PATCH v3 3/9] qemuMigrationDriveMirror: Pass disk format to qemu

[John Ferlan]

On 05/26/2015 09:01 AM, Michal Privoznik wrote:
> When playing with disk migration lately, I've noticed this warning in
> domain logs:
> 
> WARNING: Image format was not specified for 'nbd://masina:49153/drive-virtio-disk0' and probing guessed raw.
>          Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
>          Specify the 'raw' format explicitly to remove the restrictions.
> 
> So I started digging into qemu source code to see what has triggered
> the warning. I'd expect qemu to know formats of guest's disks since we
> tell them on command line. This lead me to qmp_drive_mirror() where
> the following can be found:
> 
>     if (!has_format) {
>         format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
>     }
> 
> So, format is automatically initialized from the disk iff mode !=
> "existing". Unfortunately, in migration we are tied to use this mode
> (NBD doesn't support creating new images). Therefore the only way to
> avoid this warning is to pass format. The format that libvirt thinks
> should be in sync with qemu anyway.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_migration.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 32c43f3..279b43f 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1943,6 +1943,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
>      for (i = 0; i < vm->def->ndisks; i++) {
>          virDomainDiskDefPtr disk = vm->def->disks[i];
>          qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
> +        const char *format;

format is not initialized here...

>          int mon_ret;
>  
>          /* skip shared, RO and source-less disks */
> @@ -1950,6 +1951,9 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
>              !virDomainDiskGetSource(disk))
>              continue;
>  
> +        if (disk->src->format > 0)
> +            format = virStorageFileFormatTypeToString(disk->src->format);

then only initialized if src->format > 0

Also this should be VIR_STORAGE_FILE_NONE instead of 0

> +
>          VIR_FREE(diskAlias);
>          VIR_FREE(nbd_dest);
>          if ((virAsprintf(&diskAlias, "%s%s",
> @@ -1967,7 +1971,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
>          }
>  
>          mon_ret = qemuMonitorDriveMirror(priv->mon, diskAlias, nbd_dest,
> -                                         NULL, speed, 0, 0, mirror_flags);
> +                                         format, speed, 0, 0, mirror_flags);

Then used here...  My compiler balked...

Initializing to NULL worked...

ACK with that change.

John
>  
>          if (qemuDomainObjExitMonitor(driver, vm) < 0 || mon_ret < 0) {
>              qemuBlockJobSyncEnd(driver, vm, disk, NULL);
>