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@xxxxxxxxxxxxx>
> ---
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
