Re: [PATCHv4 1/5] domifaddr: Implement the public API

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

 



On 25/08/13 07:15, Nehal J Wani wrote:


s/API/APIs/,  in the subject.

Define a new API virDomainInterfacesAddresses, which returns
the address information of a running domain's interfaces(s).
If no interface name is specified, it returns the information
of all interfaces, otherwise it only returns the information
of the specificed interface. The address information includes
the MAC and IP addresses.

Better to mention why to introduce virDomainInterfaceFree. E.g.

virDomainInterfaceFree allows the upper layer application
to free the domain interface object conveniently.


The API is going to provide multiple methods by flags, e.g.

   * Query guest agent
   * Parse lease file of dnsmasq
   * DHCP snooping

But at this stage, it will only work with guest agent, and flags
won't be supported.

include/libvirt/libvirt.h.in:
   * Define virDomainInterfacesAddresses, virDomainInterfaceFree
   * Define structs virDomainInterface, virDomainIPAddress

python/generator.py:
   * Skip the auto-generation for virDomainInterfacesAddresses
     and virDomainInterfaceFree

src/driver.h:
   * Define domainInterfacesAddresses

src/libvirt.c:
   * Implement virDomainInterfacesAddresses
   * Implement virDomainInterfaceFree

src/libvirt_public.syms:
   * Export the new symbols

---
  include/libvirt/libvirt.h.in |  33 +++++++++++++
  python/generator.py          |   3 ++
  src/driver.h                 |   6 +++
  src/libvirt.c                | 109 +++++++++++++++++++++++++++++++++++++++++++
  src/libvirt_public.syms      |   6 +++
  5 files changed, 157 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index a47e33c..deb1e1f 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2044,6 +2044,39 @@ int                     virDomainGetInterfaceParameters (virDomainPtr dom,
                                                           virTypedParameterPtr params,
                                                           int *nparams, unsigned int flags);
+typedef enum {
+    VIR_IP_ADDR_TYPE_IPV4,
+    VIR_IP_ADDR_TYPE_IPV6,
+
+#ifdef VIR_ENUM_SENTINELS
+    VIR_IP_ADDR_TYPE_LAST
+#endif
+} virIPAddrType;
+
+
+typedef struct _virDomainInterfaceIPAddress virDomainIPAddress;
+typedef virDomainIPAddress *virDomainIPAddressPtr;
+struct _virDomainInterfaceIPAddress {
+    int type;       /* virIPAddrType */
+    char *addr;     /* IP address */
+    int prefix;     /* IP address prefix */
+};
+
+typedef struct _virDomainInterface virDomainInterface;
+typedef virDomainInterface *virDomainInterfacePtr;
+struct _virDomainInterface {
+    char *name;                     /* interface name */
+    char *hwaddr;                   /* hardware address */
+    unsigned int naddrs;            /* number of items in @addrs */
+    virDomainIPAddressPtr addrs;    /* array of IP addresses */
+};
+
+int virDomainInterfacesAddresses (virDomainPtr dom,
+                                  virDomainInterfacePtr **ifaces,
+                                  unsigned int flags);
+
+void virDomainInterfaceFree (virDomainInterfacePtr iface);
+
  /* Management of domain block devices */
