[libvirt] [PATCH v2] domain_conf: skip boot order check of CD-ROM or floppy device when change-media
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
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
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
participants (2)
-
Chen Hanxiao -
Ján Tomko