On 25/06/16 18:11, Ján Tomko wrote:
> We would happily accept a usb address with a missing port attribute,
> then format it back as port='(null)', so the persistent domain
> would not be able to start (due to the XML sightseeing tour in
> virDomainObjSetDefTransient) or survive daemon restart.
> ---
> src/conf/domain_conf.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 79d15c8..d0684eb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5132,7 +5132,11 @@ virDomainDeviceUSBAddressParseXML(xmlNodePtr node,
>
> memset(addr, 0, sizeof(*addr));
>
> - port = virXMLPropString(node, "port");
> + if (!(port = virXMLPropString(node, "port"))) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("USB address must have a port"));
> + goto cleanup;
> + }
> bus = virXMLPropString(node, "bus");
>
> if (port && virDomainDeviceUSBAddressParsePort(port) < 0)
>
We talked about this on IRC in private, so you already know that this
approach will still permit port='0' as a valid port specification. The
USB address reservation will again result in an unknown error. As you
proposed on IRC, it would be possible to have some sort of validation
(analogical to PCI addresses) that would take place during address
reservation. But since reservation would currently be done only for
QEMU, other drivers would be still broken. So I'd rather see the port
attribute still as optional, error out when detecting port=0 and
skipping it completely (i.e. success) if the attribute is missing during
address reservation. Lastly, I find it a little weird that we format
back a missing port as an empty string, that we cannot even parse after
ourselves, rather that leaving it out completely.
Erik
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list