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