[PATCH libvirt v3 03/11] nodedev: detect AP queues

Shalini Chellathurai Saroja posted 11 patches 4 years ago
There is a newer version of this series
[PATCH libvirt v3 03/11] nodedev: detect AP queues
Posted by Shalini Chellathurai Saroja 4 years ago
Each AP card device can support upto 256 AP queues.  AP queues are
also detected by udev, so add support for libvirt nodedev driver.

Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
---
 docs/formatnode.html.in            | 15 +++++++
 docs/schemas/nodedev.rng           | 25 +++++++++++
 src/conf/node_device_conf.c        | 66 ++++++++++++++++++++++++++++++
 src/conf/node_device_conf.h        |  9 ++++
 src/conf/virnodedeviceobj.c        |  1 +
 src/node_device/node_device_udev.c | 30 ++++++++++++++
 tools/virsh-nodedev.c              |  1 +
 7 files changed, 147 insertions(+)

diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index d10a79e3..e8c5be79 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -439,6 +439,21 @@
               <dd>AP Card identifier.</dd>
             </dl>
           </dd>
+          <dt><code>ap_queue</code></dt>
+          <dd>Describes the AP queue on a s390 host. An AP queue is
+              an AP domain on an AP adapter which is specified by an
+              adapter identifier and a domain identifier.
+              Sub-elements include:
+            <dl>
+              <dt><code>ap-adapter</code></dt>
+              <dd>The ap-adapter of an AP queue identifies the AP
+                  card to which this AP queue belongs.</dd>
+              <dt><code>ap-domain</code></dt>
+              <dd>The ap-domain of an AP queue identifies the AP
+                  domain to which this AP queue belongs.</dd>
+              <dd>AP Queue identifier.</dd>
+            </dl>
+          </dd>
         </dl>
       </dd>
     </dl>
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index d2f9dac6..60367653 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -88,6 +88,7 @@
         <ref name="capcssdev"/>
         <ref name="capvdpa"/>
         <ref name="capapcard"/>
+        <ref name="capapqueue"/>
       </choice>
     </element>
   </define>
@@ -678,6 +679,18 @@
     </element>
   </define>
 
+  <define name="capapqueue">
+    <attribute name="type">
+      <value>ap_queue</value>
+    </attribute>
+    <element name="ap-adapter">
+      <ref name="uint8"/>
+    </element>
+    <element name="ap-domain">
+      <ref name="apDomainRange"/>
+    </element>
+  </define>
+
   <define name="address">
     <element name="address">
       <attribute name="domain"><ref name="hexuint"/></attribute>
@@ -726,4 +739,16 @@
     </element>
   </define>
 
+  <define name="apDomainRange">
+    <choice>
+      <data type="string">
+        <param name="pattern">0x[0-9a-fA-F]{1,4}</param>
+      </data>
+      <data type="int">
+        <param name="minInclusive">0</param>
+        <param name="maxInclusive">255</param>
+      </data>
+    </choice>
+  </define>
+
 </grammar>
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 5d7a23cb..923c2a05 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -68,6 +68,7 @@ VIR_ENUM_IMPL(virNodeDevCap,
               "css",
               "vdpa",
               "ap_card",
+              "ap_queue",
 );
 
 VIR_ENUM_IMPL(virNodeDevNetCap,
@@ -655,6 +656,12 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def)
             virBufferAsprintf(&buf, "<ap-adapter>0x%02x</ap-adapter>\n",
                               data->ap_card.ap_adapter);
             break;
+        case VIR_NODE_DEV_CAP_AP_QUEUE:
+            virBufferAsprintf(&buf, "<ap-adapter>0x%02x</ap-adapter>\n",
+                              data->ap_queue.ap_adapter);
+            virBufferAsprintf(&buf, "<ap-domain>0x%04x</ap-domain>\n",
+                              data->ap_queue.ap_domain);
+            break;
         case VIR_NODE_DEV_CAP_MDEV_TYPES:
         case VIR_NODE_DEV_CAP_FC_HOST:
         case VIR_NODE_DEV_CAP_VPORTS:
@@ -976,6 +983,59 @@ virNodeDevCapAPCardParseXML(xmlXPathContextPtr ctxt,
 }
 
 
