Re: [PATCH v6 07/13] conf: Introduce parser, formatter for uid and fid

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 2018/10/11 下午7:45, Andrea Bolognani 写道:
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? :)
Yes, I just want to quickly know if this change is exact.

[...]
@@ -227,9 +288,22 @@ virPCIDeviceAddressParseXML(xmlNodePtr node,
          goto cleanup;
}
+
      if (!virPCIDeviceAddressIsEmpty(addr) && !virPCIDeviceAddressIsValid(addr, true))
          goto cleanup;
Spurious whitespace change.
Oh...good catch.

[...]
@@ -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.
Sure.

[...]
@@ -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*().
It has been moved to util/virpci. Isn't it?

[...]
@@ -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().
Sure.


Everything else looks good.


--
Yi Min

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux