[libvirt] [PATCH 0/3] qemu: Don't error out on missing optional disk
More info in PATCH 3/3. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1168453 Martin Kletzander (3): qemu: Make qemuDomainCheckDiskStartupPolicy self-contained qemu: Remove unnecessary label and its only reference qemu: Fix support for startupPolicy with volume/pool disks src/qemu/qemu_domain.c | 36 +++++++++++++++++++++--------------- src/qemu/qemu_domain.h | 5 +++-- src/qemu/qemu_process.c | 20 +++++--------------- 3 files changed, 29 insertions(+), 32 deletions(-) -- 2.9.2
There is an error reset following the function and check for startupPolicy before that. Let's reflect those things inside that function so that future code doesn't have to be that complex. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_domain.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9b439df3b3a8..adbc63fdaa7f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4183,6 +4183,7 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, return -1; break; + case VIR_DOMAIN_STARTUP_POLICY_DEFAULT: case VIR_DOMAIN_STARTUP_POLICY_MANDATORY: return -1; @@ -4191,14 +4192,13 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, return -1; break; - case VIR_DOMAIN_STARTUP_POLICY_DEFAULT: case VIR_DOMAIN_STARTUP_POLICY_LAST: /* this should never happen */ break; } qemuDomainCheckRemoveOptionalDisk(driver, vm, diskIndex); - + virResetLastError(); return 0; } @@ -4232,12 +4232,8 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, if (qemuDomainDetermineDiskChain(driver, vm, disk, true, true) >= 0) continue; - if (disk->startupPolicy && - qemuDomainCheckDiskStartupPolicy(driver, vm, idx, - cold_boot) >= 0) { - virResetLastError(); + if (qemuDomainCheckDiskStartupPolicy(driver, vm, idx, cold_boot) >= 0) continue; - } goto error; } -- 2.9.2
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_domain.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index adbc63fdaa7f..939db02fe353 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4208,7 +4208,6 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, virDomainObjPtr vm, bool cold_boot) { - int ret = -1; size_t i; VIR_DEBUG("Checking for disk presence"); @@ -4235,13 +4234,10 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, if (qemuDomainCheckDiskStartupPolicy(driver, vm, idx, cold_boot) >= 0) continue; - goto error; + return -1; } - ret = 0; - - error: - return ret; + return 0; } /* -- 2.9.2
Until now we simply errored out when the translation from pool+volume failed. However, we should instead check whether that disk is needed or not since there is an option for that. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1168453 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_domain.c | 18 ++++++++++++++++-- src/qemu/qemu_domain.h | 5 +++-- src/qemu/qemu_process.c | 20 +++++--------------- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 939db02fe353..e1c9a9ab156c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -27,6 +27,7 @@ #include "qemu_alias.h" #include "qemu_cgroup.h" #include "qemu_command.h" +#include "qemu_process.h" #include "qemu_parse_command.h" #include "qemu_capabilities.h" #include "qemu_migration.h" @@ -4204,11 +4205,14 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, int -qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, +qemuDomainCheckDiskPresence(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, - bool cold_boot) + unsigned int flags) { size_t i; + bool pretend = flags & VIR_QEMU_PROCESS_START_PRETEND; + bool cold_boot = flags & VIR_QEMU_PROCESS_START_COLD; VIR_DEBUG("Checking for disk presence"); for (i = vm->def->ndisks; i > 0; i--) { @@ -4216,6 +4220,16 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, 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; + if (virStorageSourceIsEmpty(disk->src)) continue; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 6f349a137dcc..9e10ae4c7853 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -545,9 +545,10 @@ void qemuDomainSetFakeReboot(virQEMUDriverPtr driver, bool qemuDomainJobAllowed(qemuDomainObjPrivatePtr priv, qemuDomainJob job); -int qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, +int qemuDomainCheckDiskPresence(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, - bool start_with_state); + unsigned int flags); int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8e1b8961b611..3f4d79f9e7fc 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4792,21 +4792,11 @@ qemuProcessPrepareDomain(virConnectPtr conn, if (qemuProcessSetupGraphics(driver, vm, flags) < 0) goto cleanup; - /* "volume" type disk's source must be translated before - * cgroup and security setting. - */ - for (i = 0; i < vm->def->ndisks; i++) { - if (virStorageTranslateDiskSourcePool(conn, vm->def->disks[i]) < 0) - goto cleanup; - } - - /* drop possibly missing disks from the definition. This needs to happen - * after the def is copied, aliases are set and disk sources are translated */ - if (!(flags & VIR_QEMU_PROCESS_START_PRETEND)) { - if (qemuDomainCheckDiskPresence(driver, vm, - flags & VIR_QEMU_PROCESS_START_COLD) < 0) - goto cleanup; - } + /* Drop possibly missing disks from the definition. This function + * also resolves source pool/volume into a path and it needs to + * happen after the def is copied and aliases are set. */ + if (qemuDomainCheckDiskPresence(conn, driver, vm, flags) < 0) + goto cleanup; VIR_DEBUG("Create domain masterKey"); if (qemuDomainMasterKeyCreate(vm) < 0) -- 2.9.2
On 08/01/2016 12:05 PM, Martin Kletzander wrote:
More info in PATCH 3/3.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1168453
Martin Kletzander (3): qemu: Make qemuDomainCheckDiskStartupPolicy self-contained qemu: Remove unnecessary label and its only reference qemu: Fix support for startupPolicy with volume/pool disks
src/qemu/qemu_domain.c | 36 +++++++++++++++++++++--------------- src/qemu/qemu_domain.h | 5 +++-- src/qemu/qemu_process.c | 20 +++++--------------- 3 files changed, 29 insertions(+), 32 deletions(-)
ACK series, John
participants (2)
-
John Ferlan -
Martin Kletzander