[libvirt] [PATCH v3 07/12] conf: Introduce parser, formatter for uid and fid

2018-08-07 Thread Yi Min Zhao
This patch introduces new XML parser/formatter functions. Uid is
16-bit and non-zero. Fid is 32-bit. They are added as two new
attributes of PCI address, and parsed/formatted along with PCI
address parser/formatter. The related test is also added.

Signed-off-by: Yi Min Zhao 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Stefan Zimmermann 
Reviewed-by: Bjoern Walk 
Reviewed-by: Ján Tomko 
---
 docs/schemas/basictypes.rng| 23 +++
 docs/schemas/domaincommon.rng  |  1 +
 src/conf/device_conf.c | 78 ++
 src/conf/domain_addr.c |  3 +
 src/conf/domain_conf.c |  6 ++
 src/util/virpci.h  |  3 +
 tests/qemuxml2argvdata/disk-virtio-s390-zpci.args  | 26 
 tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml   | 17 +
 tests/qemuxml2argvdata/hostdev-vfio-zpci.args  | 24 +++
 tests/qemuxml2argvdata/hostdev-vfio-zpci.xml   | 19 ++
 tests/qemuxml2argvtest.c   |  4 ++
 tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml | 29 
 tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml | 30 +
 tests/qemuxml2xmltest.c|  3 +
 14 files changed, 266 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/disk-virtio-s390-zpci.args
 create mode 100644 tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci.args
 create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci.xml
 create mode 100644 tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml
 create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml

diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 1a18cd31b1..4e3365c211 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -65,6 +65,17 @@
   
 
   
+  
+
+  
+(0x)?[0-9a-fA-F]{1,8}
+  
+  
+0
+4294967295
+  
+
+  
 
   
 
@@ -111,6 +122,18 @@
   
 
   
+  
+
+  
+
+  
+
+
+  
+
+  
+
+  
 
   
   
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index ac04af51a1..e4d908f4ab 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5180,6 +5180,7 @@
 pci
   
   
+  
 
 
   
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index d69f94fadf..4d09b8219a 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -32,6 +32,79 @@
 
 #define VIR_FROM_THIS VIR_FROM_DEVICE
 
+static int
+virZPCIDeviceAddressIsValid(virZPCIDeviceAddressPtr zpci)
+{
+if (zpci->uid_assigned &&
+(zpci->zpci_uid > VIR_DOMAIN_DEVICE_ZPCI_MAX_UID ||
+ zpci->zpci_uid == 0)) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Invalid PCI address uid='0x%x', "
+ "must be > 0x0 and <= 0x%x"),
+   zpci->zpci_uid,
+   VIR_DOMAIN_DEVICE_ZPCI_MAX_UID);
+return 0;
+}
+
+if (zpci->fid_assigned) {
+/* We don't need to check fid because fid covers
+ * all range of uint32 type.
+ */
+return 1;
+}
+
+return 1;
+}
+
+static int
+virZPCIDeviceAddressParseXML(xmlNodePtr node,
+ virPCIDeviceAddressPtr addr)
+{
+char *uid, *fid;
+int ret = -1;
+virZPCIDeviceAddressPtr def = NULL;
+
+if (VIR_ALLOC(def) < 0)
+return -1;
+
+uid = virXMLPropString(node, "uid");
+fid = virXMLPropString(node, "fid");
+
+if (uid) {
+if (virStrToLong_uip(uid, NULL, 0, &def->zpci_uid) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Cannot parse  'uid' attribute"));
+goto cleanup;
+}
+def->uid_assigned = true;
+}
+
+if (fid) {
+if (virStrToLong_uip(fid, NULL, 0, &def->zpci_fid) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Cannot parse  'fid' attribute"));
+goto cleanup;
+}
+def->fid_assigned = true;
+}
+
+if (uid || fid) {
+if (!virZPCIDeviceAddressIsValid(def))
+goto cleanup;
+
+addr->zpci = def;
+def = NULL;
+}
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(uid);
+VIR_FREE(fid);
+VIR_FREE(def);
+return ret;
+}
+
 int
 virDomainDeviceInfoCopy(virDomainDeviceInfoPtr dst,
 virDomainDeviceInfoPtr src)
