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

Re: [libvirt] [PATCH 1/7] list: Define new API virConnectListAllNodeDevices



On 09/05/12 07:34, Osier Yang wrote:
This is to list the node device objects, supports to filter the results
by capability types.

include/libvirt/libvirt.h.in: Declare enum virConnectListAllNodeDeviceFlags
                               and virConnectListAllNodeDevices.
python/generator.py: Skip auto-generating
src/driver.h: (virDrvConnectListAllNodeDevices)
src/libvirt.c: Implement the public API
src/libvirt_public.syms: Export the symbol to public
---
  include/libvirt/libvirt.h.in |   25 +++++++++++++++++
  python/generator.py          |    1 +
  src/driver.h                 |    4 +++
  src/libvirt.c                |   62 ++++++++++++++++++++++++++++++++++++++++++
  src/libvirt_public.syms      |    1 +
  5 files changed, 93 insertions(+), 0 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 9226f3e..96d0760 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2834,6 +2834,31 @@ int                     virNodeListDevices      (virConnectPtr conn,
                                                   char **const names,
                                                   int maxnames,
                                                   unsigned int flags);
+/*
+ * virConnectListAllNodeDevices:
+ *
+ * Flags used to filter the returned node devices. Flags in each group
+ * are exclusive.
+ */
+typedef enum {
+    /* Reserved the first 6 bits for the possibility of persistent
+     * node device support in future.
+     */

Huh, I think reserving some bits doesn't make sense. If we add something, we might just add it at the end and nothing will happen. And if you will need to add more than 6 bits of filters, than you'll need to split it.

+
+    VIR_CONNECT_LIST_NODE_DEVICES_CAP_SYSTEM        = 1 << 6,
+    VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV       = 1 << 7,
+    VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_DEV       = 1 << 8,
+    VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_INTERFACE = 1 << 9,
+    VIR_CONNECT_LIST_NODE_DEVICES_CAP_NET           = 1 << 10,
+    VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_HOST     = 1 << 11,
+    VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_TARGET   = 1 << 12,
+    VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI          = 1 << 13,
+    VIR_CONNECT_LIST_NODE_DEVICES_CAP_STORAGE       = 1 << 14,
+} virConnectListAllNodeDeviceFlags;

Also it probably would be better to add a comment to (at least some of those) flags. For example I don't know the difference between VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI and the other two SCSI filters.

+
+int                     virConnectListAllNodeDevices (virConnectPtr conn,
+                                                      virNodeDevicePtr **devices,
+                                                      unsigned int flags);

  virNodeDevicePtr        virNodeDeviceLookupByName (virConnectPtr conn,
                                                     const char *name);
diff --git a/python/generator.py b/python/generator.py
index 8f6e455..a8e4ec6 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -464,6 +464,7 @@ skip_function = (
      'virStoragePoolListAllVolumes', # overridden in virStoragePool.py
      'virConnectListAllNetworks', # overridden in virConnect.py
      'virConnectListAllInterfaces', # overridden in virConnect.py
+    'virConnectListAllNodeDevices', # overridden in virConnect.py

      'virStreamRecvAll', # Pure python libvirt-override-virStream.py
      'virStreamSendAll', # Pure python libvirt-override-virStream.py
diff --git a/src/driver.h b/src/driver.h
index 518e9d4..34a94af 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1483,6 +1483,9 @@ typedef int (*virDevMonListDevices)(virConnectPtr conn,
                                      char **const names,
                                      int maxnames,
                                      unsigned int flags);
+typedef int (*virDevMonListAllNodeDevices)(virConnectPtr conn,
+                                           virNodeDevicePtr **devices,
+                                           unsigned int flags);

  typedef virNodeDevicePtr (*virDevMonDeviceLookupByName)(virConnectPtr conn,
                                                          const char *name);
@@ -1516,6 +1519,7 @@ struct _virDeviceMonitor {
      virDrvClose                 close;
      virDevMonNumOfDevices       numOfDevices;
      virDevMonListDevices        listDevices;
+    virDevMonListAllNodeDevices listAllNodeDevices;

In case of this API the old one actually does list all devices as this one does and has no races, but It's horrible to use.

      virDevMonDeviceLookupByName deviceLookupByName;
      virDevMonDeviceGetXMLDesc   deviceGetXMLDesc;
      virDevMonDeviceGetParent    deviceGetParent;
diff --git a/src/libvirt.c b/src/libvirt.c
index 90ed7ff..b8d0bec 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -13793,6 +13793,66 @@ error:
      return -1;
  }

+/**
+ * virConnectListAllNodeDevices:
+ * @conn: Pointer to the hypervisor connection.
+ * @devices: Pointer to a variable to store the array containing the node
+ *           device objects or NULL if the list is not required (just returns
+ *           number of node devices).
+ * @flags: bitwise-OR of virConnectListAllNodeDevices.
+ *
+ * Collect the list of node devices, and allocate an array to store those
+ * objects.
+ *
+ * Normally, all node devices are returned; however, @flags can be used to
+ * filter the results for a smaller list of targeted node devices.  The valid
+ * flags are divided into groups, where each group contains bits that
+ * describe mutually exclusive attributes of a node device, and where all bits
+ * within a group describe all possible node devices.
+ *
+ * Only one group of the @flags is supported. It supports to filter the node
+ * devices by capability type.

I'd list the flags here even if there's just the one group.

+ *
+ * Returns the number of node devices found or -1 and sets @devices to NULL in
+ * case of error.  On success, the array stored into @devices is guaranteed to
+ * have an extra allocated element set to NULL but not included in the return
+ * count, to make iteration easier.  The caller is responsible for calling
+ * virNodeDeviceFree() on each array element, then calling free() on
+ * @devices.
+ */
+int
+virConnectListAllNodeDevices(virConnectPtr conn,
+                             virNodeDevicePtr **devices,
+                             unsigned int flags)
+{
+    VIR_DEBUG("conn=%p, devices=%p, flags=%x", conn, devices, flags);
+
+    virResetLastError();
+
+    if (devices)
+        *devices = NULL;
+
+    if (!VIR_IS_CONNECT(conn)) {
+        virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
+        virDispatchError(NULL);
+        return -1;
+    }
+
+    if (conn->deviceMonitor &&
+        conn->deviceMonitor->listAllNodeDevices) {
+        int ret;
+        ret = conn->deviceMonitor->listAllNodeDevices(conn, devices, flags);
+        if (ret < 0)
+            goto error;
+        return ret;
+    }
+
+    virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+    virDispatchError(conn);
+    return -1;
+}

  /**
   * virNodeListDevices:
@@ -13804,6 +13864,8 @@ error:
   *
   * Collect the list of node devices, and store their names in @names
   *
+ * For more control over the results, see virConnectListAllNodeDevices().
+ *
   * If the optional 'cap'  argument is non-NULL, then the count
   * will be restricted to devices with the specified capability
   *
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 8dda48b..5a4451b 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -558,6 +558,7 @@ LIBVIRT_0.10.2 {
      global:
          virConnectListAllInterfaces;
          virConnectListAllNetworks;
+        virConnectListAllNodeDevices;
          virConnectListAllStoragePools;
          virStoragePoolListAllVolumes;
  } LIBVIRT_0.10.2;



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