Re: [libvirt] [PATCH 1/2] qemu: Add capability flag for usb-storage

Without knowing the exact development history of qemu, I assumed it could be the case the we have '-device' but 'usb-storage' hadn't been implemented yet (so '-usbdevice' was still the way to go).

So, what if I drop the above chunk, and do the following instead: --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8046,18 +8046,26 @@ qemuBuildCommandLine(virConnectPtr conn, /* 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); + if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support '-device " + "usb-storage'")); + goto error; + }

} else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported usb disk type for '%s'"), - disk->src); - goto error; + if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { + virCommandAddArg(cmd, "-usbdevice"); + virCommandAddArgFormat(cmd, "disk:%s", disk->src);

Alternatively, I could do the following instead, which is more concise, but doesn't happen in the same if() and thus spread the capability checking over a larger code area: --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4311,13 +4311,6 @@ qemuBuildDriveDevStr(virDomainDefPtr def, goto error; break; case VIR_DOMAIN_DISK_BUS_USB: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support '-device " + "usb-storage'")); + goto error; + + }

virBufferAddLit(&opt, "usb-storage"); if (qemuBuildDeviceAddressStr(&opt, def, &disk->info, qemuCaps) < 0) Once I have an opinion (or further explanation why I'm still confused :)) I'll re-submit a new patchset with the preferred fix, rebased on the then-current master.