@@ -57,6 +130,8 @@ void
 virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
 {
 VIR_FREE(info->alias);
+if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)
+VIR_FREE(info->addr.pci.zpci);
 memset(&info->addr, 0, sizeof(info->addr));
 info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
 VIR

Re: [libvirt] [PATCH v3 07/12] conf: Introduce parser, formatter for uid and fid

2018-08-16 Thread Andrea Bolognani
On Tue, 2018-08-07 at 17:10 +0800, Yi Min Zhao wrote:
[...]
> +static int
> +virZPCIDeviceAddressIsValid(virZPCIDeviceAddressPtr zpci)
> +{
> +if (zpci->uid_assigned &&
> +(zpci->zpci_uid > VIR_DOMAIN_DEVICE_ZPCI_MAX_UID ||
> + zpci->zpci_uid == 0)) {
> +virReportError(VIR_ERR_XML_ERROR,
> +   _("Invalid PCI address uid='0x%x', "
> + "must be > 0x0 and <= 0x%x"),
> +   zpci->zpci_uid,
> +   VIR_DOMAIN_DEVICE_ZPCI_MAX_UID);
> +return 0;
> +}
> +
> +if (zpci->fid_assigned) {
> +/* We don't need to check fid because fid covers
> + * all range of uint32 type.
> + */
> +return 1;
> +}

This branch is pointless, just drop it (but leave the comment).

[...]
> @@ -37,6 +37,9 @@ typedef virPCIDeviceAddress *virPCIDeviceAddressPtr;
>  typedef struct _virPCIDeviceList virPCIDeviceList;
>  typedef virPCIDeviceList *virPCIDeviceListPtr;
>  
> +# define VIR_DOMAIN_DEVICE_ZPCI_MAX_UID UINT16_MAX
> +# define VIR_DOMAIN_DEVICE_ZPCI_MAX_FID UINT32_MAX

A single space between the name and the value will do.


This should be

  DO_TEST("disk-virtio-s390-zpci",
  QEMU_CAPS_DEVICE_ZPCI,
  QEMU_CAPS_CCW,
  QEMU_CAPS_VIRTIO_S390);

Same later.


The rest looks good.

-- 
Andrea Bolognani / Red Hat / Virtualization

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


Re: [libvirt] [PATCH v3 07/12] conf: Introduce parser, formatter for uid and fid

2018-08-20 Thread Yi Min Zhao



在 2018/8/16 下午11:14, Andrea Bolognani 写道:

On Tue, 2018-08-07 at 17:10 +0800, Yi Min Zhao wrote:
[...]

+static int
+virZPCIDeviceAddressIsValid(virZPCIDeviceAddressPtr zpci)
+{
+if (zpci->uid_assigned &&
+(zpci->zpci_uid > VIR_DOMAIN_DEVICE_ZPCI_MAX_UID ||
+ zpci->zpci_uid == 0)) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Invalid PCI address uid='0x%x', "
+ "must be > 0x0 and <= 0x%x"),
+   zpci->zpci_uid,
+   VIR_DOMAIN_DEVICE_ZPCI_MAX_UID);
+return 0;
+}
+
+if (zpci->fid_assigned) {
+/* We don't need to check fid because fid covers
+ * all range of uint32 type.
+ */
+return 1;
+}

This branch is pointless, just drop it (but leave the comment).

[...]

@@ -37,6 +37,9 @@ typedef virPCIDeviceAddress *virPCIDeviceAddressPtr;
  typedef struct _virPCIDeviceList virPCIDeviceList;
  typedef virPCIDeviceList *virPCIDeviceListPtr;
  
+# define VIR_DOMAIN_DEVICE_ZPCI_MAX_UID UINT16_MAX

+# define VIR_DOMAIN_DEVICE_ZPCI_MAX_FID UINT32_MAX

A single space between the name and the value will do.


This should be

   DO_TEST("disk-virtio-s390-zpci",
   QEMU_CAPS_DEVICE_ZPCI,
   QEMU_CAPS_CCW,
   QEMU_CAPS_VIRTIO_S390);

Same later.


The rest looks good.


OK.

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