[libvirt] [PATCH] qemu: treat iso images as raw
if image format probing is on and image format of iso file is not specified qemu fail to start a domain or change disk media giving errors like [1]. The problem is format is being detected as 'iso' and qemu expect format to be raw for iso images. It makes sense to me because iso refers to filesystem format in image not image format itself. Thus let's just convert iso to raw in case of qemu. There is a similar patch for storage pools - 0e5db762. [1] Unknown driver 'iso' --- ISO as image format was added right at the beginning by e266ded2f without any further comments. Maybe we just can drop ISO from image formats entirely as it is not image format or some hypervisors treat it in a special way? src/qemu/qemu_domain.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c7c9e94..3da9271 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6023,8 +6023,13 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, if (virStorageFileGetMetadata(disk->src, uid, gid, cfg->allowDiskFormatProbing, - report_broken) < 0) + report_broken) < 0) { ret = -1; + goto cleanup; + } + + if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_ISO) + virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW); cleanup: virObjectUnref(cfg); -- 1.8.3.1
On Thu, Oct 26, 2017 at 03:04:08PM +0300, Nikolay Shirokovskiy wrote:
if image format probing is on and image format of iso file is not specified qemu fail to start a domain or change disk media giving errors like [1]. The problem is format is being detected as 'iso' and qemu expect format to be raw for iso images.
It makes sense to me because iso refers to filesystem format in image not image format itself. Thus let's just convert iso to raw in case of qemu.
There is a similar patch for storage pools - 0e5db762.
[1] Unknown driver 'iso'
---
ISO as image format was added right at the beginning by e266ded2f without any further comments. Maybe we just can drop ISO from image formats entirely as it is not image format or some hypervisors treat it in a special way?
Yeah, I'm inclined to say we can drop it. I don't recall either Xen or QEMU caring about an 'iso' disk format
src/qemu/qemu_domain.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c7c9e94..3da9271 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6023,8 +6023,13 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, if (virStorageFileGetMetadata(disk->src, uid, gid, cfg->allowDiskFormatProbing, - report_broken) < 0) + report_broken) < 0) { ret = -1; + goto cleanup; + } + + if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_ISO) + virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
cleanup: virObjectUnref(cfg); --
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, Oct 26, 2017 at 13:09:25 +0100, Daniel Berrange wrote:
On Thu, Oct 26, 2017 at 03:04:08PM +0300, Nikolay Shirokovskiy wrote:
if image format probing is on and image format of iso file is not specified qemu fail to start a domain or change disk media giving errors like [1]. The problem is format is being detected as 'iso' and qemu expect format to be raw for iso images.
It makes sense to me because iso refers to filesystem format in image not image format itself. Thus let's just convert iso to raw in case of qemu.
There is a similar patch for storage pools - 0e5db762.
[1] Unknown driver 'iso'
---
ISO as image format was added right at the beginning by e266ded2f without any further comments. Maybe we just can drop ISO from image formats entirely as it is not image format or some hypervisors treat it in a special way?
Yeah, I'm inclined to say we can drop it. I don't recall either Xen or QEMU caring about an 'iso' disk format
The hypervisors probably don't care about this but the storage driver may care (At least for display purposes).
src/qemu/qemu_domain.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c7c9e94..3da9271 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6023,8 +6023,13 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, if (virStorageFileGetMetadata(disk->src, uid, gid, cfg->allowDiskFormatProbing, - report_broken) < 0) + report_broken) < 0) { ret = -1; + goto cleanup; + } + + if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_ISO) + virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
This is not the right place and also not the correct way. We should reject using ISO as a format if qemu will not support it as an invalid configuration rather than silently turn it into raw. The only acceptable place to turn ISO -> RAW is when it's comming from the storage driver via <disk type=volume>
On 26.10.2017 16:18, Peter Krempa wrote:
On Thu, Oct 26, 2017 at 13:09:25 +0100, Daniel Berrange wrote:
On Thu, Oct 26, 2017 at 03:04:08PM +0300, Nikolay Shirokovskiy wrote:
if image format probing is on and image format of iso file is not specified qemu fail to start a domain or change disk media giving errors like [1]. The problem is format is being detected as 'iso' and qemu expect format to be raw for iso images.
It makes sense to me because iso refers to filesystem format in image not image format itself. Thus let's just convert iso to raw in case of qemu.
There is a similar patch for storage pools - 0e5db762.
[1] Unknown driver 'iso'
---
ISO as image format was added right at the beginning by e266ded2f without any further comments. Maybe we just can drop ISO from image formats entirely as it is not image format or some hypervisors treat it in a special way?
Yeah, I'm inclined to say we can drop it. I don't recall either Xen or QEMU caring about an 'iso' disk format
The hypervisors probably don't care about this but the storage driver may care (At least for display purposes).
src/qemu/qemu_domain.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c7c9e94..3da9271 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6023,8 +6023,13 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, if (virStorageFileGetMetadata(disk->src, uid, gid, cfg->allowDiskFormatProbing, - report_broken) < 0) + report_broken) < 0) { ret = -1; + goto cleanup; + } + + if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_ISO) + virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
This is not the right place and also not the correct way. We should reject using ISO as a format if qemu will not support it as an invalid configuration rather than silently turn it into raw.
But I want to fix the case when iso format is autodetected not specified exlicitly. If we still want to fail in the latter case I guess I can add check that iso comes from autodetection.
The only acceptable place to turn ISO -> RAW is when it's comming from the storage driver via <disk type=volume>
On Thu, Oct 26, 2017 at 16:22:37 +0300, Nikolay Shirokovskiy wrote:
On 26.10.2017 16:18, Peter Krempa wrote:
On Thu, Oct 26, 2017 at 13:09:25 +0100, Daniel Berrange wrote:
On Thu, Oct 26, 2017 at 03:04:08PM +0300, Nikolay Shirokovskiy wrote:
if image format probing is on and image format of iso file is not specified qemu fail to start a domain or change disk media giving errors like [1]. The problem is format is being detected as 'iso' and qemu expect format to be raw for iso images.
It makes sense to me because iso refers to filesystem format in image not image format itself. Thus let's just convert iso to raw in case of qemu.
There is a similar patch for storage pools - 0e5db762.
[1] Unknown driver 'iso'
---
ISO as image format was added right at the beginning by e266ded2f without any further comments. Maybe we just can drop ISO from image formats entirely as it is not image format or some hypervisors treat it in a special way?
Yeah, I'm inclined to say we can drop it. I don't recall either Xen or QEMU caring about an 'iso' disk format
The hypervisors probably don't care about this but the storage driver may care (At least for display purposes).
src/qemu/qemu_domain.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c7c9e94..3da9271 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6023,8 +6023,13 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, if (virStorageFileGetMetadata(disk->src, uid, gid, cfg->allowDiskFormatProbing, - report_broken) < 0) + report_broken) < 0) { ret = -1; + goto cleanup; + } + + if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_ISO) + virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
This is not the right place and also not the correct way. We should reject using ISO as a format if qemu will not support it as an invalid configuration rather than silently turn it into raw.
But I want to fix the case when iso format is autodetected not specified exlicitly. If we still want to fail in the latter case I guess I can add check that iso comes from autodetection.
I don't really think that such case is worth it. Running with format detection enabled is inherently dangerous. We strictly discourage running with format detection enabled. Also the code should still reject the explicit configuration.
participants (3)
-
Daniel P. Berrange -
Nikolay Shirokovskiy -
Peter Krempa