1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH v2] domain_conf: skip boot order check of CD-ROM or floppy device when change-media

Chen Hanxiao posted 1 patch 7 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180111101637.19473-1-chen_han_xiao@126.com
src/conf/domain_conf.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
[libvirt] [PATCH v2] domain_conf: skip boot order check of CD-ROM or floppy device when change-media
Posted by Chen Hanxiao 7 years, 11 months ago 
From: Chen Hanxiao <chenhanxiao@gmail.com>

If we insert or eject a CD-ROM/floppy device by:
 'virsh change-media VM --eject/--insert some.iso --live',
and the original CD-ROM device was configed with a boot order,
we may get:
  unsupported configuration: boot order 2 is already used by another device

We just updated 'source file' section rather than hotplug a new device.
This check should be skipped in this case.

Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
---
v2:
  commit message updated
  remove ATTRIBUTE_UNUSED from @device

 src/conf/domain_conf.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a1c25060f..e006cea0a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -26881,17 +26881,26 @@ virDomainDeviceIsUSB(virDomainDeviceDefPtr dev)
 
 static int
 virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def ATTRIBUTE_UNUSED,
-                                  virDomainDeviceDefPtr device ATTRIBUTE_UNUSED,
+                                  virDomainDeviceDefPtr device,
                                   virDomainDeviceInfoPtr info,
                                   void *opaque)
 {
     virDomainDeviceInfoPtr newinfo = opaque;
+    virDomainDiskDefPtr disk = device->data.disk;
+    int disk_device = disk->device;
 
     if (info->bootIndex == newinfo->bootIndex) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("boot order %u is already used by another device"),
-                       newinfo->bootIndex);
-        return -1;
+        /* Skip check for insert or eject CD-ROM device */
+        if (disk_device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
+            disk_device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
+            VIR_DEBUG("Skip boot index check for floppy or CDROM");
+            return 0;
+        } else {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("boot order %u is already used by another device"),
+                           newinfo->bootIndex);
+            return -1;
+        }
     }
     return 0;
 }
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] domain_conf: skip boot order check of CD-ROM or floppy device when change-media
Posted by Ján Tomko 7 years, 11 months ago 
On Thu, Jan 11, 2018 at 06:16:37PM +0800, Chen Hanxiao wrote:
>From: Chen Hanxiao <chenhanxiao@gmail.com>
>
>If we insert or eject a CD-ROM/floppy device by:
> 'virsh change-media VM --eject/--insert some.iso --live',
>and the original CD-ROM device was configed with a boot order,
>we may get:
>  unsupported configuration: boot order 2 is already used by another device
>
>We just updated 'source file' section rather than hotplug a new device.
>This check should be skipped in this case.
>

Attempting to change the boot index on update won't work and should be
forbidden, as stated in the review for v1:
https://www.redhat.com/archives/libvir-list/2018-January/msg00178.html

>Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
>---
>v2:
>  commit message updated
>  remove ATTRIBUTE_UNUSED from @device
>
> src/conf/domain_conf.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index a1c25060f..e006cea0a 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -26881,17 +26881,26 @@ virDomainDeviceIsUSB(virDomainDeviceDefPtr dev)
>
> static int
> virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def ATTRIBUTE_UNUSED,
>-                                  virDomainDeviceDefPtr device ATTRIBUTE_UNUSED,
>+                                  virDomainDeviceDefPtr device,
>                                   virDomainDeviceInfoPtr info,
>                                   void *opaque)
> {
>     virDomainDeviceInfoPtr newinfo = opaque;
>+    virDomainDiskDefPtr disk = device->data.disk;
>+    int disk_device = disk->device;
>
>     if (info->bootIndex == newinfo->bootIndex) {
>-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>-                       _("boot order %u is already used by another device"),
>-                       newinfo->bootIndex);
>-        return -1;
>+        /* Skip check for insert or eject CD-ROM device */
>+        if (disk_device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
>+            disk_device == VIR_DOMAIN_DISK_DEVICE_CDROM) {

Even though cdrom hotplug is not supported by libvirt, assuming that
we're dealing with an update just because of the device type is wrong:
https://www.redhat.com/archives/libvir-list/2018-January/msg00180.html

virDomainDefCompatibleDevice should be aware of the operation (attach
vs. update) and behave accordingly (forbid duplicit bootindexes for
attach and a bootindex change for update)

Jan
>+            VIR_DEBUG("Skip boot index check for floppy or CDROM");
>+            return 0;
>+        } else {
>+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+                           _("boot order %u is already used by another device"),
>+                           newinfo->bootIndex);
>+            return -1;
>+        }
>     }
>     return 0;
> }
>-- 
>2.14.3
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] domain_conf: skip boot order check of CD-ROM or floppy device when change-media
Posted by Chen Hanxiao 7 years, 11 months ago 
At 2018-01-11 21:36:29, "Ján Tomko" <jtomko@redhat.com> wrote:
>On Thu, Jan 11, 2018 at 06:16:37PM +0800, Chen Hanxiao wrote:
>>From: Chen Hanxiao <chenhanxiao@gmail.com>
>>
>>If we insert or eject a CD-ROM/floppy device by:
>> 'virsh change-media VM --eject/--insert some.iso --live',
>>and the original CD-ROM device was configed with a boot order,
>>we may get:
>>  unsupported configuration: boot order 2 is already used by another device
>>
>>We just updated 'source file' section rather than hotplug a new device.
>>This check should be skipped in this case.
>>
>
>Attempting to change the boot index on update won't work and should be
>forbidden, as stated in the review for v1:
>https://www.redhat.com/archives/libvir-list/2018-January/msg00178.html
>

My case is not try to change boot index, but to change-media:

1) boot a VM with a CD-ROM, a centos7.1 ISO inside
2) change iso from centos7.3 to centos7.2 by:
#  change-media c72 hda   /media/b/ISO/CentOS-7-x86_64-DVD-1611.iso  --live
Successfully updated media.

This works and we can see cd-rom changed inside guest.

But if we had <boot order>
    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2'/>
      <source file='/media/b/IMG/c72.qcow2'/>
      <target dev='vda' bus='virtio'/>
 +    <boot order='1'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/>
    </disk>
    <disk type='file' device='cdrom'>
      <driver name='qemu' type='raw'/>
      <source file='/media/b/ISO/CentOS-7-x86_64-DVD-1511.iso'/>
      <target dev='hda' bus='ide'/>
      <readonly/>
 +    <boot order='2'/>
      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
    </disk>

Then change media will fail:
 #  change-media c72 hda   /media/b/ISO/CentOS-7-x86_64-DVD-1511.iso  --live
error: Failed to complete action update on media
error: unsupported configuration: boot order 2 is already used by another device

This is a common case when install OS with multiple DVDs,
or change DVD media when guest is active.

>>Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
>>---
>>v2:
[...]
>>+        /* Skip check for insert or eject CD-ROM device */
>>+        if (disk_device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
>>+            disk_device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
>
>Even though cdrom hotplug is not supported by libvirt, assuming that
>we're dealing with an update just because of the device type is wrong:
>https://www.redhat.com/archives/libvir-list/2018-January/msg00180.html
>
>virDomainDefCompatibleDevice should be aware of the operation (attach
>vs. update) and behave accordingly (forbid duplicit bootindexes for
>attach and a bootindex change for update)
>

As we can successfully 'virsh change-media' without <boot order> of CD-ROM device,
should we forbid this case for a live domain?

Regards,
- Chen

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

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.