Re: [PATCH libvirt v3 01/11] nodedev: detect AP card device

On Tue, Nov 17, 2020 at 01:10:55PM +0100, Shalini Chellathurai Saroja wrote: > Introduce support for the Adjunct Processor (AP) crypto card device. > Udev already detects the device, so add support for libvirt nodedev > driver. > > Signed-off-by: Farhan Ali <alifm(a)linux.ibm.com&gt; > Signed-off-by: Shalini Chellathurai Saroja <shalini(a)linux.ibm.com&gt; > Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com&gt; > Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com&gt; > --- > docs/formatnode.html.in | 8 ++++++ > docs/schemas/nodedev.rng | 10 ++++++++ > src/conf/node_device_conf.c | 41 ++++++++++++++++++++++++++++++ > src/conf/node_device_conf.h | 8 ++++++ > src/conf/virnodedeviceobj.c | 1 + > src/node_device/node_device_udev.c | 29 +++++++++++++++++++++ > tools/virsh-nodedev.c | 1 + > 7 files changed, 98 insertions(+) > ... > +static int > +virNodeDevCapAPCardParseXML(xmlXPathContextPtr ctxt, > + virNodeDeviceDefPtr def, > + xmlNodePtr node, > + virNodeDevCapAPCardPtr ap_card) > +{ > + xmlNodePtr orig; > + g_autofree char *adapter = NULL; > + > + orig = ctxt->node; > + ctxt->node = node; > + > + if (!(adapter = virXPathString("string(./ap-adapter[1])", ctxt))) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("missing ap-adapter value for '%s'"), def->name); > + return -1; > + } > + > + if (virStrToLong_uip(adapter, NULL, 0, &ap_card->ap_adapter) < 0) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("invalid ap-adapter value '%s' for '%s'"), > + adapter, def->name); > + return -1; > + } > + > + ctxt->node = orig; ^this restore will not happen if any of the above fails. I'd suggest using the VIR_XPATH_NODE_AUTORESTORE(ctxt) macro which already adopts our latest g_auto* standard. In context of patch 3, I'd also suggest to extract the adapter parsing logic into another static helper function e.g. virNodeDevAPAdapterParseXML which could be reused later from virNodeDevCapAPQueueParseXML to avoid duplication. Erik