Re: [PATCH 6/7] qemu: assume -drive argument is always available

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

 




On 11/05/2015 12:33 PM, Daniel P. Berrange wrote:
> As of QEMU 0.9.1 the -drive argument can be used to configure
> all disks, so the QEMU driver can assume it is always available
> and drop support for -hda/-cdrom/etc.
> 
> Many of the tests need updating because a great many were
> running without CAPS_DRIVE set, so using the -hda legacy
> syntax.
> 
> Fixing the tests uncovered a bug in the argv -> xml
> convertor which failed to handle disk with if=floppy.
> 
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
>  src/qemu/qemu_capabilities.c                       |  45 ++-
>  src/qemu/qemu_capabilities.h                       |   2 +-
>  src/qemu/qemu_command.c                            | 365 ++++++++-------------
>  src/qemu/qemu_hotplug.c                            |   3 +-

[...]

format nits and a couple questions about changes.

> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 79d1692..7676237 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -1104,29 +1104,27 @@ virQEMUCapsComputeCmdFlags(const char *help,
>          virQEMUCapsSet(qemuCaps, QEMU_CAPS_XEN_DOMID);
>      else if (strstr(help, "-domid"))
>          virQEMUCapsSet(qemuCaps, QEMU_CAPS_DOMID);
> -    if (strstr(help, "-drive")) {
> -        const char *cache = strstr(help, "cache=");
> -
> -        virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE);
> -        if (cache && (p = strchr(cache, ']'))) {
> -            if (memmem(cache, p - cache, "on|off", sizeof("on|off") - 1) == NULL)
> -                virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_V2);
> -            if (memmem(cache, p - cache, "directsync", sizeof("directsync") - 1))
> -                virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC);
> +    const char *cache = strstr(help, "cache=");

Although it works for this compiler... Should this move up to the start
of the function?

