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@xxxxxxxxxx>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
