[libvirt] [PATCH 2/6] domain_conf: Add USB controler model "none"

[Osier Yang]

On 2012年07月10日 01:29, Peter Krempa wrote:
> Libvirt adds a USB controller to the guest even if the user does not
> specify any in the XML. This is due to back-compat reasons.

It's not for back-compat reasons as far as I known, 42043afcd adds
the implicit USB controller for virt-manager's use, and it causes
the regression bug of migration, as the old libvirt doesn't have
a default USB controller as a force. Jirka fixed it by commit
409b5f549.

>
> To allow disabling USB for a guest this patch adds a new USB controller
> type "none" that disables USB support for the guest.
> ---
>   docs/schemas/domaincommon.rng |    1 +
>   src/conf/domain_conf.c        |   55 ++++++++++++++++++++++++++++++++++++++++-
>   src/conf/domain_conf.h        |    1 +
>   src/qemu/qemu_command.c       |    3 +-
>   4 files changed, 58 insertions(+), 2 deletions(-)
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 3d205b0..937c2e8 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1202,6 +1202,7 @@
>               <value>vt82c686b-uhci</value>
>               <value>pci-ohci</value>
>               <value>nec-xhci</value>
> +<value>none</value>
>             </choice>
>           </attribute>
>         </optional>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 17b0e0e..9afae87 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -257,7 +257,8 @@ VIR_ENUM_IMPL(virDomainControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST,
>                 "ich9-uhci3",
>                 "vt82c686b-uhci",
>                 "pci-ohci",
> -              "nec-xhci")
> +              "nec-xhci",
> +              "none")
>
>   VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
>                 "mount",
> @@ -7910,6 +7911,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>       virBitmapPtr bootMap = NULL;
>       unsigned long bootMapSize = 0;
>       xmlNodePtr cur;
> +    bool usb_none = false;
> +    bool usb_other = false;
>
>       if (VIR_ALLOC(def)<  0) {
>           virReportOOMError();
> @@ -8635,6 +8638,27 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>           if (!controller)
>               goto error;
>
> +        /* sanitize handling of "none" usb controller */
> +        if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) {
> +            if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
> +                if (usb_other || usb_none) {
> +                    virDomainReportError(VIR_ERR_XML_DETAIL, "%s",
> +                                         _("Can't add another USB controller: "
> +                                           "USB is disabled for this domain"));
> +                    goto error;
> +                }
> +                usb_none = true;
> +            } else {
> +                if (usb_none) {
> +                    virDomainReportError(VIR_ERR_XML_DETAIL, "%s",
> +                                         _("Can't add another USB controller: "
> +                                           "USB is disabled for this domain"));
> +                    goto error;
> +                }
> +                usb_other = true;
> +            }
> +        }
> +
>           virDomainControllerInsertPreAlloced(def, controller);
>       }
>       VIR_FREE(nodes);
> @@ -8909,6 +8933,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>           if (!input)
>               goto error;
>
> +        /* Check if USB bus is required */
> +        if (input->bus == VIR_DOMAIN_INPUT_BUS_USB&&  usb_none) {
> +            virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                                 _("Can't add USB input device. "
> +                                   "USB bus is disabled"));
> +            goto error;
> +        }
>
>           /* With QEMU / KVM / Xen graphics, mouse + PS/2 is implicit
>            * with graphics, so don't store it.
> @@ -9036,6 +9067,14 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>           if (!hostdev)
>               goto error;
>
> +        if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB&&
> +            usb_none) {
> +            virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                                 _("Can't add USB device passthrough: "
> +                                   "USB is disabled in this host"));
> +            goto error;
> +        }
> +
>           def->hostdevs[def->nhostdevs++] = hostdev;
>       }
>       VIR_FREE(nodes);
> @@ -9105,6 +9144,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>           if (!hub)
>               goto error;
>
> +        if (hub->type == VIR_DOMAIN_HUB_TYPE_USB&&  usb_none) {
> +            virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                                 _("Can't add USB hub: "
> +                                   "USB is disabled for this domain"));
> +            goto error;
> +        }
> +
>           def->hubs[def->nhubs++] = hub;
>       }
>       VIR_FREE(nodes);
> @@ -9121,6 +9167,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>           if (!redirdev)
>               goto error;
>
> +        if (redirdev->bus == VIR_DOMAIN_REDIRDEV_BUS_USB&&  usb_none) {
> +             virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                                  _("Can't add USB redir device: "
> +                                    "USB is disabled for this domain"));
> +            goto error;
> +        }
> +
>           def->redirdevs[def->nredirdevs++] = redirdev;

This is not that nice from a user's point of view, one could see
a bunch of errors if he doesn't quite understand the configs. E.g
when he removes the USB input device, he may see error for host
USB device, when he removes the host USB device, he may see error
for USB hub...... The more device uses USB bus we introduce, the
more error.

Is it a hard requirement of specify the 'none' USB controller
model? if not, we may want to add the controller implicitly
only necessary, instead of add it as a force. We won't fall
into another regression, as Jirka's patch emits the USB controller
anyway.

Regards,
Osier