[libvirt] [PATCH 2/2] Avoid starting a PowerPC VM with floppy disk
madhu pavan
kmp at linux.vnet.ibm.com
Thu Jul 30 11:38:07 UTC 2015
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 at 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.
More information about the libvir-list
mailing list