Currently, there are two issues with handling the ZPCI address extension. Firstly, when the uid is to be auto-generated with a specified fid, .i.e.: ... <address type='pci'> <zpci fid='0x0000001f'/> </address> ... we expect uid='0x0001' (or the next available uid for the domain). However, we get a parsing error: $ virsh define zpci.xml error: XML error: Invalid PCI address uid='0x0000', must be > 0x0000 and <= 0xffff Secondly, when the uid is specified explicitly with the invalid numerical value '0x0000', we actually expect the parsing error above. However, the domain is being defined and the uid value is silently changed to a valid value. The first issue is a bug and the second one is undesired behaviour, and both issues are related to how we (in-band) signal invalid values for uid and fid. So let's fix the XML parsing to do validation based on what is actually specified in the XML and in later code assume that invalid numerical values mean that they have been unspecified. Signed-off-by: Bjoern Walk <bwalk@xxxxxxxxxxxxx> Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx> Signed-off-by: Shalini Chellathurai Saroja <shalini@xxxxxxxxxxxxx> Reviewed-by: Bjoern Walk <bwalk@xxxxxxxxxxxxx> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx> --- src/conf/device_conf.c | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 4dbd5c1a..001ed929 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -53,38 +53,34 @@ virZPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddressPtr addr) { virZPCIDeviceAddress def = { 0 }; - char *uid; - char *fid; - int ret = -1; + g_autofree char *uid = NULL; + g_autofree char *fid = NULL; uid = virXMLPropString(node, "uid"); fid = virXMLPropString(node, "fid"); - if (uid && - virStrToLong_uip(uid, NULL, 0, &def.uid) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'uid' attribute")); - goto cleanup; + if (uid) { + if (virStrToLong_uip(uid, NULL, 0, &def.uid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'uid' attribute")); + return -1; + } + if (!virZPCIDeviceAddressIsValid(&def)) + return -1; } - if (fid && - virStrToLong_uip(fid, NULL, 0, &def.fid) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot parse <address> 'fid' attribute")); - goto cleanup; + if (fid) { + if (virStrToLong_uip(fid, NULL, 0, &def.fid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'fid' attribute")); + return -1; + } } - if (!virZPCIDeviceAddressIsEmpty(&def) && - !virZPCIDeviceAddressIsValid(&def)) - goto cleanup; addr->zpci = def; - ret = 0; - cleanup: - VIR_FREE(uid); - VIR_FREE(fid); - return ret; + return 0; } void -- 2.21.1