3:29 a.m.
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.
---
Kothapally Madhu Pavan (2):
Caps: Disable floppy disk for PowerPC Vm
Avoid starting a PowerPC VM with floppy disk
src/conf/domain_conf.c | 19 +++++++++++++----
src/qemu/qemu_capabilities.c | 15 ++++++++++---
src/qemu/qemu_command.c | 47 ++++++++++++++++++++++++++++++++++++++++--
3 files changed, 71 insertions(+), 10 deletions(-)
--
3:30 a.m.
New subject: [libvirt] [PATCH 1/2] Caps: Disable floppy disk for PowerPC VM
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(a)linux.vnet.ibm.com>
---
src/conf/domain_conf.c | 19 +++++++++++++++----
src/qemu/qemu_capabilities.c | 15 +++++++++++----
2 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 73ac537..b9f35b4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6540,7 +6540,9 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
virHashTablePtr bootHash,
virSecurityLabelDefPtr* vmSeclabels,
int nvmSeclabels,
- unsigned int flags)
+ unsigned int flags,
+ virArch arch,
+ const char *machine)
{
virDomainDiskDefPtr def;
xmlNodePtr sourceNode = NULL;
@@ -7165,6 +7167,12 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
}
} else {
if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (ARCH_IS_PPC64(arch) && STRPREFIX(machine, "pseries"))
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("PowerPC pseries machines do not support floppy
device"));
+ goto error;
+ }
def->bus = VIR_DOMAIN_DISK_BUS_FDC;
} else if (!(flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) {
if (STRPREFIX(target, "hd"))
@@ -12375,7 +12383,8 @@ virDomainDeviceDefParse(const char *xmlStr,
if (!(dev->data.disk = virDomainDiskDefParseXML(xmlopt, node, ctxt,
NULL, def->seclabels,
def->nseclabels,
- flags)))
+ flags, def->os.arch,
+ def->os.machine)))
goto error;
break;
case VIR_DOMAIN_DEVICE_LEASE:
@@ -12519,7 +12528,8 @@ virDomainDiskDefSourceParse(const char *xmlStr,
if (!(disk = virDomainDiskDefParseXML(xmlopt, node, ctxt,
NULL, def->seclabels,
def->nseclabels,
- flags)))
+ flags, def->os.arch,
+ def->os.machine)))
goto cleanup;
ret = disk->src;
@@ -15539,7 +15549,8 @@ virDomainDefParseXML(xmlDocPtr xml,
bootHash,
def->seclabels,
def->nseclabels,
- flags);
+ flags, def->os.arch,
+ def->os.machine);
if (!disk)
goto error;
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;
7:49 p.m.
New subject: [libvirt] [PATCH 1/2] Caps: Disable floppy disk for PowerPC VM
On Fri, Jul 24, 2015 at 03:30:14PM -0400, Kothapally Madhu Pavan wrote:
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(a)linux.vnet.ibm.com>
---
src/conf/domain_conf.c | 19 +++++++++++++++----
src/qemu/qemu_capabilities.c | 15 +++++++++++----
2 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 73ac537..b9f35b4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6540,7 +6540,9 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
virHashTablePtr bootHash,
virSecurityLabelDefPtr* vmSeclabels,
int nvmSeclabels,
- unsigned int flags)
+ unsigned int flags,
+ virArch arch,
+ const char *machine)
{
virDomainDiskDefPtr def;
xmlNodePtr sourceNode = NULL;
@@ -7165,6 +7167,12 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
}
} else {
if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (ARCH_IS_PPC64(arch) && STRPREFIX(machine, "pseries"))
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("PowerPC pseries machines do not support floppy
device"));
+ goto error;
+ }
def->bus = VIR_DOMAIN_DISK_BUS_FDC;
} else if (!(flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) {
if (STRPREFIX(target, "hd"))
@@ -12375,7 +12383,8 @@ virDomainDeviceDefParse(const char *xmlStr,
if (!(dev->data.disk = virDomainDiskDefParseXML(xmlopt, node, ctxt,
NULL, def->seclabels,
def->nseclabels,
- flags)))
+ flags, def->os.arch,
+ def->os.machine)))
The architecture and machine type do not affect parsing of the disk.
If we really want to forbid even parsing a domain with a floppy,
this would better be done somewhere in the virDomainDefPostParse
functions.
But I think we should happily parse this domain and only report an error
on startup, so all the domain_conf.c changes can be dropped.
goto error;
break;
case VIR_DOMAIN_DEVICE_LEASE:
@@ -12519,7 +12528,8 @@ virDomainDiskDefSourceParse(const char *xmlStr,
if (!(disk = virDomainDiskDefParseXML(xmlopt, node, ctxt,
NULL, def->seclabels,
def->nseclabels,
- flags)))
+ flags, def->os.arch,
+ def->os.machine)))
goto cleanup;
ret = disk->src;
@@ -15539,7 +15549,8 @@ virDomainDefParseXML(xmlDocPtr xml,
bootHash,
def->seclabels,
def->nseclabels,
- flags);
+ flags, def->os.arch,
+ def->os.machine);
if (!disk)
goto error;
The rest looks good.
Jan
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;
3:30 a.m.
New subject: [libvirt] [PATCH 2/2] Avoid starting a PowerPC VM with floppy disk
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(a)linux.vnet.ibm.com>
---
src/qemu/qemu_command.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 42906a8..93f84e2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9486,6 +9486,12 @@ qemuBuildCommandLine(virConnectPtr conn,
boot[i] = 'd';
break;
case VIR_DOMAIN_BOOT_FLOPPY:
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (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;
+ }
boot[i] = 'a';
break;
case VIR_DOMAIN_BOOT_DISK:
@@ -9769,6 +9775,12 @@ qemuBuildCommandLine(virConnectPtr conn,
bootCD = 0;
break;
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (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;
+ }
bootindex = bootFloppy;
bootFloppy = 0;
break;
@@ -9812,6 +9824,12 @@ qemuBuildCommandLine(virConnectPtr conn,
if (withDeviceArg) {
if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) {
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (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;
+ }
if (virAsprintf(&optstr, "drive%c=drive-%s",
disk->info.addr.drive.unit ? 'B' :
'A',
disk->info.alias) < 0)
@@ -9854,6 +9872,12 @@ qemuBuildCommandLine(virConnectPtr conn,
/* Newer Q35 machine types require an explicit FDC controller */
virBufferTrim(&fdc_opts, ",", -1);
if ((fdc_opts_str = virBufferContentAndReset(&fdc_opts))) {
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (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;
+ }
virCommandAddArg(cmd, "-device");
virCommandAddArgFormat(cmd, "isa-fdc,%s", fdc_opts_str);
VIR_FREE(fdc_opts_str);
@@ -9918,10 +9942,17 @@ qemuBuildCommandLine(virConnectPtr conn,
_("cannot create virtual FAT disks in read-write
mode"));
goto error;
}
- if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
+ if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (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;
+ }
fmt = "fat:floppy:%s";
- else
+ } else {
fmt = "fat:%s";
+ }
if (virAsprintf(&file, fmt, disk->src) < 0)
goto error;
@@ -11674,6 +11705,12 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
def->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
def->src->readonly = true;
} else if (STREQ(values[i], "floppy")) {
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (ARCH_IS_PPC64(dom->os.arch) &&
STRPREFIX(dom->os.machine, "pseries")) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("PowerPC pseries machines do not support floppy
device"));
+ goto error;
+ }
def->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY;
}
} else if (STREQ(keywords[i], "format")) {
@@ -12908,6 +12945,12 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
disk->src->readonly = true;
} else {
if (STRPREFIX(arg, "-fd")) {
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (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;
+ }
disk->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY;
disk->bus = VIR_DOMAIN_DISK_BUS_FDC;
} else {
8:02 p.m.
New subject: [libvirt] [PATCH 2/2] Avoid starting a PowerPC VM with floppy disk
On Fri, Jul 24, 2015 at 03:30:49PM -0400, Kothapally Madhu Pavan wrote:
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(a)linux.vnet.ibm.com>
---
src/qemu/qemu_command.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
Extending the qemuxml2argvtest with a test case that fails on such
config would be nice.
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 42906a8..93f84e2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9486,6 +9486,12 @@ qemuBuildCommandLine(virConnectPtr conn,
boot[i] = 'd';
break;
case VIR_DOMAIN_BOOT_FLOPPY:
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (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;
+ }
There is no need to error out earlier for machines that do not use
deviceboot. This error can be dropped, we will hit the next one.
boot[i] = 'a';
break;
case VIR_DOMAIN_BOOT_DISK:
@@ -9769,6 +9775,12 @@ qemuBuildCommandLine(virConnectPtr conn,
bootCD = 0;
break;
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (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;
+ }
This is more appropriate place for the error message. Personally, I would move it
above the switch() and let the switch only deal with boot indexes.
bootindex = bootFloppy;
bootFloppy = 0;
break;
@@ -9812,6 +9824,12 @@ qemuBuildCommandLine(virConnectPtr conn,
if (withDeviceArg) {
if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) {
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (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;
+ }
This is dead code, the condition will never be true here, we already
matched the error above.
if (virAsprintf(&optstr,
"drive%c=drive-%s",
disk->info.addr.drive.unit ? 'B' :
'A',
disk->info.alias) < 0)
@@ -9854,6 +9872,12 @@ qemuBuildCommandLine(virConnectPtr conn,
/* Newer Q35 machine types require an explicit FDC controller */
virBufferTrim(&fdc_opts, ",", -1);
if ((fdc_opts_str = virBufferContentAndReset(&fdc_opts))) {
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (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;
+ }
Same here.
virCommandAddArg(cmd, "-device");
virCommandAddArgFormat(cmd, "isa-fdc,%s", fdc_opts_str);
VIR_FREE(fdc_opts_str);
@@ -9918,10 +9942,17 @@ qemuBuildCommandLine(virConnectPtr conn,
_("cannot create virtual FAT disks in read-write
mode"));
goto error;
}
- if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
+ if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (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;
+ }
fmt = "fat:floppy:%s";
- else
+ } else {
fmt = "fat:%s";
+ }
This code path can only be taken for QEMUs not having QEMU_CAPS_DRIVE,
i.e. older than at least 7 years. I do not think touching this part of
code is worth it.
if (virAsprintf(&file, fmt, disk->src) <
0)
goto error;
@@ -11674,6 +11705,12 @@
qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
def->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
def->src->readonly = true;
} else if (STREQ(values[i], "floppy")) {
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (ARCH_IS_PPC64(dom->os.arch) &&
STRPREFIX(dom->os.machine, "pseries")) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("PowerPC pseries machines do not support
floppy device"));
+ goto error;
+ }
This function is used to parse the command line of a running QEMU. If
QEMU rejects the floppy drive on the command line, such a machine would
not be running. If it quietly ignores it, we should do that too to get a
matching config.
def->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY;
}
} else if (STREQ(keywords[i], "format")) {
@@ -12908,6 +12945,12 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
disk->src->readonly = true;
} else {
if (STRPREFIX(arg, "-fd")) {
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (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;
+ }
Same here.
Jan
7:38 p.m.
New subject: [libvirt] [PATCH 2/2] Avoid starting a PowerPC VM with floppy disk
On 07/28/2015 05:32 PM, Ján Tomko wrote:
On Fri, Jul 24, 2015 at 03:30:49PM -0400, Kothapally Madhu Pavan
wrote:
> 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(a)linux.vnet.ibm.com>
> ---
> src/qemu/qemu_command.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
Extending the qemuxml2argvtest with a test case that fails on such
config would be nice.
> 1 file changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 42906a8..93f84e2 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9486,6 +9486,12 @@ qemuBuildCommandLine(virConnectPtr conn,
> boot[i] = 'd';
> break;
> case VIR_DOMAIN_BOOT_FLOPPY:
> + /* PowerPC pseries based VMs do not support floppy device */
> + if (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;
> + }
There is no need to error out earlier for machines that do not use
deviceboot. This error can be dropped, we will hit the next one.
> boot[i] = 'a';
> break;
> case VIR_DOMAIN_BOOT_DISK:
> @@ -9769,6 +9775,12 @@ qemuBuildCommandLine(virConnectPtr conn,
> bootCD = 0;
> break;
> case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
> + /* PowerPC pseries based VMs do not support floppy device */
> + if (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;
> + }
This is more appropriate place for the error message. Personally, I would move it
above the switch() and let the switch only deal with boot indexes.
> bootindex = bootFloppy;
> bootFloppy = 0;
> break;
> @@ -9812,6 +9824,12 @@ qemuBuildCommandLine(virConnectPtr conn,
>
> if (withDeviceArg) {
> if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) {
> + /* PowerPC pseries based VMs do not support floppy device */
> + if (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;
> + }
This is dead code, the condition will never be true here, we already
matched the error above.
> if (virAsprintf(&optstr, "drive%c=drive-%s",
> disk->info.addr.drive.unit ? 'B' :
'A',
> disk->info.alias) < 0)
> @@ -9854,6 +9872,12 @@ qemuBuildCommandLine(virConnectPtr conn,
> /* Newer Q35 machine types require an explicit FDC controller */
> virBufferTrim(&fdc_opts, ",", -1);
> if ((fdc_opts_str = virBufferContentAndReset(&fdc_opts))) {
> + /* PowerPC pseries based VMs do not support floppy device */
> + if (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;
> + }
Same here.
> virCommandAddArg(cmd, "-device");
> virCommandAddArgFormat(cmd, "isa-fdc,%s", fdc_opts_str);
> VIR_FREE(fdc_opts_str);
> @@ -9918,10 +9942,17 @@ qemuBuildCommandLine(virConnectPtr conn,
> _("cannot create virtual FAT disks in
read-write mode"));
> goto error;
> }
> - if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
> + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
> + /* PowerPC pseries based VMs do not support floppy device */
> + if (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;
> + }
> fmt = "fat:floppy:%s";
> - else
> + } else {
> fmt = "fat:%s";
> + }
>
This code path can only be taken for QEMUs not having QEMU_CAPS_DRIVE,
i.e. older than at least 7 years. I do not think touching this part of
code is worth it.
> if (virAsprintf(&file, fmt, disk->src) < 0)
> goto error;
> @@ -11674,6 +11705,12 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
> def->device = VIR_DOMAIN_DISK_DEVICE_CDROM;
> def->src->readonly = true;
> } else if (STREQ(values[i], "floppy")) {
> + /* PowerPC pseries based VMs do not support floppy device */
> + if (ARCH_IS_PPC64(dom->os.arch) &&
STRPREFIX(dom->os.machine, "pseries")) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("PowerPC pseries machines do not support
floppy device"));
> + goto error;
> + }
This function is used to parse the command line of a running QEMU. If
QEMU rejects the floppy drive on the command line, such a machine would
not be running. If it quietly ignores it, we should do that too to get a
matching config.
> def->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY;
> }
> } else if (STREQ(keywords[i], "format")) {
> @@ -12908,6 +12945,12 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
> disk->src->readonly = true;
> } else {
> if (STRPREFIX(arg, "-fd")) {
> + /* PowerPC pseries based VMs do not support floppy device */
> + if (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;
> + }
Same here.
Thanks for the response Jan. I have made necessary changes as per your
comments. Along with the changes I have added a test case.
Thanks,
Madhu Pavan.
8:55 p.m.
On Fri, 2015-07-24 at 15:29 -0400, Kothapally Madhu Pavan wrote:
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.
Hi,
sorry for not replying right away.
I started looking into that bug a while ago but I got sidetracked
shortly afterwards, so I don't have much to show for it. I'd like
to share my opinion on the matter anyway.
I believe you're basically following the right approach, eg. avoid
setting floppy-related capabilities and erroring out afterwards if
attempts are made to use devices that require such capabilites.
However, I think the implementation should be a little more
generic: ideally, the code would contain no references to the
ppc64 architecture and whether or not floppy disks are be allowed
would be determined by probing the QEMU binary.
Writing the code this way would allow us to handle automatically
other architectures where floppy disks do not make sense[1] and
situations where floppy support is not available[2].
If that turns out not to be possible or practical, at the very
least the check should be performed in one single spot, and not
replicated a dozen times thorough the library.
Cheers.
[1] I expect ARM to be one such architecture
[2] Such as stripped down / hardened QEMU binaries
--
Andrea Bolognani
Software Engineer - Virtualization Team
7:41 p.m.
On 07/29/2015 06:25 PM, Andrea Bolognani wrote:
On Fri, 2015-07-24 at 15:29 -0400, Kothapally Madhu Pavan wrote:
> 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.
Hi,
sorry for not replying right away.
I started looking into that bug a while ago but I got sidetracked
shortly afterwards, so I don't have much to show for it. I'd like
to share my opinion on the matter anyway.
I believe you're basically following the right approach, eg. avoid
setting floppy-related capabilities and erroring out afterwards if
attempts are made to use devices that require such capabilites.
However, I think the implementation should be a little more
generic: ideally, the code would contain no references to the
ppc64 architecture and whether or not floppy disks are be allowed
would be determined by probing the QEMU binary.
Writing the code this way would allow us to handle automatically
other architectures where floppy disks do not make sense[1] and
situations where floppy support is not available[2].
If that turns out not to be possible or practical, at the very
least the check should be performed in one single spot, and not
replicated a dozen times thorough the library.So,
As, present method of
capabilities parsing uses "qemu -h", and the list
received in this case is arch agnostic. So, we cannot do arch-specific
caps parsing unless qemu undergoes a rewrite. I will be providing a new
patch set with as minimal changes as possible.
Thanks,
Madhu Pavan.
3369
days inactive
3375
days old
7 comments
4 participants
participants (4)
-
Andrea Bolognani -
Ján Tomko -
Kothapally Madhu Pavan -
madhu pavan