int virDomainBlockPeek (virDomainPtr dom,
diff --git a/python/generator.py b/python/generator.py
index fb321c6..f24561e 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -458,6 +458,7 @@ skip_impl = (
      'virNodeGetMemoryParameters',
      'virNodeSetMemoryParameters',
      'virNodeGetCPUMap',
+    'virDomainInterfacesAddresses',
      'virDomainMigrate3',
      'virDomainMigrateToURI3',
  )
@@ -560,6 +561,8 @@ skip_function = (
      "virTypedParamsGetString",
      "virTypedParamsGetUInt",
      "virTypedParamsGetULLong",
+
+    "virDomainInterfaceFree", # Only useful in C
  )
lxc_skip_function = (
diff --git a/src/driver.h b/src/driver.h
index be64333..8b6182e 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -518,6 +518,11 @@ typedef int
                                        unsigned int flags);
typedef int
+(*virDrvDomainInterfacesAddresses)(virDomainPtr dom,
+                                   virDomainInterfacePtr **ifaces,
+                                   unsigned int flags);
+
+typedef int
  (*virDrvDomainMemoryStats)(virDomainPtr domain,
                             struct _virDomainMemoryStat *stats,
                             unsigned int nr_stats,
@@ -1238,6 +1243,7 @@ struct _virDriver {
      virDrvDomainInterfaceStats domainInterfaceStats;
      virDrvDomainSetInterfaceParameters domainSetInterfaceParameters;
      virDrvDomainGetInterfaceParameters domainGetInterfaceParameters;
+    virDrvDomainInterfacesAddresses    domainInterfacesAddresses;
      virDrvDomainMemoryStats domainMemoryStats;
      virDrvDomainBlockPeek domainBlockPeek;
      virDrvDomainMemoryPeek domainMemoryPeek;
diff --git a/src/libvirt.c b/src/libvirt.c
index 07a3fd5..05e3a03 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -8643,6 +8643,94 @@ error:
      return -1;
  }
+ /**
+ * virDomainInterfacesAddresses:
+ * @dom: domain object
+ * @ifaces: pointer to an array of pointers pointing interface objects

s/pointing/pointing to/,

+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Return a pointer to the allocated array of interfaces present in given

s/array of/array of pointers pointing to/,

+ * domain along with their IP and MAC addresses. Note that single interface
+ * can have multiple or even 0 IP address.
+ *
+ * This API dynamically allocates the virDomainInterfacePtr struct based
+ * on how many interfaces domain @dom has, usually there's 1:1 correlation.
+ * The count of the interfaces is returned as the return value.
+ *
+ * Note that for some hypervisors, a configured guest agent is needed
+ * for successful return from this API. Moreover, if guest agent is
+ * used then the interface name is the one seen by guest OS. To match
+ * such interface with the one from @dom XML use MAC address or IP range.
+ *
+ * @ifaces->name is never NULL, @ifaces->hwaddr might be NULL.
+ *
+ * The caller *must* free @ifaces when no longer needed. Usual use
+ * case looks like this:
+ *
+ * virDomainInterfacePtr *ifaces = NULL;
+ * int ifaces_count = 0;
+ * size_t i, j;
+ * virDomainPtr dom = ... obtain a domain here ...;
+ *
+ * if ((ifaces_count = virDomainInterfacesAddresses(dom, &ifaces, 0)) < 0)
+ *     goto cleanup;
+ *
+ * ... do something with returned values, for example:
+ * for (i = 0; i < ifaces_count; i++) {
+ *     printf("name: %s", ifaces[i]->name);
+ *     if (ifaces[i]->hwaddr)
+ *         printf(" hwaddr: %s", ifaces[i]->hwaddr);
+ *
+ *     for (j = 0; j < ifaces[i]->naddrs; j++) {
+ *         virDomainIPAddressPtr ip_addr = ifaces[i]->addrs + j;
+ *         printf("[addr: %s prefix: %d type: %d]",
+ *                ip_addr->addr, ip_addr->prefix, ip_addr->type);
+ *     }
+ *     printf("\n");
+ * }
+ *
+ * cleanup:
+ * for (i = 0; i < ifaces_count; i++)
+ *     virDomainInterfaceFree(ifaces[i]);
+ * free(ifaces);

Indentions.

+ *
+ * Returns the number of interfaces on success, -1 in case of error.
+ */
+int
+virDomainInterfacesAddresses(virDomainPtr dom,
+                             virDomainInterfacePtr **ifaces,
+                             unsigned int flags)
+{
+    virConnectPtr conn;
+
+    VIR_DOMAIN_DEBUG(dom, "ifaces=%p, flags=%x", ifaces, flags);
+
+    virResetLastError();
+
+    if (!VIR_IS_CONNECTED_DOMAIN(dom)) {
+        virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+        goto error;
+    }
+
+    conn = dom->conn;
+
+    virCheckNonNullArgGoto(ifaces, error);
+
+    if (conn->driver->domainInterfacesAddresses) {
+        int ret;
+        ret = conn->driver->domainInterfacesAddresses(dom, ifaces, flags);
+        if (ret < 0)
+            goto error;
+        return ret;
+    }
+
+    virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+    virDispatchError(dom ? dom->conn : NULL);
+    return -1;
+}
+
  /**
   * virDomainMemoryStats:
   * @dom: pointer to the domain object
@@ -21961,3 +22049,24 @@ error:
      virDispatchError(dom->conn);
      return -1;
  }
+
+/**
+ * virDomainInterfaceFree:
+ * @iface: an interface object
+ *
+ * Free the interface object. The data structure is
+ * freed and should not be used thereafter.
+ */
+void
+virDomainInterfaceFree(virDomainInterfacePtr iface)
+{
+    if (iface) {
+        size_t i;
+        VIR_FREE(iface->name);
+        VIR_FREE(iface->hwaddr);
+        for (i = 0; i < iface->naddrs; i++)
+            VIR_FREE(iface->addrs[i].addr);
+        VIR_FREE(iface->addrs);
+    }
+    VIR_FREE(iface);

I'd like write it like:

{
    size_t i;

    if (!iface)
        return;

    VIR_FREE(iface->name);
    VIR_FREE(iface->hwaddr);
    for (i = 0; i < iface->naddrs; i++)
        VIR_FREE(iface->addrs[i].addr);
    VIR_FREE(iface->addrs);

    VIR_FREE(iface);
}
+}
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index bbdf78a..df1c166 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -634,4 +634,10 @@ LIBVIRT_1.1.1 {
          virDomainSetMemoryStatsPeriod;
  } LIBVIRT_1.1.0;
+LIBVIRT_1.1.2 {
+    global:
+        virDomainInterfacesAddresses;
+        virDomainInterfaceFree;
+} LIBVIRT_1.1.1;
+
  # .... define new API here using predicted next version number ....

ACK with comments fixed:
diff --git a/src/libvirt.c b/src/libvirt.c
index 05e3a03..0af8aa2 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -8646,12 +8646,12 @@ error:
  /**
  * virDomainInterfacesAddresses:
  * @dom: domain object
- * @ifaces: pointer to an array of pointers pointing interface objects
+ * @ifaces: pointer to an array of pointers pointing to interface objects
  * @flags: extra flags; not used yet, so callers should always pass 0
  *
- * Return a pointer to the allocated array of interfaces present in given
- * domain along with their IP and MAC addresses. Note that single interface
- * can have multiple or even 0 IP address.
+ * Return a pointer to the allocated array of pointers pointing to interfaces
+ * present in given domain along with their IP and MAC addresses. Note that
+ * single interface can have multiple or even 0 IP address.
  *
  * This API dynamically allocates the virDomainInterfacePtr struct based
  * on how many interfaces domain @dom has, usually there's 1:1 correlation.
@@ -8690,9 +8690,9 @@ error:
  * }
  *
  * cleanup:
- * for (i = 0; i < ifaces_count; i++)
- *     virDomainInterfaceFree(ifaces[i]);
- * free(ifaces);
+ *     for (i = 0; i < ifaces_count; i++)
+ *         virDomainInterfaceFree(ifaces[i]);
+ *     free(ifaces);
  *
  * Returns the number of interfaces on success, -1 in case of error.
  */
@@ -8732,6 +8732,30 @@ error:
 }
 
 /**
+ * virDomainInterfaceFree:
+ * @iface: an domain interface object
+ *
+ * Free the domain interface object. The data structure is
+ * freed and should not be used thereafter.
+ */
+void
+virDomainInterfaceFree(virDomainInterfacePtr iface)
+{
+    size_t i;
+
+    if (!iface)
+        return;
+
+    VIR_FREE(iface->name);
+    VIR_FREE(iface->hwaddr);
+    for (i = 0; i < iface->naddrs; i++)
+        VIR_FREE(iface->addrs[i].addr);
+    VIR_FREE(iface->addrs);
+
+    VIR_FREE(iface);
+}
+
+/**
  * virDomainMemoryStats:
  * @dom: pointer to the domain object
  * @stats: nr_stats-sized array of stat structures (returned)
@@ -22049,24 +22073,3 @@ error:
     virDispatchError(dom->conn);
     return -1;
 }
-
-/**
- * virDomainInterfaceFree:
- * @iface: an interface object
- *
- * Free the interface object. The data structure is
- * freed and should not be used thereafter.
- */
-void
-virDomainInterfaceFree(virDomainInterfacePtr iface)
-{
-    if (iface) {
-        size_t i;
-        VIR_FREE(iface->name);
-        VIR_FREE(iface->hwaddr);
-        for (i = 0; i < iface->naddrs; i++)
-            VIR_FREE(iface->addrs[i].addr);
-        VIR_FREE(iface->addrs);
-    }
-    VIR_FREE(iface);
-}
--
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]