On 5/4/21 1:39 PM, Kristina Hanicova wrote: > Signed-off-by: Kristina Hanicova <khanicov@xxxxxxxxxx> > --- > src/conf/domain_conf.c | 130 +++++++++++++++++++---------------------- > 1 file changed, 60 insertions(+), 70 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index e073644810..5c5b4ad6d7 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -6709,14 +6709,16 @@ virDomainDeviceInfoParseXML(virDomainXMLOption *xmlopt, > > static int > virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, > - xmlXPathContextPtr ctxt G_GNUC_UNUSED, > + xmlXPathContextPtr ctxt, > virDomainHostdevDef *def) > { > bool got_product, got_vendor; > - xmlNodePtr cur; > virDomainHostdevSubsysUSB *usbsrc = &def->source.subsys.u.usb; > g_autofree char *startupPolicy = NULL; > g_autofree char *autoAddress = NULL; > + VIR_XPATH_NODE_AUTORESTORE(ctxt) > + > + ctxt->node = node; > > if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) { > def->startupPolicy = > @@ -6737,79 +6739,67 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, > got_product = false; > got_vendor = false; > > - cur = node->children; > - while (cur != NULL) { > - if (cur->type == XML_ELEMENT_NODE) { > - if (virXMLNodeNameEqual(cur, "vendor")) { > - g_autofree char *vendor = virXMLPropString(cur, "id"); > - > - if (vendor) { > - got_vendor = true; > - if (virStrToLong_ui(vendor, NULL, 0, &usbsrc->vendor) < 0) { > - virReportError(VIR_ERR_INTERNAL_ERROR, > - _("cannot parse vendor id %s"), vendor); > - return -1; > - } > - } else { > - virReportError(VIR_ERR_INTERNAL_ERROR, > - "%s", _("usb vendor needs id")); > - return -1; > - } > - } else if (virXMLNodeNameEqual(cur, "product")) { > - g_autofree char *product = virXMLPropString(cur, "id"); > - > - if (product) { > - got_product = true; > - if (virStrToLong_ui(product, NULL, 0, > - &usbsrc->product) < 0) { > - virReportError(VIR_ERR_INTERNAL_ERROR, > - _("cannot parse product %s"), > - product); > - return -1; > - } > - } else { > - virReportError(VIR_ERR_INTERNAL_ERROR, > - "%s", _("usb product needs id")); > - return -1; > - } > - } else if (virXMLNodeNameEqual(cur, "address")) { > - g_autofree char *bus = NULL; > - g_autofree char *device = NULL; > - > - bus = virXMLPropString(cur, "bus"); > - if (bus) { > - if (virStrToLong_ui(bus, NULL, 0, &usbsrc->bus) < 0) { > - virReportError(VIR_ERR_INTERNAL_ERROR, > - _("cannot parse bus %s"), bus); > - return -1; > - } > - } else { > - virReportError(VIR_ERR_INTERNAL_ERROR, > - "%s", _("usb address needs bus id")); > - return -1; > - } > + if (virXPathNode("./vendor", ctxt)) { > + g_autofree char *vendor = NULL; > > - device = virXMLPropString(cur, "device"); > - if (device) { > - if (virStrToLong_ui(device, NULL, 0, &usbsrc->device) < 0) { > - virReportError(VIR_ERR_INTERNAL_ERROR, > - _("cannot parse device %s"), > - device); > - return -1; > - } > - } else { > - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("usb address needs device id")); > - return -1; > - } > - } else { > + if ((vendor = virXPathString("string(./vendor/@id)", ctxt))) { > + got_vendor = true; > + if (virStrToLong_ui(vendor, NULL, 0, &usbsrc->vendor) < 0) { Since you are touching this strToint conversion anyway, you could have used virXMLPropUInt(). Nothing critical, just pointing that out. It also applies to the rest of patches. I'll fix them before merge. > virReportError(VIR_ERR_INTERNAL_ERROR, > - _("unknown usb source type '%s'"), > - cur->name); > + _("cannot parse vendor id %s"), vendor); > return -1; > } > + } else { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + "%s", _("usb vendor needs id")); > + return -1; > + } > + } > + > + if (virXPathNode("./product", ctxt)) { > + g_autofree char *product = NULL; > + > + if ((product = virXPathString("string(./product/@id)", ctxt))) { > + got_product = true; > + if (virStrToLong_ui(product, NULL, 0, &usbsrc->product) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("cannot parse product %s"), product); > + return -1; > + } > + } else { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + "%s", _("usb product needs id")); > + return -1; > + } > + } > + > + if (virXPathNode("./address", ctxt)) { > + g_autofree char *bus = NULL; > + g_autofree char *device = NULL; > + > + if ((bus = virXPathString("string(./address/@bus)", ctxt))) { > + if (virStrToLong_ui(bus, NULL, 0, &usbsrc->bus) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("cannot parse bus %s"), bus); > + return -1; > + } > + } else { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + "%s", _("usb address needs bus id")); > + return -1; > + } > + > + if ((device = virXPathString("string(./address/@device)", ctxt))) { > + if (virStrToLong_ui(device, NULL, 0, &usbsrc->device) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("cannot parse device %s"), device); > + return -1; > + } > + } else { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("usb address needs device id")); > + return -1; > } > - cur = cur->next; > } > > if (got_vendor && usbsrc->vendor == 0) { > Michal