[libvirt] [PATCH v2 0/3] Disable floppy disk for PowerPC VMs

This is an attempt to fix: Libvirt BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1180486 Libvirt currently assumes ISA_based floppy disks to be available across all architectures and machine types. However, PowerPC Book 3S compatible ('ie pseries) virtual machines do not support Floppy disks. This patch series prevents libvirt from launching ppc64[le] -based 'pseries' VMs with floppy devices. This version of patch attempts to fulfill the coments of Andrea Bolognani and Jan Tomko on previous version of patch. --- Kothapally Madhu Pavan (3): Caps: Disable floppy disk for PowerPC VM Avoid starting a PowerPC VM with floppy disk test: Introduce test case to disallow floppy disk on pseries src/qemu/qemu_capabilities.c | 15 +++++-- src/qemu/qemu_command.c | 8 ++++ .../qemuxml2argv-disk-floppy-pseries.args | 6 +++ .../qemuxml2argv-disk-floppy-pseries.xml | 41 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 5 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.xml -- Kothapally Madhu Pavan

PowerPC pseries based VMs do not support a floppy disk controller. This prohibits libvirt from adding floppy disk for a PowerPC pseries VM. Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d8cb32d..e304473 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3919,25 +3919,32 @@ virQEMUCapsFillDomainOSCaps(virQEMUCapsPtr qemuCaps, static int virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, + const char *machine, virDomainCapsDeviceDiskPtr disk) { disk->device.supported = true; /* QEMU supports all of these */ VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, VIR_DOMAIN_DISK_DEVICE_DISK, - VIR_DOMAIN_DISK_DEVICE_CDROM, - VIR_DOMAIN_DISK_DEVICE_FLOPPY); + VIR_DOMAIN_DISK_DEVICE_CDROM); + + /* PowerPC pseries based VMs do not support floppy device */ + if (!(ARCH_IS_PPC64(qemuCaps->arch) && STRPREFIX(machine, "pseries"))) + VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, VIR_DOMAIN_DISK_DEVICE_FLOPPY); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_SG_IO)) VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, VIR_DOMAIN_DISK_DEVICE_LUN); VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_IDE, - VIR_DOMAIN_DISK_BUS_FDC, VIR_DOMAIN_DISK_BUS_SCSI, VIR_DOMAIN_DISK_BUS_VIRTIO, /* VIR_DOMAIN_DISK_BUS_SD */); + /* PowerPC pseries based VMs do not support floppy device */ + if (!(ARCH_IS_PPC64(qemuCaps->arch) && STRPREFIX(machine, "pseries"))) + VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_FDC); + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_USB); return 0; @@ -4008,7 +4015,7 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, if (virQEMUCapsFillDomainOSCaps(qemuCaps, os, loader, nloader) < 0 || - virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, disk) < 0 || + virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, domCaps->machine, disk) < 0 || virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0) return -1; return 0;

PowerPC pseries based VMs do not support a floppy disk controller. This prohibits libvirt from creating qemu command with floppy device. Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 09f30c4..501c7df 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9767,6 +9767,14 @@ qemuBuildCommandLine(virConnectPtr conn, continue; } + /* PowerPC pseries based VMs do not support floppy device */ + if ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) && + ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("PowerPC pseries machines do not support floppy device")); + goto error; + } + switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: bootindex = bootCD;

On Thu, Jul 30, 2015 at 07:55:36AM -0400, Kothapally Madhu Pavan wrote:
PowerPC pseries based VMs do not support a floppy disk controller.
Here the commit message mentions a controller, but the code only checks for the <disk type='floppy'/> device, not for <controller type='fdc'/> Should we forbid that one too? AFAIK we've auto-added it to the XML only if there was at least floppy. Either way, the series looks good to me. Jan
This prohibits libvirt from creating qemu command with floppy device.
Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 09f30c4..501c7df 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9767,6 +9767,14 @@ qemuBuildCommandLine(virConnectPtr conn, continue; }
+ /* PowerPC pseries based VMs do not support floppy device */ + if ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) && + ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("PowerPC pseries machines do not support floppy device")); + goto error; + } + switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: bootindex = bootCD;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 08/03/2015 06:23 PM, Ján Tomko wrote:
On Thu, Jul 30, 2015 at 07:55:36AM -0400, Kothapally Madhu Pavan wrote:
PowerPC pseries based VMs do not support a floppy disk controller. Here the commit message mentions a controller, but the code only checks for the <disk type='floppy'/> device, not for <controller type='fdc'/>
Should we forbid that one too? AFAIK we've auto-added it to the XML only if there was at least floppy. We need not forbid fdc, as checking for floppy device will fulfill the need. Yes, fdc is added only if there was at least one floppy device.
Either way, the series looks good to me.
Jan
This prohibits libvirt from creating qemu command with floppy device.
Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 09f30c4..501c7df 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9767,6 +9767,14 @@ qemuBuildCommandLine(virConnectPtr conn, continue; }
+ /* PowerPC pseries based VMs do not support floppy device */ + if ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) && + ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("PowerPC pseries machines do not support floppy device")); + goto error; + } + switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: bootindex = bootCD;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Thanks, Madhu Pavan.

On Mon, Aug 03, 2015 at 11:27:56PM +0530, madhu pavan wrote:
On 08/03/2015 06:23 PM, Ján Tomko wrote:
On Thu, Jul 30, 2015 at 07:55:36AM -0400, Kothapally Madhu Pavan wrote:
PowerPC pseries based VMs do not support a floppy disk controller. Here the commit message mentions a controller, but the code only checks for the <disk type='floppy'/> device, not for <controller type='fdc'/>
Should we forbid that one too? AFAIK we've auto-added it to the XML only if there was at least floppy. We need not forbid fdc, as checking for floppy device will fulfill the need. Yes, fdc is added only if there was at least one floppy device.
Okay, I've squashed the test together with 2/3 and pushed the series. Jan

PowerPC pseries based VMs do not support a floppy disk controller. This test case fails if libvirt allows a floppy disk on pseries VMs. Signed-off-by: Kothapally Madhu Pavan <kmp@linux.vnet.ibm.com> --- .../qemuxml2argv-disk-floppy-pseries.args | 6 +++ .../qemuxml2argv-disk-floppy-pseries.xml | 41 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 3 files changed, 48 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.args new file mode 100644 index 0000000..6f68121 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.args @@ -0,0 +1,6 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-ppc64 -S -M pseries -m 214 -smp 1 -nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \ +file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ +file=/dev/fd0,if=floppy,unit=0 -drive file=/tmp/firmware.img,if=floppy,unit=1 \ +-net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.xml new file mode 100644 index 0000000..be0ede6 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.xml @@ -0,0 +1,41 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='block' device='floppy'> + <driver name='qemu' type='raw'/> + <source dev='/dev/fd0'/> + <target dev='fda' bus='fdc'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='file' device='floppy'> + <driver name='qemu' type='raw'/> + <source file='/tmp/firmware.img'/> + <target dev='fdb' bus='fdc'/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> + <controller type='usb' index='0'/> + <controller type='fdc' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index f9b30d9..afd2ef3 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -737,6 +737,7 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_VIRTIO_TX_ALG); DO_TEST("disk-cdrom-tray-no-device-cap", NONE); DO_TEST("disk-floppy", NONE); + DO_TEST_FAILURE("disk-floppy-pseries", QEMU_CAPS_DRIVE); DO_TEST("disk-floppy-tray-no-device-cap", NONE); DO_TEST("disk-floppy-tray", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE);
participants (3)
-
Ján Tomko
-
Kothapally Madhu Pavan
-
madhu pavan