[libvirt] [PATCH] qemu: Fix startupPolicy regression

Commit 82d5fe543720da6d83c1d6bfa1c347d7d9fda278 qemu: check backing chains even when cgroup is omitted added backing file checks just before the code that removes optional disks if they are not present. However, the backing chain code fails in case the disk file does not exist, which makes qemuProcessStart fail regardless on configured startupPolicy. Note that startupPolicy implementation is still wrong after this patch since it only check the first file in a possible chain. It should rather check the complete backing chain. But this is an existing limitation that can be solved later. After all, startupPolicy is most useful for CDROM images and they won't make use of backing files in most cases. --- src/qemu/qemu_process.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1941d4e..3d2b7d6 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3621,14 +3621,15 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; VIR_DEBUG("Checking for CDROM and floppy presence"); + if (qemuDomainCheckDiskPresence(driver, vm, + flags & VIR_QEMU_PROCESS_START_COLD) < 0) + goto cleanup; + for (i = 0; i < vm->def->ndisks ; i++) { if (qemuDomainDetermineDiskChain(driver, vm->def->disks[i], false) < 0) goto cleanup; } - if (qemuDomainCheckDiskPresence(driver, vm, - flags & VIR_QEMU_PROCESS_START_COLD) < 0) - goto cleanup; /* Get the advisory nodeset from numad if 'placement' of * either <vcpu> or <numatune> is 'auto'. -- 1.8.1.5

On 03/18/2013 09:18 PM, Jiri Denemark wrote:
Commit 82d5fe543720da6d83c1d6bfa1c347d7d9fda278
qemu: check backing chains even when cgroup is omitted
added backing file checks just before the code that removes optional disks if they are not present. However, the backing chain code fails in case the disk file does not exist, which makes qemuProcessStart fail regardless on configured startupPolicy.
Note that startupPolicy implementation is still wrong after this patch since it only check the first file in a possible chain. It should rather check the complete backing chain. But this is an existing limitation that can be solved later. After all, startupPolicy is most useful for CDROM images and they won't make use of backing files in most cases. --- src/qemu/qemu_process.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1941d4e..3d2b7d6 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3621,14 +3621,15 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup;
VIR_DEBUG("Checking for CDROM and floppy presence"); + if (qemuDomainCheckDiskPresence(driver, vm, + flags & VIR_QEMU_PROCESS_START_COLD) < 0) + goto cleanup; + for (i = 0; i < vm->def->ndisks ; i++) { if (qemuDomainDetermineDiskChain(driver, vm->def->disks[i], false) < 0) goto cleanup; } - if (qemuDomainCheckDiskPresence(driver, vm, - flags & VIR_QEMU_PROCESS_START_COLD) < 0) - goto cleanup;
/* Get the advisory nodeset from numad if 'placement' of * either <vcpu> or <numatune> is 'auto'.
This fix is also in my patch set, so I give ACK. https://www.redhat.com/archives/libvir-list/2013-March/msg00990.html Guannan

On Mon, Mar 18, 2013 at 21:33:04 +0800, Guannan Ren wrote:
On 03/18/2013 09:18 PM, Jiri Denemark wrote:
Commit 82d5fe543720da6d83c1d6bfa1c347d7d9fda278
qemu: check backing chains even when cgroup is omitted
added backing file checks just before the code that removes optional disks if they are not present. However, the backing chain code fails in case the disk file does not exist, which makes qemuProcessStart fail regardless on configured startupPolicy.
Note that startupPolicy implementation is still wrong after this patch since it only check the first file in a possible chain. It should rather check the complete backing chain. But this is an existing limitation that can be solved later. After all, startupPolicy is most useful for CDROM images and they won't make use of backing files in most cases.
This fix is also in my patch set, so I give ACK. https://www.redhat.com/archives/libvir-list/2013-March/msg00990.html
Oh, I haven't looked at your series yet. I hope you don't mind I pushed my version since it provides more details about the regression. Thanks and pushed. Jirka

qemu: check backing chains even when cgroup is omitted
added backing file checks just before the code that removes optional disks if they are not present. However, the backing chain code fails in case the disk file does not exist, which makes qemuProcessStart fail regardless on configured startupPolicy.
Ahh, this does make sense. Thanks for the fast cleanup, on a case I hadn't tested. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Eric Blake
-
Guannan Ren
-
Jiri Denemark