+static int
+virNodeDevCapAPQueueParseXML(xmlXPathContextPtr ctxt,
+                            virNodeDeviceDefPtr def,
+                            xmlNodePtr node,
+                            virNodeDevCapAPQueuePtr ap_queue)
+{
+    xmlNodePtr orig;
+    int ret = -1;
+    g_autofree char *adapter = NULL, *dom = NULL;
+
+    orig = ctxt->node;
+    ctxt->node = node;
+
+   if (!(adapter = virXPathString("string(./ap-adapter[1])", ctxt))) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("missing ap-adapter value for '%s'"), def->name);
+        return -1;
+    }
+
+    if (virStrToLong_uip(adapter, NULL, 0, &ap_queue->ap_adapter) < 0) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("invalid ap-adapter value '%s' for '%s'"),
+                       adapter, def->name);
+        goto out;
+    }
+
+    if (!(dom = virXPathString("string(./ap-domain[1])", ctxt))) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("missing ap-domain value for '%s'"), def->name);
+        goto out;
+    }
+
+    if (virStrToLong_uip(dom, NULL, 0, &ap_queue->ap_domain) < 0) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("invalid ap-domain value '%s' for '%s'"),
+                       dom, def->name);
+        goto out;
+    }
+
+    if (ap_queue->ap_domain > 255) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("ap-domain value '%s' is out of range for '%s'"),
+                       dom, def->name);
+        goto out;
+    }
+
+    ret = 0;
+ out:
+    ctxt->node = orig;
+    return ret;
+}
+
+
 static int
 virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt,
                              virNodeDeviceDefPtr def,
@@ -2018,6 +2078,10 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt,
         ret = virNodeDevCapAPCardParseXML(ctxt, def, node,
                                           &caps->data.ap_card);
         break;
+    case VIR_NODE_DEV_CAP_AP_QUEUE:
+        ret = virNodeDevCapAPQueueParseXML(ctxt, def, node,
+                                           &caps->data.ap_queue);
+        break;
     case VIR_NODE_DEV_CAP_MDEV_TYPES:
     case VIR_NODE_DEV_CAP_FC_HOST:
     case VIR_NODE_DEV_CAP_VPORTS:
@@ -2346,6 +2410,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
     case VIR_NODE_DEV_CAP_CCW_DEV:
     case VIR_NODE_DEV_CAP_VDPA:
     case VIR_NODE_DEV_CAP_AP_CARD:
+    case VIR_NODE_DEV_CAP_AP_QUEUE:
     case VIR_NODE_DEV_CAP_LAST:
         /* This case is here to shutup the compiler */
         break;
@@ -2406,6 +2471,7 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def)
         case VIR_NODE_DEV_CAP_CCW_DEV:
         case VIR_NODE_DEV_CAP_VDPA:
         case VIR_NODE_DEV_CAP_AP_CARD:
+        case VIR_NODE_DEV_CAP_AP_QUEUE:
         case VIR_NODE_DEV_CAP_LAST:
             break;
         }
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index f86f880c..27cb0004 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -67,6 +67,7 @@ typedef enum {
     VIR_NODE_DEV_CAP_CSS_DEV,           /* s390 channel subsystem device */
     VIR_NODE_DEV_CAP_VDPA,              /* vDPA device */
     VIR_NODE_DEV_CAP_AP_CARD,           /* s390 AP Card device */
+    VIR_NODE_DEV_CAP_AP_QUEUE,          /* s390 AP Queue */
 
     VIR_NODE_DEV_CAP_LAST
 } virNodeDevCapType;
@@ -296,6 +297,13 @@ struct _virNodeDevCapAPCard {
     unsigned int ap_adapter;
 };
 
+typedef struct _virNodeDevCapAPQueue virNodeDevCapAPQueue;
+typedef virNodeDevCapAPQueue *virNodeDevCapAPQueuePtr;
+struct _virNodeDevCapAPQueue {
+    unsigned int ap_adapter;
+    unsigned int ap_domain;
+};
+
 typedef struct _virNodeDevCapData virNodeDevCapData;
 typedef virNodeDevCapData *virNodeDevCapDataPtr;
 struct _virNodeDevCapData {
@@ -316,6 +324,7 @@ struct _virNodeDevCapData {
         virNodeDevCapCCW ccw_dev;
         virNodeDevCapVDPA vdpa;
         virNodeDevCapAPCard ap_card;
+        virNodeDevCapAPQueue ap_queue;
     };
 };
 
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index 0aa59289..8b4302d7 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -718,6 +718,7 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj,
         case VIR_NODE_DEV_CAP_CCW_DEV:
         case VIR_NODE_DEV_CAP_VDPA:
         case VIR_NODE_DEV_CAP_AP_CARD:
+        case VIR_NODE_DEV_CAP_AP_QUEUE:
         case VIR_NODE_DEV_CAP_LAST:
             break;
         }
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index b4eb4553..5c9ac6fd 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1218,6 +1218,32 @@ udevProcessAPCard(struct udev_device *device,
 }
 
 
