Re: [PATCH 2/2] Avoid starting a PowerPC VM with floppy disk

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxx>
> ---
>  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

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]