[libvirt] [PATCH] libxl: fix cdrom default driver name

Christian Ehrhardt posted 1 patch 6 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1500302574-4012-1-git-send-email-christian.ehrhardt@canonical.com
src/libxl/libxl_domain.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[libvirt] [PATCH] libxl: fix cdrom default driver name
Posted by Christian Ehrhardt 6 years, 9 months ago
In recent libvirt libxl does fill non-existant driver elements with type='raw',
but lacks the name='qemu' attribute that it had in the past.
Without that the xml fails to validate, therefore old XML files can't be
edited/defined anymore.

Since the cdrom device type only works with the qemu driver, add that as
the default attribute just as it was added in the past.

Example of the verification error:
  $ virt-xml-validate mytest.xml
  Relax-NG validity error : Extra element devices in interleave

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
---
 src/libxl/libxl_domain.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 68a501c..70ddcea 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -367,8 +367,9 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
         int actual_type = virStorageSourceGetActualType(disk->src);
         int format = virDomainDiskGetFormat(disk);
 
-        /* for network-based disks, set 'qemu' as the default driver */
-        if (actual_type == VIR_STORAGE_TYPE_NETWORK) {
+        /* for network-based disk and cdrom, set 'qemu' as the default driver */
+        if (actual_type == VIR_STORAGE_TYPE_NETWORK ||
+            disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
             if (!virDomainDiskGetDriver(disk) &&
                 virDomainDiskSetDriver(disk, "qemu") < 0)
                 return -1;
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: fix cdrom default driver name
Posted by Laine Stump 6 years, 9 months ago
On 07/17/2017 10:42 AM, Christian Ehrhardt wrote:
> In recent libvirt libxl does fill non-existant driver elements with type='raw',
> but lacks the name='qemu' attribute that it had in the past.
> Without that the xml fails to validate, therefore old XML files can't be
> edited/defined anymore.
>
> Since the cdrom device type only works with the qemu driver, add that as
> the default attribute just as it was added in the past.
>
> Example of the verification error:
>   $ virt-xml-validate mytest.xml
>   Relax-NG validity error : Extra element devices in interleave
>
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> ---
>  src/libxl/libxl_domain.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 68a501c..70ddcea 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -367,8 +367,9 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
>          int actual_type = virStorageSourceGetActualType(disk->src);
>          int format = virDomainDiskGetFormat(disk);
>  
> -        /* for network-based disks, set 'qemu' as the default driver */
> -        if (actual_type == VIR_STORAGE_TYPE_NETWORK) {
> +        /* for network-based disk and cdrom, set 'qemu' as the default driver */
> +        if (actual_type == VIR_STORAGE_TYPE_NETWORK ||
> +            disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
>              if (!virDomainDiskGetDriver(disk) &&
>                  virDomainDiskSetDriver(disk, "qemu") < 0)
>                  return -1;


So is it the case that libxl never uses this attribute, and it's just
being added to the config in order to pass RNG validation? If so, then
my opinion would be that the RNG is too strict, and that attribute
should be made optional.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: fix cdrom default driver name
Posted by Jim Fehlig 6 years, 9 months ago
On 07/18/2017 11:10 AM, Laine Stump wrote:
> On 07/17/2017 10:42 AM, Christian Ehrhardt wrote:
>> In recent libvirt libxl does fill non-existant driver elements with type='raw',
>> but lacks the name='qemu' attribute that it had in the past.
>> Without that the xml fails to validate, therefore old XML files can't be
>> edited/defined anymore.
>>
>> Since the cdrom device type only works with the qemu driver, add that as
>> the default attribute just as it was added in the past.
>>
>> Example of the verification error:
>>    $ virt-xml-validate mytest.xml
>>    Relax-NG validity error : Extra element devices in interleave
>>
>> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
>> ---
>>   src/libxl/libxl_domain.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
>> index 68a501c..70ddcea 100644
>> --- a/src/libxl/libxl_domain.c
>> +++ b/src/libxl/libxl_domain.c
>> @@ -367,8 +367,9 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
>>           int actual_type = virStorageSourceGetActualType(disk->src);
>>           int format = virDomainDiskGetFormat(disk);
>>   
>> -        /* for network-based disks, set 'qemu' as the default driver */
>> -        if (actual_type == VIR_STORAGE_TYPE_NETWORK) {
>> +        /* for network-based disk and cdrom, set 'qemu' as the default driver */
>> +        if (actual_type == VIR_STORAGE_TYPE_NETWORK ||
>> +            disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
>>               if (!virDomainDiskGetDriver(disk) &&
>>                   virDomainDiskSetDriver(disk, "qemu") < 0)
>>                   return -1;
> 
> 
> So is it the case that libxl never uses this attribute, and it's just
> being added to the config in order to pass RNG validation? If so, then
> my opinion would be that the RNG is too strict, and that attribute
> should be made optional.

Yep, agreed. I'd rather relax the RNG than restrict CDROM devices to the qemu 
backend. Although IIRC qemu is currently the only usable backend wrt eject and 
other CDROM-specific operations, blkback should be able to read them just fine. 
And perhaps another CDROM backend will appear in Xen in the future. Instead of 
forcing the qemu backend here, I've sent a patch to relax the RNG

https://www.redhat.com/archives/libvir-list/2017-July/msg00670.html

Regards,
Jim

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