1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] qemu: treat iso images as raw

Nikolay Shirokovskiy posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1509019448-1001364-1-git-send-email-nshirokovskiy@virtuozzo.com
src/qemu/qemu_domain.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
[libvirt] [PATCH] qemu: treat iso images as raw
Posted by Nikolay Shirokovskiy 6 years, 6 months ago 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: treat iso images as raw
Posted by Daniel P. Berrange 6 years, 6 months ago 
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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: treat iso images as raw
Posted by Peter Krempa 6 years, 6 months ago 
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>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: treat iso images as raw
Posted by Nikolay Shirokovskiy 6 years, 6 months ago 

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>
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: treat iso images as raw
Posted by Peter Krempa 6 years, 6 months ago 
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.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.