+static int
+udevProcessAPQueue(struct udev_device *device,
+                   virNodeDeviceDefPtr def)
+{
+    char *c;
+    virNodeDevCapDataPtr data = &def->caps->data;
+
+    /* The sysfs path would be in the format /sys/bus/ap/devices
+       /XX.YYYY, where XX is the ap adapter id and YYYY is the ap
+       domain id  */
+    if ((c = strrchr(def->sysfs_path, '/')) == NULL ||
+        virStrToLong_ui(c + 1, &c, 16, &data->ap_queue.ap_adapter) < 0 ||
+        virStrToLong_ui(c + 1, &c, 16, &data->ap_queue.ap_domain) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("failed to parse the AP Queue from sysfs path: '%s'"),
+                       def->sysfs_path);
+        return -1;
+    }
+
+    if (udevGenerateDeviceName(device, def, NULL) != 0)
+        return -1;
+
+    return 0;
+}
+
+
 static int
 udevGetDeviceNodes(struct udev_device *device,
                    virNodeDeviceDefPtr def)
@@ -1274,6 +1300,8 @@ udevGetDeviceType(struct udev_device *device,
             *type = VIR_NODE_DEV_CAP_DRM;
         else if (STREQ(devtype, "ap_card"))
             *type = VIR_NODE_DEV_CAP_AP_CARD;
+        else if (STREQ(devtype, "ap_queue"))
+            *type = VIR_NODE_DEV_CAP_AP_QUEUE;
     } else {
         /* PCI devices don't set the DEVTYPE property. */
         if (udevHasDeviceProperty(device, "PCI_CLASS"))
@@ -1351,6 +1379,8 @@ udevGetDeviceDetails(struct udev_device *device,
         return udevProcessVDPA(device, def);
     case VIR_NODE_DEV_CAP_AP_CARD:
         return udevProcessAPCard(device, def);
+    case VIR_NODE_DEV_CAP_AP_QUEUE:
+        return udevProcessAPQueue(device, def);
     case VIR_NODE_DEV_CAP_MDEV_TYPES:
     case VIR_NODE_DEV_CAP_SYSTEM:
     case VIR_NODE_DEV_CAP_FC_HOST:
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 32c12553..81752e85 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -468,6 +468,7 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED)
             flags |= VIR_CONNECT_LIST_NODE_DEVICES_CAP_VDPA;
             break;
         case VIR_NODE_DEV_CAP_AP_CARD:
+        case VIR_NODE_DEV_CAP_AP_QUEUE:
         case VIR_NODE_DEV_CAP_LAST:
             break;
         }
-- 
2.26.2

Re: [PATCH libvirt v3 03/11] nodedev: detect AP queues
Posted by Erik Skultety 3 years, 12 months ago
On Tue, Nov 17, 2020 at 01:10:57PM +0100, Shalini Chellathurai Saroja wrote:
> Each AP card device can support upto 256 AP queues.  AP queues are
> also detected by udev, so add support for libvirt nodedev driver.
> 
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> ---
>  docs/formatnode.html.in            | 15 +++++++
>  docs/schemas/nodedev.rng           | 25 +++++++++++
>  src/conf/node_device_conf.c        | 66 ++++++++++++++++++++++++++++++
>  src/conf/node_device_conf.h        |  9 ++++
>  src/conf/virnodedeviceobj.c        |  1 +
>  src/node_device/node_device_udev.c | 30 ++++++++++++++
>  tools/virsh-nodedev.c              |  1 +
>  7 files changed, 147 insertions(+)
> 

...

> +static int
> +virNodeDevCapAPQueueParseXML(xmlXPathContextPtr ctxt,
> +                            virNodeDeviceDefPtr def,
> +                            xmlNodePtr node,
> +                            virNodeDevCapAPQueuePtr ap_queue)
> +{
> +    xmlNodePtr orig;

We could benefit from VIR_XPATH_NODE_AUTORESTORE(ctxt) here as well.

> +    int ret = -1;
> +    g_autofree char *adapter = NULL, *dom = NULL;

^One definition per line please.

> +
> +    orig = ctxt->node;
> +    ctxt->node = node;
> +
> +   if (!(adapter = virXPathString("string(./ap-adapter[1])", ctxt))) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("missing ap-adapter value for '%s'"), def->name);
> +        return -1;
> +    }
> +
> +    if (virStrToLong_uip(adapter, NULL, 0, &ap_queue->ap_adapter) < 0) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("invalid ap-adapter value '%s' for '%s'"),
> +                       adapter, def->name);
> +        goto out;
> +    }

Like I mentioned in patch 1, we could extract ^this into a tiny static helper.

Regards,
Erik