9:09 a.m.
One of the recent refactors broke disk startup policy checking. Fix it with a
few cleanups.
Peter Krempa (3):
qemu: domain: Sanitize return value handling in disk presence checker
qemu: process: Unexport qemuProcessStartValidate
qemu: process: Call disk startup policy check after cloning domain def
src/qemu/qemu_domain.c | 16 +++++-----------
src/qemu/qemu_process.c | 14 +++++++++-----
src/qemu/qemu_process.h | 7 -------
3 files changed, 14 insertions(+), 23 deletions(-)
--
2.8.3
9:09 a.m.
New subject: [libvirt] [PATCH 1/3] qemu: domain: Sanitize return value handling in disk presence checker
One of the functions is returning always 0 and the second one uses
unnecessary labels.
---
src/qemu/qemu_domain.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c21465d..fe64a55 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3863,7 +3863,7 @@ qemuDomainSetFakeReboot(virQEMUDriverPtr driver,
virObjectUnref(cfg);
}
-static int
+static void
qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver,
virDomainObjPtr vm,
size_t diskIndex)
@@ -3895,8 +3895,6 @@ qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver,
}
qemuDomainEventQueue(driver, event);
-
- return 0;
}
static int
@@ -3916,15 +3914,15 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
if (!cold_boot &&
device != VIR_DOMAIN_DISK_DEVICE_FLOPPY &&
device != VIR_DOMAIN_DISK_DEVICE_CDROM)
- goto error;
+ return -1;
break;
case VIR_DOMAIN_STARTUP_POLICY_MANDATORY:
- goto error;
+ return -1;
case VIR_DOMAIN_STARTUP_POLICY_REQUISITE:
if (cold_boot)
- goto error;
+ return -1;
break;
case VIR_DOMAIN_STARTUP_POLICY_DEFAULT:
@@ -3933,13 +3931,9 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver,
break;
}
- if (qemuDomainCheckRemoveOptionalDisk(driver, vm, diskIndex) < 0)
- goto error;
+ qemuDomainCheckRemoveOptionalDisk(driver, vm, diskIndex);
return 0;
-
- error:
- return -1;
}
--
2.8.3
4:09 p.m.
New subject: [libvirt] [PATCH 1/3] qemu: domain: Sanitize return value handling in disk presence checker
On 06/02/2016 10:09 AM, Peter Krempa wrote:
One of the functions is returning always 0 and the second one uses
unnecessary labels.
---
src/qemu/qemu_domain.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
ACK
John
9:09 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: process: Unexport qemuProcessStartValidate
---
src/qemu/qemu_process.c | 2 +-
src/qemu/qemu_process.h | 7 -------
2 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e847cd1..a5bb99e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4325,7 +4325,7 @@ qemuProcessStartValidateXML(virDomainObjPtr vm,
* start the domain but create a valid qemu command. If some code shouldn't be
* executed in this case, make sure to check this flag.
*/
-int
+static int
qemuProcessStartValidate(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virQEMUCapsPtr qemuCaps,
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 61d9f56..13845d7 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -89,13 +89,6 @@ virCommandPtr qemuProcessCreatePretendCmd(virConnectPtr conn,
bool standalone,
unsigned int flags);
-int qemuProcessStartValidate(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virQEMUCapsPtr qemuCaps,
- bool migration,
- bool snap,
- unsigned int flags);
-
int qemuProcessInit(virQEMUDriverPtr driver,
virDomainObjPtr vm,
qemuDomainAsyncJob asyncJob,
--
2.8.3
4:09 p.m.
New subject: [libvirt] [PATCH 2/3] qemu: process: Unexport qemuProcessStartValidate
On 06/02/2016 10:09 AM, Peter Krempa wrote:
---
src/qemu/qemu_process.c | 2 +-
src/qemu/qemu_process.h | 7 -------
2 files changed, 1 insertion(+), 8 deletions(-)
already sent/pushed as patch 5 of series:
http://www.redhat.com/archives/libvir-list/2016-May/msg02000.html
John
1:18 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: process: Unexport qemuProcessStartValidate
On Tue, Jun 07, 2016 at 17:09:53 -0400, John Ferlan wrote:
On 06/02/2016 10:09 AM, Peter Krempa wrote:
> ---
> src/qemu/qemu_process.c | 2 +-
> src/qemu/qemu_process.h | 7 -------
> 2 files changed, 1 insertion(+), 8 deletions(-)
already sent/pushed as patch 5 of series:
I've just noticed that while rebasing. I'm wondering how that happened
as I don't remember cherry-picking the commit to any branch. I either
re-implemented it or messed up a rebase.
Sorry for wasting cycles.
Peter
9:09 a.m.
New subject: [libvirt] [PATCH 3/3] qemu: process: Call disk startup policy check after cloning domain def
In commit 1e38ef72 the disk startup policy check was moved prior to the
call to virDomainObjSetDefTransient which dropped the disk from the
config rather than the def to be started which is a bug.
Additionally we'd not report the disk change event for this since the
disk aliases were not set at that point.
Finally 'volume' based disks would not work with startup policy too.
Fix it by moving it back after the definition is copied, aliases are
assigned and disk sources are translated.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1341415
---
src/qemu/qemu_process.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a5bb99e..ad04a96 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4348,10 +4348,6 @@ qemuProcessStartValidate(virQEMUDriverPtr driver,
}
}
- if (qemuDomainCheckDiskPresence(driver, vm,
- flags & VIR_QEMU_PROCESS_START_COLD) < 0)
- return -1;
-
VIR_DEBUG("Checking domain and device security labels");
if (virSecurityManagerCheckAllLabel(driver->securityManager, vm->def) <
0)
return -1;
@@ -4876,6 +4872,14 @@ qemuProcessPrepareDomain(virConnectPtr conn,
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;
+ }
+
VIR_DEBUG("Create domain masterKey");
if (qemuDomainMasterKeyCreate(vm) < 0)
goto cleanup;
--
2.8.3
4:10 p.m.
New subject: [libvirt] [PATCH 3/3] qemu: process: Call disk startup policy check after cloning domain def
On 06/02/2016 10:09 AM, Peter Krempa wrote:
In commit 1e38ef72 the disk startup policy check was moved prior to
the
call to virDomainObjSetDefTransient which dropped the disk from the
config rather than the def to be started which is a bug.
Additionally we'd not report the disk change event for this since the
disk aliases were not set at that point.
Finally 'volume' based disks would not work with startup policy too.
Fix it by moving it back after the definition is copied, aliases are
assigned and disk sources are translated.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1341415
---
src/qemu/qemu_process.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
Does it make sense to check all disk labels for something that's going
to be dropped? (e.g. call to virSecurityManagerCheckAllLabel also goes
through ndisks)...
Your comment below makes me wonder if there's anything else that got
moved that may have assumed Translate had been run. Would make that
virDomainDiskDefValidate less interesting if the Translate could be done
during Init prior to Validate. My head/eyes hurt
ACK to what's here though - just was trying to think out loud...
John
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a5bb99e..ad04a96 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4348,10 +4348,6 @@ qemuProcessStartValidate(virQEMUDriverPtr driver,
}
}
- if (qemuDomainCheckDiskPresence(driver, vm,
- flags & VIR_QEMU_PROCESS_START_COLD) <
0)
- return -1;
-
VIR_DEBUG("Checking domain and device security labels");
if (virSecurityManagerCheckAllLabel(driver->securityManager, vm->def) <
0)
return -1;
@@ -4876,6 +4872,14 @@ qemuProcessPrepareDomain(virConnectPtr conn,
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;
+ }
+
VIR_DEBUG("Create domain masterKey");
if (qemuDomainMasterKeyCreate(vm) < 0)
goto cleanup;
3119
days inactive
3125
days old
7 comments
2 participants
participants (2)
-
John Ferlan -
Peter Krempa