[libvirt] [PATCH 5/7] qemu: process: Move 'volume' translation to domain prepare stage

[John Ferlan]

On 10/04/2017 07:42 AM, Peter Krempa wrote:
> Introduce a new function to prepare domain disks which will also do the
> volume source to actual disk source translation.
> ---
>  src/qemu/qemu_domain.c  | 10 +---------
>  src/qemu/qemu_domain.h  |  3 +--
>  src/qemu/qemu_process.c | 36 ++++++++++++++++++++++++++++++++----
>  3 files changed, 34 insertions(+), 15 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 8aa082618..bf2ce29bf 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5682,8 +5682,7 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
> 
> 
>  int
> -qemuDomainCheckDiskPresence(virConnectPtr conn,
> -                            virQEMUDriverPtr driver,
> +qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
>                              virDomainObjPtr vm,
>                              unsigned int flags)
>  {
> @@ -5697,13 +5696,6 @@ qemuDomainCheckDiskPresence(virConnectPtr conn,
>          virDomainDiskDefPtr disk = vm->def->disks[idx];
>          virStorageFileFormat format = virDomainDiskGetFormat(disk);
> 
> -        if (virStorageTranslateDiskSourcePool(conn, vm->def->disks[idx]) < 0) {
> -            if (pretend ||
> -                qemuDomainCheckDiskStartupPolicy(driver, vm, idx, cold_boot) < 0)
> -                return -1;
> -            continue;
> -        }
> -
>          if (pretend)
>              continue;

So the reality is we don't even have to run through any of the loop if
pretend == true as we're literally doing nothing with it.

I know, moot point since 2 patches later the whole function moves to
being called from PrepareHost which never checked pretend anyway -
although it perhaps could now that PrepareHost takes @flags

> 
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index b3db50c2f..914f2bec9 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -644,8 +644,7 @@ int qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
>                                       size_t diskIndex,
>                                       bool cold_boot);
> 
> -int qemuDomainCheckDiskPresence(virConnectPtr conn,
> -                                virQEMUDriverPtr driver,
> +int qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
>                                  virDomainObjPtr vm,
>                                  unsigned int flags);
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index dfaacbcb9..ad7c7ee81 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5275,6 +5275,32 @@ qemuProcessPrepareDomainNUMAPlacement(virDomainObjPtr vm,
>  }
> 
> 
> +static int
> +qemuProcessPrepareDomainStorage(virConnectPtr conn,
> +                                virQEMUDriverPtr driver,
> +                                virDomainObjPtr vm,
> +                                unsigned int flags)
> +{
> +    size_t i;
> +    bool cold_boot = flags & VIR_QEMU_PROCESS_START_COLD;
> +
> +    for (i = vm->def->ndisks; i > 0; i--) {
> +        size_t idx = i - 1;
> +        virDomainDiskDefPtr disk = vm->def->disks[idx];
> +
> +        if (virStorageTranslateDiskSourcePool(conn, disk) < 0) {
> +            if (qemuDomainCheckDiskStartupPolicy(driver, vm, idx, cold_boot) < 0)

Missing the "pretend ||" failure option/shortcut from commit 'a2b97a8d'

I would think it would be necessary in this failure path since it's
still eventually going to be called via/through PrepareDomain which
seems to care about pretending...  Although make check passes, so who knows.


> +                return -1;
> +
> +            /* disk source was dropped */
> +            continue;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> Reviewed-by: John Ferlan <jferlan at redhat.com>