Re: [libvirt] [PATCH v6 07/13] conf: Introduce parser, formatter for uid and fid
On Fri, 2018-09-28 at 16:46 +0800, Yi Min Zhao wrote:
This patch introduces new XML parser/formatter functions. Uid is
16-bit and non-zero. Fid is 32-bit. They are the two attributes of zpci
which is introduced as PCI address element. Zpci element is parsed and
formatted along with PCI address. And add the related test cases.
Signed-off-by: Yi Min Zhao <zyimin(a)linux.ibm.com>
---
No internal reviews this time around? :)
[...]
@@ -227,9 +288,22 @@ virPCIDeviceAddressParseXML(xmlNodePtr node,
goto cleanup;
}
+
if (!virPCIDeviceAddressIsEmpty(addr) && !virPCIDeviceAddressIsValid(addr,
true))
goto cleanup;
Spurious whitespace change.
[...]
@@ -6528,7 +6528,19 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
break;
}
- virBufferAddLit(buf, "/>\n");
+ if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
+ !virZPCIDeviceAddressIsEmpty(&info->addr.pci.zpci)) {
+ virBufferAddLit(buf, ">\n");
+ virBufferAdjustIndent(buf, 2);
+ virBufferAsprintf(buf,
+ "<zpci uid='0x%.4x'
fid='0x%.8x'/>\n",
+ info->addr.pci.zpci.uid,
+ info->addr.pci.zpci.fid);
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</address>\n");
+ } else {
+ virBufferAddLit(buf, "/>\n");
+ }
This looks like a perfect candidate for virXMLFormatElement(). You
should probably convert the original code to use that function in a
separate commit, then start actually using a child buffer here.
[...]
@@ -2566,6 +2566,7 @@ virPCIHeaderTypeToString;
virPCIIsVirtualFunction;
virPCIStubDriverTypeFromString;
virPCIStubDriverTypeToString;
+virZPCIDeviceAddressIsEmpty;
You can move virZPCIDeviceAddressIsValid() to util/virpci and
export it too, to be consistent with virPCIDeviceAddress*().
[...]
@@ -272,4 +275,6 @@ VIR_DEFINE_AUTOPTR_FUNC(virPCIDevice,
virPCIDeviceFree)
VIR_DEFINE_AUTOPTR_FUNC(virPCIDeviceAddress, virPCIDeviceAddressFree)
VIR_DEFINE_AUTOPTR_FUNC(virPCIEDeviceInfo, virPCIEDeviceInfoFree)
+bool virZPCIDeviceAddressIsEmpty(const virZPCIDeviceAddress *addr);
+
Please place this further up in the file, eg. after
virPCIDeviceAddressParse().
Everything else looks good.
--
Andrea Bolognani / Red Hat / Virtualization