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

From: Chen Hanxiao <chenhanxiao@gmail.com> If we insert or eject a CD-ROM/floppy device with a boot order, we may get: unsupported configuration: boot order 2 is already used by another device This check should be skipped in this case. Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- src/conf/domain_conf.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9a62bc472..885ab88d2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26880,11 +26880,19 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def ATTRIBUTE_UNUSED, { virDomainDeviceInfoPtr newinfo = opaque; + int disk_device = device->data.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 12/25/2017 06:21 AM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
If we insert or eject a CD-ROM/floppy device with a boot order, we may get: unsupported configuration: boot order 2 is already used by another device
This check should be skipped in this case.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- src/conf/domain_conf.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
Is there a specific reason for your assertion? Given what scenario? virDomainDefCompatibleDevice is called from Attach and Update qemu/lxc code currently. I dunno, but if someone is trying to attach or update a floppy/cdrom and wanted to use a boot index in use by something else, then I would think inhibiting that is a good idea...
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9a62bc472..885ab88d2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26880,11 +26880,19 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def ATTRIBUTE_UNUSED, { virDomainDeviceInfoPtr newinfo = opaque;
+ int disk_device = device->data.disk->device;
Typically the deref would be something like virDomainDiskDefPtr disk = device->data.disk; and then "disk->device" BTW: @device would then no longer be ATTRIBUTE_UNUSED. John
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; }

On Fri, Jan 05, 2018 at 10:01:50AM -0500, John Ferlan wrote:
On 12/25/2017 06:21 AM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
If we insert or eject a CD-ROM/floppy device with a boot order, we may get: unsupported configuration: boot order 2 is already used by another device
This check should be skipped in this case.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- src/conf/domain_conf.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
Is there a specific reason for your assertion? Given what scenario?
virDomainDefCompatibleDevice is called from Attach and Update qemu/lxc code currently.
I dunno, but if someone is trying to attach or update a floppy/cdrom and wanted to use a boot index in use by something else, then I would think inhibiting that is a good idea...
Yes. Also, even though libvirt does not support cdrom/floppy hotplug, assuming that this is an update based only on the disk type feels wrong. Jan

At 2018-01-05 23:01:50, "John Ferlan" <jferlan@redhat.com> wrote:
On 12/25/2017 06:21 AM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
If we insert or eject a CD-ROM/floppy device with a boot order, we may get: unsupported configuration: boot order 2 is already used by another device
This check should be skipped in this case.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- src/conf/domain_conf.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
Is there a specific reason for your assertion? Given what scenario?
1) create a VM by virt-manger with a virtio disk and a cd-rom 2) set boot order of them 3) Try to eject a CD-rom device, such as installing an OS with multiple DVDs i. e: virsh change-media ... 4) got: unsupported configuration: boot order 2 is already used by another device.
virDomainDefCompatibleDevice is called from Attach and Update qemu/lxc code currently.
I dunno, but if someone is trying to attach or update a floppy/cdrom and wanted to use a boot index in use by something else, then I would think inhibiting that is a good idea...
We updated the source file of floppy/cdrom, which is not a hotplug operation, so the boot order check should be skipped. Regards, - Chen
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9a62bc472..885ab88d2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -26880,11 +26880,19 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def ATTRIBUTE_UNUSED, { virDomainDeviceInfoPtr newinfo = opaque;
+ int disk_device = device->data.disk->device;
Typically the deref would be something like virDomainDiskDefPtr disk = device->data.disk; and then "disk->device"
BTW: @device would then no longer be ATTRIBUTE_UNUSED.
John
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; }
participants (3)
-
Chen Hanxiao
-
John Ferlan
-
Ján Tomko