[libvirt] [PATCH] qemu: check hostdev address type

[John Ferlan]

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)) {
>