> +
> +    if (cache && (p = strchr(cache, ']'))) {
> +        if (memmem(cache, p - cache, "on|off", sizeof("on|off") - 1) == NULL)
> +            virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_V2);
> +        if (memmem(cache, p - cache, "directsync", sizeof("directsync") - 1))
> +            virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC);
>              if (memmem(cache, p - cache, "unsafe", sizeof("unsafe") - 1))
>                  virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_UNSAFE);
> -        }
> -        if (strstr(help, "format="))
> -            virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT);
> -        if (strstr(help, "readonly="))
> -            virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_READONLY);
> -        if (strstr(help, "aio=threads|native"))
> -            virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_AIO);
> -        if (strstr(help, "copy-on-read=on|off"))
> -            virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_COPY_ON_READ);
> -        if (strstr(help, "bps="))
> -            virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE);
>      }
> +    if (strstr(help, "format="))
> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT);
> +    if (strstr(help, "readonly="))
> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_READONLY);
> +    if (strstr(help, "aio=threads|native"))
> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_AIO);
> +    if (strstr(help, "copy-on-read=on|off"))
> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_COPY_ON_READ);
> +    if (strstr(help, "bps="))
> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE);
> +
>      if ((p = strstr(help, "-vga")) && !strstr(help, "-std-vga")) {
>          const char *nl = strstr(p, "\n");
>  
> @@ -3214,7 +3212,6 @@ static qemuMonitorCallbacks callbacks = {
>  static void
>  virQEMUCapsInitQMPBasic(virQEMUCapsPtr qemuCaps)
>  {
> -    virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE);
>      virQEMUCapsSet(qemuCaps, QEMU_CAPS_NAME);
>      virQEMUCapsSet(qemuCaps, QEMU_CAPS_UUID);
>      virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNET_HDR);
> @@ -3938,8 +3935,7 @@ virQEMUCapsFillDomainLoaderCaps(virQEMUCapsPtr qemuCaps,
>      VIR_DOMAIN_CAPS_ENUM_SET(capsLoader->type,
>                               VIR_DOMAIN_LOADER_TYPE_ROM);
>  
> -    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE) &&
> -        virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT))
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT))
>          VIR_DOMAIN_CAPS_ENUM_SET(capsLoader->type,
>                                   VIR_DOMAIN_LOADER_TYPE_PFLASH);
>  
> @@ -4023,8 +4019,7 @@ virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps,
>      VIR_DOMAIN_CAPS_ENUM_SET(hostdev->subsysType,
>                               VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB,
>                               VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI);
> -    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE) &&
> -        virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) &&
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) &&
>          virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC))
>          VIR_DOMAIN_CAPS_ENUM_SET(hostdev->subsysType,
>                                   VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI);
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 2179162..ff66ca9 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -36,7 +36,7 @@ typedef enum {
>      X_QEMU_CAPS_KQEMU, /* Whether KQEMU is compiled in */
>      X_QEMU_CAPS_VNC_COLON, /* VNC takes or address + display */
>      X_QEMU_CAPS_NO_REBOOT, /* Is the -no-reboot flag available */
> -    QEMU_CAPS_DRIVE, /* Is the new -drive arg available */
> +    X_QEMU_CAPS_DRIVE, /* Is the new -drive arg available */
>      QEMU_CAPS_DRIVE_BOOT, /* Does -drive support boot=on */
>  
>      /* 5 */
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 77913b3..956345f 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -777,20 +777,6 @@ int qemuDomainNetVLAN(virDomainNetDefPtr def)
>  }
>  
>  
> -/* Names used before -drive existed */
> -static int qemuAssignDeviceDiskAliasLegacy(virDomainDiskDefPtr disk)
> -{
> -    char *dev_name;
> -
> -    if (VIR_STRDUP(dev_name,
> -                   disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM &&
> -                   STREQ(disk->dst, "hdc") ? "cdrom" : disk->dst) < 0)
> -        return -1;
> -    disk->info.alias = dev_name;
> -    return 0;
> -}
> -
> -
>  char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk,
>                                 virQEMUCapsPtr qemuCaps)
>  {
> @@ -845,6 +831,9 @@ static int qemuAssignDeviceDiskAliasFixed(virDomainDiskDefPtr disk)
>      case VIR_DOMAIN_DISK_BUS_SD:
>          ret = virAsprintf(&dev_name, "sd%d", devid);
>          break;
> +    case VIR_DOMAIN_DISK_BUS_USB:
> +        ret = virAsprintf(&dev_name, "usb%d", devid);
> +        break;

[1]
Is this an unrelated bug?  Or perhaps was support added after/with
QEMU_CAPS_DEVICE.  IOW: Why haven't we hit this before?

Reading further in the patch seems to have another clue, but getting
into this helper means !QEMU_CAPS_DEVICE

>      default:
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                         _("Unsupported disk name mapping for bus '%s'"),
> @@ -967,14 +956,10 @@ qemuAssignDeviceDiskAlias(virDomainDefPtr vmdef,
>                            virDomainDiskDefPtr def,
>                            virQEMUCapsPtr qemuCaps)
>  {
> -    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) {
> -        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))
> -            return qemuAssignDeviceDiskAliasCustom(vmdef, def, qemuCaps);
> -        else
> -            return qemuAssignDeviceDiskAliasFixed(def);
> -    } else {
> -        return qemuAssignDeviceDiskAliasLegacy(def);
> -    }
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))
> +        return qemuAssignDeviceDiskAliasCustom(vmdef, def, qemuCaps);
> +    else
> +        return qemuAssignDeviceDiskAliasFixed(def);
>  }
>  
>  
> @@ -9084,11 +9069,6 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd,
>          break;
>  
>      case VIR_DOMAIN_LOADER_TYPE_PFLASH:
> -        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) {
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                           _("this QEMU binary doesn't support -drive"));
> -            goto cleanup;
> -        }
>          if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT)) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                             _("this QEMU binary doesn't support passing "
> @@ -9267,6 +9247,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>      char *boot_order_str = NULL, *boot_opts_str = NULL;
>      virBuffer fdc_opts = VIR_BUFFER_INITIALIZER;
>      char *fdc_opts_str = NULL;
> +    int bootCD = 0, bootFloppy = 0, bootDisk = 0;
>  
>      VIR_DEBUG("conn=%p driver=%p def=%p mon=%p json=%d "
>                "qemuCaps=%p migrateFrom=%s migrateFD=%d "
> @@ -10081,111 +10062,122 @@ qemuBuildCommandLine(virConnectPtr conn,
>          VIR_FREE(optstr);
>      }
>  

ugh - git diff makes it hard to follow the indents !

> -    /* If QEMU supports -drive param instead of old -hda, -hdb, -cdrom .. */
> -    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) {
> -        int bootCD = 0, bootFloppy = 0, bootDisk = 0;
> -
> -        if ((virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_BOOT) || emitBootindex)) {
> -            /* bootDevs will get translated into either bootindex=N or boot=on
> -             * depending on what qemu supports */
> -            for (i = 0; i < def->os.nBootDevs; i++) {
> -                switch (def->os.bootDevs[i]) {
> -                case VIR_DOMAIN_BOOT_CDROM:
> -                    bootCD = i + 1;
> -                    break;
> -                case VIR_DOMAIN_BOOT_FLOPPY:
> -                    bootFloppy = i + 1;
> -                    break;
> -                case VIR_DOMAIN_BOOT_DISK:
> -                    bootDisk = i + 1;
> -                    break;
> -                }
> +    if ((virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_BOOT) || emitBootindex)) {
> +        /* bootDevs will get translated into either bootindex=N or boot=on
> +         * depending on what qemu supports */
> +        for (i = 0; i < def->os.nBootDevs; i++) {
> +            switch (def->os.bootDevs[i]) {
> +            case VIR_DOMAIN_BOOT_CDROM:
> +                bootCD = i + 1;
> +                break;
> +            case VIR_DOMAIN_BOOT_FLOPPY:
> +                bootFloppy = i + 1;
> +                break;
> +            case VIR_DOMAIN_BOOT_DISK:
> +                bootDisk = i + 1;
> +                break;
>              }
>          }
> +    }
>  
> -        for (i = 0; i < def->ndisks; i++) {
> -            char *optstr;
> -            int bootindex = 0;
> -            virDomainDiskDefPtr disk = def->disks[i];
> -            bool withDeviceArg = false;
> -            bool deviceFlagMasked = false;
> -
> -            /* Unless we have -device, then USB disks need special
> -               handling */
> -            if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) &&
> -                !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
> -                if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
> -                    virCommandAddArg(cmd, "-usbdevice");
> -                    virCommandAddArgFormat(cmd, "disk:%s", disk->src->path);
> -                } else {
> -                    virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                   _("unsupported usb disk type for '%s'"),
> -                                   disk->src->path);
> -                    goto error;
> -                }
> -                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"));
> +    for (i = 0; i < def->ndisks; i++) {
> +        char *optstr;
> +        int bootindex = 0;
> +        virDomainDiskDefPtr disk = def->disks[i];
> +        bool withDeviceArg = false;
> +        bool deviceFlagMasked = false;
> +
> +        /* Unless we have -device, then USB disks need special
> +           handling */

[1] From above when adding "usb%d" to qemuAssignDeviceDiskAliasFixed

> +        if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) &&
> +            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
> +            if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
> +                virCommandAddArg(cmd, "-usbdevice");
> +                virCommandAddArgFormat(cmd, "disk:%s", disk->src->path);
> +            } else {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("unsupported usb disk type for '%s'"),
> +                               disk->src->path);
>                  goto error;
>              }
> +            continue;
> +        }
>  
> -            switch (disk->device) {
> -            case VIR_DOMAIN_DISK_DEVICE_CDROM:
> -                bootindex = bootCD;
> -                bootCD = 0;
> -                break;
> -            case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
> -                bootindex = bootFloppy;
> -                bootFloppy = 0;
> -                break;
> -            case VIR_DOMAIN_DISK_DEVICE_DISK:
> -            case VIR_DOMAIN_DISK_DEVICE_LUN:
> -                bootindex = bootDisk;
> -                bootDisk = 0;
> -                break;
> +        /* 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;
> +            bootCD = 0;
> +            break;
> +        case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
> +            bootindex = bootFloppy;
> +            bootFloppy = 0;
> +            break;
> +        case VIR_DOMAIN_DISK_DEVICE_DISK:
> +        case VIR_DOMAIN_DISK_DEVICE_LUN:
> +            bootindex = bootDisk;
> +            bootDisk = 0;
> +            break;
> +        }
> +
> +        virCommandAddArg(cmd, "-drive");
> +
> +        /* Unfortunately it is not possible to use
> +           -device for floppies, xen PV, or SD
> +           devices. Fortunately, those don't need
> +           static PCI addresses, so we don't really
> +           care that we can't use -device */
> +        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
> +            if (disk->bus != VIR_DOMAIN_DISK_BUS_XEN &&
> +                disk->bus != VIR_DOMAIN_DISK_BUS_SD) {
> +                withDeviceArg = true;
> +            } else {
> +                virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE);
> +                deviceFlagMasked = true;
>              }
> +        }
> +        optstr = qemuBuildDriveStr(conn, disk,
> +                                   emitBootindex ? false : !!bootindex,
> +                                   qemuCaps);
> +        if (deviceFlagMasked)
> +            virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE);
> +        if (!optstr)
> +            goto error;
> +        virCommandAddArg(cmd, optstr);
> +        VIR_FREE(optstr);
>  
> -            virCommandAddArg(cmd, "-drive");
> +        if (!emitBootindex)
> +            bootindex = 0;
> +        else if (disk->info.bootIndex)
> +            bootindex = disk->info.bootIndex;
>  
> -            /* Unfortunately it is not possible to use
> -               -device for floppies, xen PV, or SD
> -               devices. Fortunately, those don't need
> -               static PCI addresses, so we don't really
> -               care that we can't use -device */
> -            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
> -                if (disk->bus != VIR_DOMAIN_DISK_BUS_XEN &&
> -                    disk->bus != VIR_DOMAIN_DISK_BUS_SD) {
> -                    withDeviceArg = true;
> +        if (withDeviceArg) {
> +            if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) {
> +                if (virAsprintf(&optstr, "drive%c=drive-%s",
> +                                disk->info.addr.drive.unit ? 'B' : 'A',
> +                                disk->info.alias) < 0)
> +                    goto error;
> +
> +                if (!qemuDomainMachineNeedsFDC(def)) {
> +                    virCommandAddArg(cmd, "-global");
> +                    virCommandAddArgFormat(cmd, "isa-fdc.%s", optstr);
>                  } else {
> -                    virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE);
> -                    deviceFlagMasked = true;
> +                    virBufferAsprintf(&fdc_opts, "%s,", optstr);
>                  }
> -            }
> -            optstr = qemuBuildDriveStr(conn, disk,
> -                                       emitBootindex ? false : !!bootindex,
> -                                       qemuCaps);
> -            if (deviceFlagMasked)
> -                virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE);
> -            if (!optstr)
> -                goto error;
> -            virCommandAddArg(cmd, optstr);
> -            VIR_FREE(optstr);
> -
> -            if (!emitBootindex)
> -                bootindex = 0;
> -            else if (disk->info.bootIndex)
> -                bootindex = disk->info.bootIndex;
> +                VIR_FREE(optstr);
>  
> -            if (withDeviceArg) {
> -                if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) {
> -                    if (virAsprintf(&optstr, "drive%c=drive-%s",
> -                                    disk->info.addr.drive.unit ? 'B' : 'A',
> -                                    disk->info.alias) < 0)
> +                if (bootindex) {
> +                    if (virAsprintf(&optstr, "bootindex%c=%d",
> +                                    disk->info.addr.drive.unit
> +                                    ? 'B' : 'A',
> +                                    bootindex) < 0)
>                          goto error;
>  
>                      if (!qemuDomainMachineNeedsFDC(def)) {
> @@ -10195,126 +10187,25 @@ qemuBuildCommandLine(virConnectPtr conn,
>                          virBufferAsprintf(&fdc_opts, "%s,", optstr);
>                      }
>                      VIR_FREE(optstr);
> -
> -                    if (bootindex) {
> -                        if (virAsprintf(&optstr, "bootindex%c=%d",
> -                                        disk->info.addr.drive.unit
> -                                        ? 'B' : 'A',
> -                                        bootindex) < 0)
> -                            goto error;
> -
> -                        if (!qemuDomainMachineNeedsFDC(def)) {
> -                            virCommandAddArg(cmd, "-global");
> -                            virCommandAddArgFormat(cmd, "isa-fdc.%s", optstr);
> -                        } else {
> -                            virBufferAsprintf(&fdc_opts, "%s,", optstr);
> -                        }
> -                        VIR_FREE(optstr);
> -                    }

While at first glance it felt like the following hunk was lost - it's
intermixed in the middle of the former "} else {" to the if -drive check...

> -                } else {
> -                    virCommandAddArg(cmd, "-device");
> -
> -                    if (!(optstr = qemuBuildDriveDevStr(def, disk, bootindex,
> -                                                        qemuCaps)))
> -                        goto error;
> -                    virCommandAddArg(cmd, optstr);
> -                    VIR_FREE(optstr);
> -                }
> -            }
> -        }
> -        /* Newer Q35 machine types require an explicit FDC controller */
> -        virBufferTrim(&fdc_opts, ",", -1);
> -        if ((fdc_opts_str = virBufferContentAndReset(&fdc_opts))) {
> -            virCommandAddArg(cmd, "-device");
> -            virCommandAddArgFormat(cmd, "isa-fdc,%s", fdc_opts_str);
> -            VIR_FREE(fdc_opts_str);
> -        }
> -    } else {
> -        for (i = 0; i < def->ndisks; i++) {
> -            char dev[NAME_MAX];
> -            char *file;
> -            const char *fmt;
> -            virDomainDiskDefPtr disk = def->disks[i];
> -
> -            if ((disk->src->type == VIR_STORAGE_TYPE_BLOCK) &&
> -                (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN)) {
> -                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                               _("tray status 'open' is invalid for "
> -                                 "block type disk"));
> -                goto error;
> -            }
> -
> -            if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
> -                if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
> -                    virCommandAddArg(cmd, "-usbdevice");
> -                    virCommandAddArgFormat(cmd, "disk:%s", disk->src->path);
> -                } else {
> -                    virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                   _("unsupported usb disk type for '%s'"),
> -                                   disk->src->path);
> -                    goto error;
> -                }
> -                continue;
> -            }
> -
> -            if (STREQ(disk->dst, "hdc") &&
> -                disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
> -                if (disk->src->path) {
> -                    snprintf(dev, NAME_MAX, "-%s", "cdrom");
> -                } else {
> -                    continue;
>                  }
>              } else {
> -                if (STRPREFIX(disk->dst, "hd") ||
> -                    STRPREFIX(disk->dst, "fd")) {
> -                    snprintf(dev, NAME_MAX, "-%s", disk->dst);
> -                } else {
> -                    virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                   _("unsupported disk type '%s'"), disk->dst);
> -                    goto error;
> -                }
> -            }
> -
> -            if (disk->src->type == VIR_STORAGE_TYPE_DIR) {
> -                /* QEMU only supports magic FAT format for now */
> -                if (disk->src->format > 0 &&
> -                    disk->src->format != VIR_STORAGE_FILE_FAT) {
> -                    virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                   _("unsupported disk driver type for '%s'"),
> -                                   virStorageFileFormatTypeToString(disk->src->format));
> -                    goto error;
> -                }
> -                if (!disk->src->readonly) {
> -                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                                   _("cannot create virtual FAT disks in read-write mode"));
> -                    goto error;
> -                }
> -                if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
> -                    fmt = "fat:floppy:%s";
> -                else
> -                    fmt = "fat:%s";
> +                virCommandAddArg(cmd, "-device");
>  
> -                if (virAsprintf(&file, fmt, disk->src) < 0)
> -                    goto error;
> -            } else if (disk->src->type == VIR_STORAGE_TYPE_NETWORK) {
> -                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                               _("network disks are only supported with -drive"));
> -                goto error;
> -            } else {
> -                if (VIR_STRDUP(file, disk->src->path) < 0)
> +                if (!(optstr = qemuBuildDriveDevStr(def, disk, bootindex,
> +                                                    qemuCaps)))
>                      goto error;
> +                virCommandAddArg(cmd, optstr);
> +                VIR_FREE(optstr);
>              }
> -
> -            /* Don't start with source if the tray is open for
> -             * CDROM and Floppy device.
> -             */
> -            if (!((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY ||
> -                   disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) &&
> -                  disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN))
> -                virCommandAddArgList(cmd, dev, file, NULL);
> -            VIR_FREE(file);
>          }
>      }
> +    /* Newer Q35 machine types require an explicit FDC controller */
> +    virBufferTrim(&fdc_opts, ",", -1);
> +    if ((fdc_opts_str = virBufferContentAndReset(&fdc_opts))) {
> +        virCommandAddArg(cmd, "-device");
> +        virCommandAddArgFormat(cmd, "isa-fdc,%s", fdc_opts_str);
> +        VIR_FREE(fdc_opts_str);
> +    }
>  
>      if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV)) {
>          for (i = 0; i < def->nfss; i++) {
> @@ -11167,8 +11058,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>          /* SCSI */
>          if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
>              hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
> -            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE) &&
> -                virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) &&
> +            if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) &&
>                  virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) {
>                  char *drvstr;
>  
> @@ -12033,6 +11923,9 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
>                  }
>              } else if (STREQ(values[i], "scsi")) {
>                  def->bus = VIR_DOMAIN_DISK_BUS_SCSI;
> +            } else if (STREQ(values[i], "floppy")) {
> +                def->bus = VIR_DOMAIN_DISK_BUS_FDC;
> +                def->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY;

Is this another lurking bug? or part of the "if=floppy" bug?  Is it
worth separating out just in case in needs to be applied elsewhere?

>              } else if (STREQ(values[i], "virtio")) {
>                  def->bus = VIR_DOMAIN_DISK_BUS_VIRTIO;
>              } else if (STREQ(values[i], "xen")) {
> @@ -12202,6 +12095,8 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
>          ignore_value(VIR_STRDUP(def->dst, "vda"));
>      } else if (def->bus == VIR_DOMAIN_DISK_BUS_XEN) {
>          ignore_value(VIR_STRDUP(def->dst, "xvda"));
> +    } else if (def->bus == VIR_DOMAIN_DISK_BUS_FDC) {
> +        ignore_value(VIR_STRDUP(def->dst, "fda"));

Similar comment

>      } else {
>          ignore_value(VIR_STRDUP(def->dst, "hda"));
>      }

--
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]