[libvirt] [PATCH] qemu: domain: Don't treat unknown storage type as not having backing chain

qemuDomainCheckDiskPresence has short-circuit code to skip the determination of the disk backing chain for storage formats that can't have backing volumes. The code treats VIR_STORAGE_FILE_NONE as not having backing chain and skips the call to qemuDomainDetermineDiskChain. This is wrong as qemuDomainDetermineDiskChain is responsible for storage format detection and has logic to determine the default type if format detection is disabled. This allows to storage passed via <disk type="volume"> to circumvent the enforcement to have correct storage format or that we shall default to format='raw', since we don't set the default type via the post parse callback for "volume" backed disks as the translation code could come up with a better guess. This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1328003 --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cae356c..78a717a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3817,7 +3817,7 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, * without backing support, the fact that the file exists is * more than enough */ if (virStorageSourceIsLocalStorage(disk->src) && - format >= VIR_STORAGE_FILE_NONE && + format > VIR_STORAGE_FILE_NONE && format < VIR_STORAGE_FILE_BACKING && virFileExists(virDomainDiskGetSource(disk))) continue; -- 2.8.2

On 05/06/2016 09:48 AM, Peter Krempa wrote:
qemuDomainCheckDiskPresence has short-circuit code to skip the determination of the disk backing chain for storage formats that can't have backing volumes. The code treats VIR_STORAGE_FILE_NONE as not having backing chain and skips the call to qemuDomainDetermineDiskChain.
This is wrong as qemuDomainDetermineDiskChain is responsible for storage format detection and has logic to determine the default type if format detection is disabled.
This allows to storage passed via <disk type="volume"> to circumvent the enforcement to have correct storage format or that we shall default to format='raw', since we don't set the default type via the post parse callback for "volume" backed disks as the translation code could come up with a better guess.
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1328003 --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK... John Do you believe it still would be worthwhile to add: if (virDomainDiskGetFormat(def) == VIR_STORAGE_FILE_NONE) virDomainDiskSetFormat(def, VIR_STORAGE_FILE_RAW); in virStorageTranslateDiskSourcePool in the ISCSI case for MODE_HOST?

On Fri, May 06, 2016 at 11:23:13 -0400, John Ferlan wrote: [...]
Do you believe it still would be worthwhile to add:
if (virDomainDiskGetFormat(def) == VIR_STORAGE_FILE_NONE) virDomainDiskSetFormat(def, VIR_STORAGE_FILE_RAW);
in virStorageTranslateDiskSourcePool in the ISCSI case for MODE_HOST?
You can do that only if format detection is disabled, otherwise it would not be possible to use it. The rest of the code needs to handle the possibility that the format needs to be detected.
participants (2)
-
John Ferlan
-
Peter Krempa