[libvirt] [PATCH v2] qemu: don't check for backing chains for formats w/o snapshot support
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1019926 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=868673 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- Notes: Smaller and cleaner variant of v1 [1] after discussion with Eric [1] https://www.redhat.com/archives/libvir-list/2014-April/msg00716.html src/qemu/qemu_domain.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 45ed872..5687075 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2268,11 +2268,14 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, VIR_DEBUG("Checking for disk presence"); for (i = vm->def->ndisks; i > 0; i--) { disk = vm->def->disks[i - 1]; + enum virStorageFileFormat format = virDomainDiskGetFormat(disk); if (!virDomainDiskGetSource(disk)) continue; - if (qemuDomainDetermineDiskChain(driver, vm, disk, false) >= 0 && + if ((format < VIR_STORAGE_FILE_NONE || + format >= VIR_STORAGE_FILE_BACKING) && + qemuDomainDetermineDiskChain(driver, vm, disk, false) >= 0 && qemuDiskChainCheckBroken(disk) >= 0) continue; -- 1.9.2
On Thu, Apr 24, 2014 at 12:32:01 +0200, Martin Kletzander wrote:
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1019926 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=868673
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> ---
Notes: Smaller and cleaner variant of v1 [1] after discussion with Eric
[1] https://www.redhat.com/archives/libvir-list/2014-April/msg00716.html
src/qemu/qemu_domain.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 45ed872..5687075 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2268,11 +2268,14 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, VIR_DEBUG("Checking for disk presence"); for (i = vm->def->ndisks; i > 0; i--) { disk = vm->def->disks[i - 1]; + enum virStorageFileFormat format = virDomainDiskGetFormat(disk);
if (!virDomainDiskGetSource(disk)) continue;
- if (qemuDomainDetermineDiskChain(driver, vm, disk, false) >= 0 && + if ((format < VIR_STORAGE_FILE_NONE || + format >= VIR_STORAGE_FILE_BACKING) && + qemuDomainDetermineDiskChain(driver, vm, disk, false) >= 0 && qemuDiskChainCheckBroken(disk) >= 0) continue;
Better than the original but still... wouldn't it be better to move the check to qemuDomainDetermineDiskChain or even virStorageFileGetMetadata, where we could even use fileTypeInfo[format].getBackingStore as a base for the decision? Jirka
On Thu, Apr 24, 2014 at 12:45:52PM +0200, Jiri Denemark wrote:
On Thu, Apr 24, 2014 at 12:32:01 +0200, Martin Kletzander wrote:
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1019926 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=868673
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> ---
Notes: Smaller and cleaner variant of v1 [1] after discussion with Eric
[1] https://www.redhat.com/archives/libvir-list/2014-April/msg00716.html
src/qemu/qemu_domain.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 45ed872..5687075 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2268,11 +2268,14 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, VIR_DEBUG("Checking for disk presence"); for (i = vm->def->ndisks; i > 0; i--) { disk = vm->def->disks[i - 1]; + enum virStorageFileFormat format = virDomainDiskGetFormat(disk);
if (!virDomainDiskGetSource(disk)) continue;
- if (qemuDomainDetermineDiskChain(driver, vm, disk, false) >= 0 && + if ((format < VIR_STORAGE_FILE_NONE || + format >= VIR_STORAGE_FILE_BACKING) && + qemuDomainDetermineDiskChain(driver, vm, disk, false) >= 0 && qemuDiskChainCheckBroken(disk) >= 0) continue;
Better than the original but still... wouldn't it be better to move the check to qemuDomainDetermineDiskChain or even virStorageFileGetMetadata, where we could even use fileTypeInfo[format].getBackingStore as a base for the decision?
This particular skip is intended precisely for this situation and should not affect any disk chain determination nor should it disable the possibility of getting metadata of images without backing support, so I believe this is the best approach for that. However, since this patch is not applicable anymore, I'll send a v3 in a minute. Martin
participants (2)
-
Jiri Denemark -
Martin Kletzander