Re: [PATCH] qemu: check hostdev address type

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

 




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




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