On 06/17/2015 12:00 PM, Ján Tomko wrote: > For USB and SCSI hostdevs, we passed the invalid address to QEMU. > Report an error earlier. > > PCI hostdevs check the address type when parsing the XML. > > https://bugzilla.redhat.com/show_bug.cgi?id=1225339 > --- > src/qemu/qemu_command.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > So does this more or less obviate the need for my change to domain_conf.c here: http://www.redhat.com/archives/libvir-list/2015-June/msg01105.html I know it's always a "decision" whether to fail at parse or run time and we usually defer to run time; however, in this case it seems more reasonable to check at parse instead of run since it's a bogus XML address provided. Understand that the 'scsi' domain could disappear in the bz example provided, but would that device even be usable anyway? I suppose for the pci type it's "fortunate" that virDomainDeviceDriveAddressPtr and virDevicePCIAddressPtr are "similar" enough to within their union to not be cause an issue. I'm not opposed to this change as it's technically correct, just want to understand why not check sooner. John > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 3886b4f..a4853ab 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -10572,7 +10572,12 @@ qemuBuildCommandLine(virConnectPtr conn, > /* USB */ > if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && > hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { > - > + if (hostdev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && > + hostdev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("USB host devices must use 'usb' address type")); > + goto error; > + } > if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { > virCommandAddArg(cmd, "-device"); > if (!(devstr = qemuBuildUSBHostdevDevStr(def, hostdev, qemuCaps))) > @@ -10644,6 +10649,12 @@ qemuBuildCommandLine(virConnectPtr conn, > /* SCSI */ > if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && > hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { > + if (hostdev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && > + hostdev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("SCSI host devices must use 'drive' address type")); > + goto error; > + } > if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE) && > virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && > virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) { > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list