12:49 a.m.
This API returns the leases information stored in the DHCP leases
file of dnsmasq for a given virtual network. It contacts the
bridge network driver, which parses the leases file.
It supports two methods:
1. Return info for all network interfaces connected to a given
virtual network
2. Return information for a particular network interface in a
given virtual network by providing its MAC Address
v3
* Mostly small nits, change in MACRO names, use of virSocketAddrGetIpPrefix to
retrieve IP prefix from @dom XML.
v2
* Since DHCPv6 is supposed to be suported in future, virNetworkGetDHCPLeasesForMAC
changed, prefix and virIPAddrType added in virNetworkDHCPLeases struct.
Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg00732.html
v1
* Refer: https://www.redhat.com/archives/libvir-list/2013-September/msg00620.html
* The need for these APIs were result of a RFC was proposed on the list.
Refer: http://www.redhat.com/archives/libvir-list/2013-July/msg01603.html
Nehal J Wani (4):
net-dhcp-leases: Implement the public APIs
net-dhcp-leases: Implement the remote protocol
net-dhcp-leases: Private implementation inside network driver
net-dhcp-leases: Add virsh support
daemon/remote.c | 133 ++++++++++++++++++++++++++++++++
include/libvirt/libvirt.h.in | 33 ++++++++
python/generator.py | 3 +
src/driver.h | 13 ++++
src/libvirt.c | 173 ++++++++++++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 7 ++
src/network/bridge_driver.c | 177 +++++++++++++++++++++++++++++++++++++++++++
src/remote/remote_driver.c | 150 ++++++++++++++++++++++++++++++++++++
src/remote/remote_protocol.x | 50 +++++++++++-
src/remote_protocol-structs | 32 ++++++++
src/rpc/gendispatch.pl | 1 +
tools/virsh-network.c | 100 ++++++++++++++++++++++++
tools/virsh.pod | 6 ++
13 files changed, 877 insertions(+), 1 deletion(-)
--
1.7.11.7
12:49 a.m.
New subject: [libvirt] [PATCHv3 1/4] net-dhcp-leases: Implement the public APIs
Introduce 3 new APIs, virNetworkGetDHCPLeases, virNetworkGetDHCPLeasesForMAC
and virNetworkDHCPLeaseFree.
* virNetworkGetDHCPLeases: returns the dhcp leases information for a given
virtual network. The information includes lease expirytime, MAC Address,
IP Address (with type and prefix), hostname and clientid.
* virNetworkGetDHCPLeasesForMAC: returns the dhcp leases information for a
given virtual network and specified MAC Address.
* virNetworkDHCPLeaseFree: allows the upper layer application to free the
network interface object conveniently.
There is no support for flags, so user is expected to pass 0 for
both the APIs.
include/libvirt/libvirt.h.in:
* Define virNetworkGetDHCPLeases
* Define virNetworkGetDHCPLeasesForMAC
* Define virNetworkDHCPLeaseFree
python/generator.py:
* Skip the auto-generation for virNetworkGetDHCPLeases
* Skip the auto-generation for virNetworkGetDHCPLeasesForMAC
* Skip the auto-generation for virNetworkDHCPLeaseFree
src/driver.h:
* Define networkGetDHCPLeases
* Define networkGetDHCPLeasesForMAC
src/libvirt.c:
* Implement virNetworkGetDHCPLeases
* Implement virNetworkGetDHCPLeasesForMAC
* Implement virNetworkDHCPLeaseFree
src/libvirt_public.syms:
* Export the new symbols
---
include/libvirt/libvirt.h.in | 33 +++++++++
python/generator.py | 3 +
src/driver.h | 13 ++++
src/libvirt.c | 173 +++++++++++++++++++++++++++++++++++++++++++
src/libvirt_public.syms | 7 ++
5 files changed, 229 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index a47e33c..d92a720 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2800,6 +2800,39 @@ int virConnectNumOfDefinedInterfaces
(virConnectPtr conn);
int virConnectListDefinedInterfaces (virConnectPtr conn,
char **const names,
int maxnames);
+
+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 _virNetworkDHCPLeases virNetworkDHCPLeases;
+typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr;
+struct _virNetworkDHCPLeases {
+ long long expirytime;
+ char *mac; /* MAC address */
+ char *ipaddr; /* IP address */
+ char *hostname; /* Hostname */
+ char *clientid; /* Client ID */
+ int type; /* virIPAddrType */
+ unsigned int prefix; /* IP address prefix */
+};
+
+void virNetworkDHCPLeaseFree(virNetworkDHCPLeasesPtr lease);
+
+int virNetworkGetDHCPLeases(virNetworkPtr network,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags);
+
+int virNetworkGetDHCPLeasesForMAC(virNetworkPtr network,
+ const char *mac,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags);
+
/*
* virConnectListAllInterfaces:
*
diff --git a/python/generator.py b/python/generator.py
index a91dde8..e76cbfc 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -460,6 +460,8 @@ skip_impl = (
'virNodeGetCPUMap',
'virDomainMigrate3',
'virDomainMigrateToURI3',
+ 'virNetworkGetDHCPLeases',
+ 'virNetworkGetDHCPLeasesForMAC',
)
lxc_skip_impl = (
@@ -560,6 +562,7 @@ skip_function = (
"virTypedParamsGetString",
"virTypedParamsGetUInt",
"virTypedParamsGetULLong",
+ 'virNetworkDHCPLeaseFree',
)
lxc_skip_function = (
diff --git a/src/driver.h b/src/driver.h
index be64333..698e0ca 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1121,6 +1121,17 @@ typedef int
int cookieinlen,
unsigned int flags,
int cancelled);
+typedef int
+(*virDrvNetworkGetDHCPLeases)(virNetworkPtr network,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags);
+
+typedef int
+(*virDrvNetworkGetDHCPLeasesForMAC)(virNetworkPtr network,
+ const char *mac,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags);
+
typedef struct _virDriver virDriver;
typedef virDriver *virDriverPtr;
@@ -1451,6 +1462,8 @@ struct _virNetworkDriver {
virDrvNetworkSetAutostart networkSetAutostart;
virDrvNetworkIsActive networkIsActive;
virDrvNetworkIsPersistent networkIsPersistent;
+ virDrvNetworkGetDHCPLeases networkGetDHCPLeases;
+ virDrvNetworkGetDHCPLeasesForMAC networkGetDHCPLeasesForMAC;
};
diff --git a/src/libvirt.c b/src/libvirt.c
index a6fcab0..05d20e9 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -68,6 +68,7 @@
#include "virstring.h"
#include "virutil.h"
#include "virtypedparam.h"
+#include "virmacaddr.h"
#ifdef WITH_TEST
# include "test/test_driver.h"
@@ -21966,3 +21967,175 @@ error:
virDispatchError(dom->conn);
return -1;
}
+
+/**
+ * virNetworkGetDHCPLeases:
+ * @network: pointer to network object
+ * @leases: pointer to an array of pointers pointing to the obtained leases
+ * @flags: extra flags, not used yet, so callers should always pass 0
+ *
+ * The API returns lease info for all network interfaces connected to the
+ * given virtual network.
+ *
+ * Returns the number of leases found or -1 and sets @leases to NULL in case
+ * of error. On success, the array stored into @leases 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
+ * virNetworkDHCPLeaseFree on each array element, then calling free() on @leases.
+ *
+ * Example of usage:
+ *
+ * virNetworkDHCPLeasesPtr *leases = NULL;
+ * virNetworkPtr network = ... obtain a network pointer here ...;
+ * size_t i;
+ * int nleases;
+ * unsigned int flags = 0;
+ *
+ * nleases = virNetworkGetDHCPLeases(network, &leases, flags);
+ * if (nleases < 0)
+ * error();
+ *
+ * ... do something with returned values, for example:
+ *
+ * for (i = 0; i < nleases; i++) {
+ * virNetworkDHCPLeasesPtr lease = leases[i];
+ * printf("Time(epoch): %lu, MAC address: %s, "
+ * "IP address: %s, Hostname: %s, ClientID: %s\n",
+ * leas->expirytime, lease->mac, lease->ipaddr,
+ * lease->hostname, lease->clientid);
+ * }
+ *
+ * if (leases) {
+ * for (i = 0; i < nleases; i++)
+ * virNetworkDHCPLeaseFree(leases[i]);
+ * }
+ * free(leases);
+ *
+ */
+int
+virNetworkGetDHCPLeases(virNetworkPtr network,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags)
+{
+ virConnectPtr conn;
+ VIR_DEBUG("network=%p, leases=%p, flags=%x",
+ network, leases, flags);
+
+ virResetLastError();
+
+ virCheckNonNullArgGoto(network, error);
+
+ if (leases)
+ *leases = NULL;
+
+ if (!VIR_IS_CONNECTED_NETWORK(network)) {
+ virLibNetworkError(VIR_ERR_INVALID_NETWORK, __FUNCTION__);
+ virDispatchError(NULL);
+ return -1;
+ }
+
+ conn = network->conn;
+
+ if (conn->networkDriver &&
conn->networkDriver->networkGetDHCPLeases) {
+ int ret;
+ ret = conn->networkDriver->networkGetDHCPLeases(network,leases, flags);
+ if (ret < 0)
+ goto error;
+ return ret;
+ }
+
+ virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+ virDispatchError(network->conn);
+ return -1;
+}
+
+/**
+ * virNetworkGetDHCPLeasesForMAC:
+ * @network: pointer to network object
+ * @mac: MAC Address of an interface
+ * @leases: pointer to an array of pointers pointing to the obtained leases
+ * @flags: extra flags, not used yet, so callers should always pass 0
+ *
+ * The API returns the leases info of the interface which matches with the
+ * given @mac. There can be multiple leases for a single @mac because this
+ * API intends to support DHCPv6 too.
+ *
+ * Returns the number of leases found or -1 and sets @leases to NULL in case
+ * of error. On success, the array stored into @leases 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
+ * virNetworkDHCPLeaseFree on each array element, then calling free() on @leases.
+ */
+int
+virNetworkGetDHCPLeasesForMAC(virNetworkPtr network,
+ const char *mac,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags)
+{
+ virConnectPtr conn;
+ virMacAddr addr;
+
+ VIR_DEBUG("network=%p, mac=%s, leases=%p, flags=%x",
+ network, mac, leases, flags);
+
+ virResetLastError();
+
+ virCheckNonNullArgGoto(network, error);
+ virCheckNonNullArgGoto(mac, error);
+
+ if (leases)
+ *leases = NULL;
+
+ if (!VIR_IS_CONNECTED_NETWORK(network)) {
+ virLibNetworkError(VIR_ERR_INVALID_NETWORK, __FUNCTION__);
+ virDispatchError(NULL);
+ return -1;
+ }
+
+ /* Validate the MAC address */
+ if (mac && virMacAddrParse(mac, &addr) < 0) {
+ virReportInvalidArg(mac,
+ _("Given MAC Address doesn't comply "
+ "with the standard (IEEE 802) format in %s"),
+ __FUNCTION__);
+ goto error;
+ }
+
+ conn = network->conn;
+
+ if (conn->networkDriver &&
+ conn->networkDriver->networkGetDHCPLeasesForMAC) {
+ int ret;
+ ret = conn->networkDriver->networkGetDHCPLeasesForMAC(network, mac,
+ leases, flags);
+ if (ret < 0)
+ goto error;
+ return ret;
+ }
+
+ virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+ virDispatchError(network->conn);
+ return -1;
+}
+
+/**
+ * virNetworkDHCPLeaseFree:
+ * @lease: pointer to a leases object
+ *
+ * Frees all the memory occupied by @lease.
+ */
+void
+virNetworkDHCPLeaseFree(virNetworkDHCPLeasesPtr lease)
+{
+ if (!lease)
+ return;
+ VIR_FREE(lease->mac);
+ VIR_FREE(lease->ipaddr);
+ VIR_FREE(lease->hostname);
+ VIR_FREE(lease->clientid);
+ VIR_FREE(lease);
+}
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index bbdf78a..e952869 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -634,4 +634,11 @@ LIBVIRT_1.1.1 {
virDomainSetMemoryStatsPeriod;
} LIBVIRT_1.1.0;
+LIBVIRT_1.1.3 {
+ global:
+ virNetworkGetDHCPLeases;
+ virNetworkGetDHCPLeasesForMAC;
+ virNetworkDHCPLeaseFree;
+} LIBVIRT_1.1.1;
+
# .... define new API here using predicted next version number ....
--
1.7.11.7
4:52 a.m.
New subject: [libvirt] [PATCHv3 1/4] net-dhcp-leases: Implement the public APIs
On Mon, Sep 16, 2013 at 11:19:13AM +0530, Nehal J Wani wrote:
+int
+virNetworkGetDHCPLeasesForMAC(virNetworkPtr network,
+ const char *mac,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags)
+{
+ virConnectPtr conn;
+ virMacAddr addr;
+
+ VIR_DEBUG("network=%p, mac=%s, leases=%p, flags=%x",
+ network, mac, leases, flags);
+
+ virResetLastError();
+
+ virCheckNonNullArgGoto(network, error);
+ virCheckNonNullArgGoto(mac, error);
+
+ if (leases)
+ *leases = NULL;
+
+ if (!VIR_IS_CONNECTED_NETWORK(network)) {
+ virLibNetworkError(VIR_ERR_INVALID_NETWORK, __FUNCTION__);
+ virDispatchError(NULL);
+ return -1;
+ }
+
+ /* Validate the MAC address */
+ if (mac && virMacAddrParse(mac, &addr) < 0) {
+ virReportInvalidArg(mac,
+ _("Given MAC Address doesn't comply "
+ "with the standard (IEEE 802) format in %s"),
+ __FUNCTION__);
Don't pass __FUNCTION__ in this error message - that is already done automatically
+ goto error;
+ }
'mac' should be a mandatory parameter here.
+
+ conn = network->conn;
+
+ if (conn->networkDriver &&
+ conn->networkDriver->networkGetDHCPLeasesForMAC) {
+ int ret;
+ ret = conn->networkDriver->networkGetDHCPLeasesForMAC(network, mac,
+ leases, flags);
+ if (ret < 0)
+ goto error;
+ return ret;
+ }
+
+ virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+ virDispatchError(network->conn);
+ return -1;
+}
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
10:14 a.m.
New subject: [libvirt] [PATCHv3 1/4] net-dhcp-leases: Implement the public APIs
On Mon, Sep 16, 2013 at 3:22 PM, Daniel P. Berrange <berrange(a)redhat.com> wrote:
On Mon, Sep 16, 2013 at 11:19:13AM +0530, Nehal J Wani wrote:
> +int
> +virNetworkGetDHCPLeasesForMAC(virNetworkPtr network,
> + const char *mac,
> + virNetworkDHCPLeasesPtr **leases,
> + unsigned int flags)
> +{
> + virConnectPtr conn;
> + virMacAddr addr;
> +
> + VIR_DEBUG("network=%p, mac=%s, leases=%p, flags=%x",
> + network, mac, leases, flags);
> +
> + virResetLastError();
> +
> + virCheckNonNullArgGoto(network, error);
> + virCheckNonNullArgGoto(mac, error);
> +
> + if (leases)
> + *leases = NULL;
> +
> + if (!VIR_IS_CONNECTED_NETWORK(network)) {
> + virLibNetworkError(VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> + virDispatchError(NULL);
> + return -1;
> + }
> +
> + /* Validate the MAC address */
> + if (mac && virMacAddrParse(mac, &addr) < 0) {
> + virReportInvalidArg(mac,
> + _("Given MAC Address doesn't comply "
> + "with the standard (IEEE 802) format in
%s"),
> + __FUNCTION__);
Don't pass __FUNCTION__ in this error message - that is already done automatically
> + goto error;
> + }
'mac' should be a mandatory parameter here.
Attached diff should fix it.
But there are still calls to virReportInvalidArg() with __FUNCTION__
as one of its arguments in src/libvirt.c
> +
> + conn = network->conn;
> +
> + if (conn->networkDriver &&
> + conn->networkDriver->networkGetDHCPLeasesForMAC) {
> + int ret;
> + ret = conn->networkDriver->networkGetDHCPLeasesForMAC(network, mac,
> + leases, flags);
> + if (ret < 0)
> + goto error;
> + return ret;
> + }
> +
> + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> + virDispatchError(network->conn);
> + return -1;
> +}
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
Nehal J Wani
UG3, BTech CS+MS(CL)
IIIT-Hyderabad
http://commandlinewani.blogspot.com
10:15 a.m.
New subject: [libvirt] [PATCHv3 1/4] net-dhcp-leases: Implement the public APIs
PFA: Diff
On Mon, Sep 16, 2013 at 8:44 PM, Nehal J Wani <nehaljw.kkd1(a)gmail.com> wrote:
On Mon, Sep 16, 2013 at 3:22 PM, Daniel P. Berrange
<berrange(a)redhat.com> wrote:
>
> On Mon, Sep 16, 2013 at 11:19:13AM +0530, Nehal J Wani wrote:
> > +int
> > +virNetworkGetDHCPLeasesForMAC(virNetworkPtr network,
> > + const char *mac,
> > + virNetworkDHCPLeasesPtr **leases,
> > + unsigned int flags)
> > +{
> > + virConnectPtr conn;
> > + virMacAddr addr;
> > +
> > + VIR_DEBUG("network=%p, mac=%s, leases=%p, flags=%x",
> > + network, mac, leases, flags);
> > +
> > + virResetLastError();
> > +
> > + virCheckNonNullArgGoto(network, error);
> > + virCheckNonNullArgGoto(mac, error);
> > +
> > + if (leases)
> > + *leases = NULL;
> > +
> > + if (!VIR_IS_CONNECTED_NETWORK(network)) {
> > + virLibNetworkError(VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> > + virDispatchError(NULL);
> > + return -1;
> > + }
> > +
> > + /* Validate the MAC address */
> > + if (mac && virMacAddrParse(mac, &addr) < 0) {
> > + virReportInvalidArg(mac,
> > + _("Given MAC Address doesn't comply
"
> > + "with the standard (IEEE 802) format in
%s"),
> > + __FUNCTION__);
>
> Don't pass __FUNCTION__ in this error message - that is already done
automatically
>
> > + goto error;
> > + }
>
> 'mac' should be a mandatory parameter here.
Attached diff should fix it.
But there are still calls to virReportInvalidArg() with __FUNCTION__
as one of its arguments in src/libvirt.c
>
> > +
> > + conn = network->conn;
> > +
> > + if (conn->networkDriver &&
> > + conn->networkDriver->networkGetDHCPLeasesForMAC) {
> > + int ret;
> > + ret = conn->networkDriver->networkGetDHCPLeasesForMAC(network,
mac,
> > + leases, flags);
> > + if (ret < 0)
> > + goto error;
> > + return ret;
> > + }
> > +
> > + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> > +
> > +error:
> > + virDispatchError(network->conn);
> > + return -1;
> > +}
>
> Daniel
> --
> |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org -o- http://virt-manager.org :|
> |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
Nehal J Wani
UG3, BTech CS+MS(CL)
IIIT-Hyderabad
http://commandlinewani.blogspot.com
--
Nehal J Wani
UG3, BTech CS+MS(CL)
IIIT-Hyderabad
http://commandlinewani.blogspot.com
10:25 a.m.
New subject: [libvirt] [PATCHv3 1/4] net-dhcp-leases: Implement the public APIs
On Mon, Sep 16, 2013 at 08:45:09PM +0530, Nehal J Wani wrote:
PFA: Diff
On Mon, Sep 16, 2013 at 8:44 PM, Nehal J Wani <nehaljw.kkd1(a)gmail.com> wrote:
> On Mon, Sep 16, 2013 at 3:22 PM, Daniel P. Berrange <berrange(a)redhat.com>
wrote:
>>
>> On Mon, Sep 16, 2013 at 11:19:13AM +0530, Nehal J Wani wrote:
>> > +int
>> > +virNetworkGetDHCPLeasesForMAC(virNetworkPtr network,
>> > + const char *mac,
>> > + virNetworkDHCPLeasesPtr **leases,
>> > + unsigned int flags)
>> > +{
>> > + virConnectPtr conn;
>> > + virMacAddr addr;
>> > +
>> > + VIR_DEBUG("network=%p, mac=%s, leases=%p, flags=%x",
>> > + network, mac, leases, flags);
>> > +
>> > + virResetLastError();
>> > +
>> > + virCheckNonNullArgGoto(network, error);
>> > + virCheckNonNullArgGoto(mac, error);
>> > +
>> > + if (leases)
>> > + *leases = NULL;
>> > +
>> > + if (!VIR_IS_CONNECTED_NETWORK(network)) {
>> > + virLibNetworkError(VIR_ERR_INVALID_NETWORK, __FUNCTION__);
>> > + virDispatchError(NULL);
>> > + return -1;
>> > + }
>> > +
>> > + /* Validate the MAC address */
>> > + if (mac && virMacAddrParse(mac, &addr) < 0) {
>> > + virReportInvalidArg(mac,
>> > + _("Given MAC Address doesn't comply
"
>> > + "with the standard (IEEE 802) format in
%s"),
>> > + __FUNCTION__);
>>
>> Don't pass __FUNCTION__ in this error message - that is already done
automatically
>>
>> > + goto error;
>> > + }
>>
>> 'mac' should be a mandatory parameter here.
>
> Attached diff should fix it.
No, it makes it worse - you're now allowing a NULL mac parameter
to be passed to virMacAddrParse which will cause a segv. You need
to be checking for mac == NULL with virCheckNonNullArg
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
10:27 a.m.
New subject: [libvirt] [PATCHv3 1/4] net-dhcp-leases: Implement the public APIs
On Mon, Sep 16, 2013 at 8:55 PM, Daniel P. Berrange <berrange(a)redhat.com> wrote:
On Mon, Sep 16, 2013 at 08:45:09PM +0530, Nehal J Wani wrote:
> PFA: Diff
>
> On Mon, Sep 16, 2013 at 8:44 PM, Nehal J Wani <nehaljw.kkd1(a)gmail.com> wrote:
> > On Mon, Sep 16, 2013 at 3:22 PM, Daniel P. Berrange <berrange(a)redhat.com>
wrote:
> >>
> >> On Mon, Sep 16, 2013 at 11:19:13AM +0530, Nehal J Wani wrote:
> >> > +int
> >> > +virNetworkGetDHCPLeasesForMAC(virNetworkPtr network,
> >> > + const char *mac,
> >> > + virNetworkDHCPLeasesPtr **leases,
> >> > + unsigned int flags)
> >> > +{
> >> > + virConnectPtr conn;
> >> > + virMacAddr addr;
> >> > +
> >> > + VIR_DEBUG("network=%p, mac=%s, leases=%p, flags=%x",
> >> > + network, mac, leases, flags);
> >> > +
> >> > + virResetLastError();
> >> > +
> >> > + virCheckNonNullArgGoto(network, error);
> >> > + virCheckNonNullArgGoto(mac, error);
> >> > +
> >> > + if (leases)
> >> > + *leases = NULL;
> >> > +
> >> > + if (!VIR_IS_CONNECTED_NETWORK(network)) {
> >> > + virLibNetworkError(VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> >> > + virDispatchError(NULL);
> >> > + return -1;
> >> > + }
> >> > +
> >> > + /* Validate the MAC address */
> >> > + if (mac && virMacAddrParse(mac, &addr) < 0) {
> >> > + virReportInvalidArg(mac,
> >> > + _("Given MAC Address doesn't
comply "
> >> > + "with the standard (IEEE 802)
format in %s"),
> >> > + __FUNCTION__);
> >>
> >> Don't pass __FUNCTION__ in this error message - that is already done
automatically
> >>
> >> > + goto error;
> >> > + }
> >>
> >> 'mac' should be a mandatory parameter here.
> >
> > Attached diff should fix it.
No, it makes it worse - you're now allowing a NULL mac parameter
to be passed to virMacAddrParse which will cause a segv. You need
to be checking for mac == NULL with virCheckNonNullArg
I think you missed this, I am already using virCheckNonNullArg :
+ virConnectPtr conn;
+ virMacAddr addr;
+
+ VIR_DEBUG("network=%p, mac=%s, leases=%p, flags=%x",
+ network, mac, leases, flags);
+
+ virResetLastError();
+
+ virCheckNonNullArgGoto(network, error);
+ virCheckNonNullArgGoto(mac, error);
+
+ if (leases)
+ *leases = NULL;
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
Nehal J Wani
UG3, BTech CS+MS(CL)
IIIT-Hyderabad
http://commandlinewani.blogspot.com
12:36 p.m.
New subject: [libvirt] [PATCHv3 1/4] net-dhcp-leases: Implement the public APIs
On 09/15/2013 11:49 PM, Nehal J Wani wrote:
Introduce 3 new APIs, virNetworkGetDHCPLeases,
virNetworkGetDHCPLeasesForMAC
and virNetworkDHCPLeaseFree.
+
+typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases;
+typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr;
+struct _virNetworkDHCPLeases {
+ long long expirytime;
In what unit? Seconds since Epoch?
+
+/**
+ * virNetworkGetDHCPLeases:
+ * @network: pointer to network object
+ * @leases: pointer to an array of pointers pointing to the obtained leases
Try to make it more clear that this is an output parameter. Copying
from virConnectListAllDomains, I'd try something like:
@leases: Pointer to a variable to store the array containing details on
obtained leases, or NULL if the list is not requires (just returns
number of leases)
+ * @flags: extra flags, not used yet, so callers should always pass
0
+ *
+ * The API returns lease info for all network interfaces connected to the
+ * given virtual network.
+ *
+ * Returns the number of leases found or -1 and sets @leases to NULL in case
+ * of error. On success, the array stored into @leases 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
+ * virNetworkDHCPLeaseFree on each array element, then calling free() on @leases.
I'm not sure if writing virNetworkDHCPLeaseFree() (with the () at the
end) will make cross-referencing any nicer in the generated html docs,
but it can't hurt.
+ *
+ * for (i = 0; i < nleases; i++) {
+ * virNetworkDHCPLeasesPtr lease = leases[i];
+ * printf("Time(epoch): %lu, MAC address: %s, "
+ * "IP address: %s, Hostname: %s, ClientID: %s\n",
+ * leas->expirytime, lease->mac, lease->ipaddr,
+ * lease->hostname, lease->clientid);
If you stick 'virNetworkDHCPLeaseFree(lease);' here,...
+ * }
+ *
+ * if (leases) {
+ * for (i = 0; i < nleases; i++)
+ * virNetworkDHCPLeaseFree(leases[i]);
+ * }
...you can omit this loop entirely.
+ * free(leases);
+ *
+ */
+int
+virNetworkGetDHCPLeases(virNetworkPtr network,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags)
+/**
+ * virNetworkGetDHCPLeasesForMAC:
+ * @network: pointer to network object
+ * @mac: MAC Address of an interface
Maybe make it clear that this is the stringized form:
@mac: ASCII formatted MAC address of an interface
+ * @leases: pointer to an array of pointers pointing to the obtained
leases
Same suggestion as above.
+virNetworkGetDHCPLeasesForMAC(virNetworkPtr network,
+ const char *mac,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags)
+{
+ virConnectPtr conn;
+ virMacAddr addr;
+
+ VIR_DEBUG("network=%p, mac=%s, leases=%p, flags=%x",
+ network, mac, leases, flags);
+
+ virResetLastError();
+
+ virCheckNonNullArgGoto(network, error);
This check is redundant...[1]
+ virCheckNonNullArgGoto(mac, error);
This check...[2]
+
+ if (leases)
+ *leases = NULL;
[2]...should be moved after this, so that we guarantee to set *leases on
ALL possible errors.
+
+ if (!VIR_IS_CONNECTED_NETWORK(network)) {
[1]...with this. (Same problem with the other function, too).
+ virLibNetworkError(VIR_ERR_INVALID_NETWORK, __FUNCTION__);
+ virDispatchError(NULL);
+ return -1;
+ }
+
+ /* Validate the MAC address */
+ if (mac && virMacAddrParse(mac, &addr) < 0) {
You already required non-NULL mac, so 'mac &&' is bogus.
+++ b/src/libvirt_public.syms
@@ -634,4 +634,11 @@ LIBVIRT_1.1.1 {
virDomainSetMemoryStatsPeriod;
} LIBVIRT_1.1.0;
+LIBVIRT_1.1.3 {
+ global:
+ virNetworkGetDHCPLeases;
+ virNetworkGetDHCPLeasesForMAC;
+ virNetworkDHCPLeaseFree;
Not strictly required, but I like sorting symbols in this listing.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:49 a.m.
New subject: [libvirt] [PATCHv3 2/4] net-dhcp-leases: Implement the remote protocol
Implement RPC calls for virNetworkGetDHCPLeases, virNetworkGetDHCPLeasesForMAC
daemon/remote.c
* Define remoteSerializeNetworkDHCPLeases,
remoteDispatchNetworkGetDHCPLeases
* Define remoteDispatchNetworkGetDHCPLeasesForMAC
src/remote/remote_driver.c
* Define remoteNetworkGetDHCPLeases
* Define remoteNetworkGetDHCPLeasesForMAC
src/remote/remote_protocol.x
* New RPC procedure: REMOTE_PROC_NETWORK_GET_DHCP_LEASES
* Define structs remote_network_dhcp_leases, remote_network_get_dhcp_leases_args,
remote_network_get_dhcp_leases_ret
* New RPC procedure: REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC
* Define structs remote_network_dhcp_leases_for_mac,
remote_network_get_dhcp_leases_for_mac_args,
remote_network_get_dhcp_leases_for_mac_ret
src/remote_protocol-structs
* New structs added
src/rpc/gendispatch.pl
* Add exception (s/Dhcp/DHCP) for auto-generating names of the remote functions
in daemon/remote_dispatch.h
---
daemon/remote.c | 133 ++++++++++++++++++++++++++++++++++++++
src/remote/remote_driver.c | 150 +++++++++++++++++++++++++++++++++++++++++++
src/remote/remote_protocol.x | 50 ++++++++++++++-
src/remote_protocol-structs | 32 +++++++++
src/rpc/gendispatch.pl | 1 +
5 files changed, 365 insertions(+), 1 deletion(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 2aff7c1..5252893 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -5137,7 +5137,140 @@ cleanup:
return rv;
}
+static int
+remoteSerializeNetworkDHCPLeases(virNetworkDHCPLeasesPtr *leases,
+ int nleases,
+ remote_network_get_dhcp_leases_ret *ret)
+{
+ size_t i;
+
+ if (nleases > REMOTE_NETWORK_DHCP_LEASES_MAX) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Number of leases is %d, which exceeds max limit:
%d"),
+ nleases, REMOTE_NETWORK_DHCP_LEASES_MAX);
+ return -1;
+ }
+
+ if (VIR_ALLOC_N(ret->leases.leases_val, nleases) < 0)
+ return -1;
+
+ ret->leases.leases_len = nleases;
+
+ for (i = 0; i < nleases; i++) {
+ virNetworkDHCPLeasesPtr lease = leases[i];
+ remote_network_dhcp_lease *lease_ret = &(ret->leases.leases_val[i]);
+ lease_ret->expirytime = lease->expirytime;
+ lease_ret->type = lease->type;
+ lease_ret->prefix = lease->prefix;
+
+ if ((VIR_STRDUP(lease_ret->mac, lease->mac) < 0) ||
+ (VIR_STRDUP(lease_ret->ipaddr, lease->ipaddr) < 0) ||
+ (VIR_STRDUP(lease_ret->hostname, lease->hostname) < 0) ||
+ (VIR_STRDUP(lease_ret->clientid, lease->clientid) < 0))
+ goto error;
+ }
+
+ return 0;
+
+error:
+ for (i = 0; i < nleases; i++) {
+ remote_network_dhcp_lease *lease_ret = &(ret->leases.leases_val[i]);
+ virNetworkDHCPLeaseFree((virNetworkDHCPLeasesPtr)lease_ret);
+ }
+ return -1;
+}
+
+static int
+remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_network_get_dhcp_leases_args *args,
+ remote_network_get_dhcp_leases_ret *ret)
+{
+ int rv = -1;
+ struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
+ virNetworkDHCPLeasesPtr *leases = NULL;
+ virNetworkPtr net = NULL;
+ int nleases = 0;
+
+ if (!priv->conn) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
+ goto cleanup;
+ }
+
+ if (!(net = get_nonnull_network(priv->conn, args->net)))
+ goto cleanup;
+
+ if ((nleases = virNetworkGetDHCPLeases(net, &leases, args->flags)) < 0)
+ goto cleanup;
+
+ if (remoteSerializeNetworkDHCPLeases(leases, nleases, ret) < 0)
+ goto cleanup;
+ rv = nleases;
+
+cleanup:
+ if (rv < 0)
+ virNetMessageSaveError(rerr);
+ if (leases) {
+ size_t i;
+ for (i = 0; i < nleases; i++) {
+ virNetworkDHCPLeaseFree(leases[i]);
+ }
+ }
+ VIR_FREE(leases);
+ virNetworkFree(net);
+ return rv;
+}
+
+static int
+remoteDispatchNetworkGetDHCPLeasesForMAC(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_network_get_dhcp_leases_for_mac_args
*args,
+ remote_network_get_dhcp_leases_for_mac_ret
*ret)
+{
+ int rv = -1;
+ struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
+ virNetworkDHCPLeasesPtr *leases = NULL;
+ virNetworkPtr net = NULL;
+ int nleases = 0;
+ const char *mac = NULL;
+
+ if (!priv->conn) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
+ goto cleanup;
+ }
+
+ if (!(net = get_nonnull_network(priv->conn, args->net)))
+ goto cleanup;
+
+ mac = args->mac ? *args->mac : NULL;
+
+ if ((nleases = virNetworkGetDHCPLeasesForMAC(net, mac, &leases, args->flags))
< 0)
+ goto cleanup;
+
+ if (remoteSerializeNetworkDHCPLeases(leases, nleases,
+ (remote_network_get_dhcp_leases_ret *)ret) <
0)
+ goto cleanup;
+
+ rv = nleases;
+
+cleanup:
+ if (rv < 0)
+ virNetMessageSaveError(rerr);
+ if (leases) {
+ size_t i;
+ for (i = 0; i < nleases; i++) {
+ virNetworkDHCPLeaseFree(leases[i]);
+ }
+ }
+ VIR_FREE(leases);
+ virNetworkFree(net);
+ return rv;
+}
/*----- Helpers. -----*/
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 62e77a5..92ac8ef 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -6599,6 +6599,154 @@ done:
return rv;
}
+static int
+remoteNetworkGetDHCPLeasesForMAC(virNetworkPtr net,
+ const char *mac,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags)
+{
+ int rv = -1;
+ size_t i;
+ struct private_data *priv = net->conn->networkPrivateData;
+ remote_network_get_dhcp_leases_for_mac_args args;
+ remote_network_get_dhcp_leases_for_mac_ret ret;
+
+ virNetworkDHCPLeasesPtr *leases_ret;
+ remoteDriverLock(priv);
+
+ make_nonnull_network(&args.net, net);
+ args.mac = mac ? (char **)&mac : NULL;
+ args.flags = flags;
+
+ memset(&ret, 0, sizeof(ret));
+
+ if (call(net->conn, priv, 0, REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC,
+ (xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_args, (char
*)&args,
+ (xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_ret, (char *)&ret)
== -1) {
+ goto done;
+ }
+
+ if (ret.leases.leases_len > REMOTE_NETWORK_DHCP_LEASES_MAX) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Number of leases is %d, which exceeds max limit:
%d"),
+ ret.leases.leases_len, REMOTE_NETWORK_DHCP_LEASES_MAX);
+ return -1;
+ goto cleanup;
+ }
+
+ if (ret.leases.leases_len &&
+ VIR_ALLOC_N(leases_ret, ret.leases.leases_len + 1) < 0) {
+ goto cleanup;
+ }
+
+ for (i = 0; i < ret.leases.leases_len; i++) {
+ if (VIR_ALLOC(leases_ret[i]) < 0)
+ goto cleanup;
+
+ virNetworkDHCPLeasesPtr lease = leases_ret[i];
+ remote_network_dhcp_lease *lease_ret = &(ret.leases.leases_val[i]);
+ lease->expirytime = lease_ret->expirytime;
+ lease->type = lease_ret->type;
+ lease->prefix = lease_ret->prefix;
+
+ if ((VIR_STRDUP(lease->mac, lease_ret->mac) < 0) ||
+ (VIR_STRDUP(lease->ipaddr, lease_ret->ipaddr) < 0) ||
+ (VIR_STRDUP(lease->hostname, lease_ret->hostname) < 0) ||
+ (VIR_STRDUP(lease->clientid, lease_ret->clientid) < 0))
+ goto cleanup;
+ }
+
+ *leases = leases_ret;
+ leases_ret = NULL;
+ rv = ret.leases.leases_len;
+
+cleanup:
+ if (leases_ret) {
+ for (i = 0; i < ret.leases.leases_len; i++)
+ virNetworkDHCPLeaseFree(leases_ret[i]);
+ }
+ VIR_FREE(leases_ret);
+ xdr_free((xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_ret,
+ (char *) &ret);
+
+done:
+ remoteDriverUnlock(priv);
+ return rv;
+}
+
+static int
+remoteNetworkGetDHCPLeases(virNetworkPtr net,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags)
+{
+ int rv = -1;
+ size_t i;
+ struct private_data *priv = net->conn->networkPrivateData;
+ remote_network_get_dhcp_leases_args args;
+ remote_network_get_dhcp_leases_ret ret;
+
+ virNetworkDHCPLeasesPtr *leases_ret;
+ remoteDriverLock(priv);
+
+ make_nonnull_network(&args.net, net);
+ args.flags = flags;
+
+ memset(&ret, 0, sizeof(ret));
+
+ if (call(net->conn, priv, 0, REMOTE_PROC_NETWORK_GET_DHCP_LEASES,
+ (xdrproc_t)xdr_remote_network_get_dhcp_leases_args, (char *)&args,
+ (xdrproc_t)xdr_remote_network_get_dhcp_leases_ret, (char *)&ret) == -1)
{
+ goto done;
+ }
+
+ if (ret.leases.leases_len > REMOTE_NETWORK_DHCP_LEASES_MAX) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Number of leases is %d, which exceeds max limit:
%d"),
+ ret.leases.leases_len, REMOTE_NETWORK_DHCP_LEASES_MAX);
+ return -1;
+ goto cleanup;
+ }
+
+ if (ret.leases.leases_len &&
+ VIR_ALLOC_N(leases_ret, ret.leases.leases_len + 1) < 0) {
+ goto cleanup;
+ }
+
+ for (i = 0; i < ret.leases.leases_len; i++) {
+ if (VIR_ALLOC(leases_ret[i]) < 0)
+ goto cleanup;
+
+ virNetworkDHCPLeasesPtr lease = leases_ret[i];
+ remote_network_dhcp_lease *lease_ret = &(ret.leases.leases_val[i]);
+ lease->expirytime = lease_ret->expirytime;
+ lease->type = lease_ret->type;
+ lease->prefix = lease_ret->prefix;
+
+ if ((VIR_STRDUP(lease->mac, lease_ret->mac) < 0) ||
+ (VIR_STRDUP(lease->ipaddr, lease_ret->ipaddr) < 0) ||
+ (VIR_STRDUP(lease->hostname, lease_ret->hostname) < 0) ||
+ (VIR_STRDUP(lease->clientid, lease_ret->clientid) < 0))
+ goto cleanup;
+ }
+
+ *leases = leases_ret;
+ leases_ret = NULL;
+ rv = ret.leases.leases_len;
+
+cleanup:
+ if (leases_ret) {
+ for (i = 0; i < ret.leases.leases_len; i++)
+ virNetworkDHCPLeaseFree(leases_ret[i]);
+ }
+ VIR_FREE(leases_ret);
+ xdr_free((xdrproc_t)xdr_remote_network_get_dhcp_leases_ret,
+ (char *) &ret);
+
+done:
+ remoteDriverUnlock(priv);
+ return rv;
+}
+
static void
remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event)
{
@@ -6958,6 +7106,8 @@ static virNetworkDriver network_driver = {
.networkSetAutostart = remoteNetworkSetAutostart, /* 0.3.0 */
.networkIsActive = remoteNetworkIsActive, /* 0.7.3 */
.networkIsPersistent = remoteNetworkIsPersistent, /* 0.7.3 */
+ .networkGetDHCPLeases = remoteNetworkGetDHCPLeases, /* 1.1.3 */
+ .networkGetDHCPLeasesForMAC = remoteNetworkGetDHCPLeasesForMAC, /* 1.1.3 */
};
static virInterfaceDriver interface_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 85ad9ba..a29646b 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -232,6 +232,11 @@ const REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX = 64;
/* Upper limit on number of job stats */
const REMOTE_DOMAIN_JOB_STATS_MAX = 16;
+/*
+ * Upper limit on the maximum number of leases in one lease file
+ */
+const REMOTE_NETWORK_DHCP_LEASES_MAX = 32768;
+
/* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */
typedef opaque remote_uuid[VIR_UUID_BUFLEN];
@@ -2835,6 +2840,35 @@ struct remote_domain_event_device_removed_msg {
remote_nonnull_string devAlias;
};
+struct remote_network_dhcp_lease {
+ hyper expirytime;
+ remote_nonnull_string mac;
+ remote_nonnull_string ipaddr;
+ remote_nonnull_string hostname;
+ remote_nonnull_string clientid;
+ int type;
+ unsigned int prefix;
+};
+
+struct remote_network_get_dhcp_leases_args {
+ remote_nonnull_network net;
+ unsigned int flags;
+};
+
+struct remote_network_get_dhcp_leases_ret {
+ remote_network_dhcp_lease leases<REMOTE_NETWORK_DHCP_LEASES_MAX>;
+};
+
+struct remote_network_get_dhcp_leases_for_mac_args {
+ remote_nonnull_network net;
+ remote_string mac;
+ unsigned int flags;
+};
+
+struct remote_network_get_dhcp_leases_for_mac_ret {
+ remote_network_dhcp_lease leases<REMOTE_NETWORK_DHCP_LEASES_MAX>;
+};
+
/*----- Protocol. -----*/
/* Define the program number, protocol version and procedure numbers here. */
@@ -4998,5 +5032,19 @@ enum remote_procedure {
* @generate: both
* @acl: none
*/
- REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311
+ REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311,
+
+ /**
+ * @generate: none
+ * @priority: high
+ * @acl: network:read
+ */
+ REMOTE_PROC_NETWORK_GET_DHCP_LEASES = 312,
+
+ /**
+ * @generate: none
+ * @priority: high
+ * @acl: network:read
+ */
+ REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC = 313
};
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 4e27aae..9b625e8 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -2316,6 +2316,36 @@ struct remote_domain_event_device_removed_msg {
remote_nonnull_domain dom;
remote_nonnull_string devAlias;
};
+struct remote_network_dhcp_lease {
+ int64_t expirytime;
+ remote_nonnull_string mac;
+ remote_nonnull_string ipaddr;
+ remote_nonnull_string hostname;
+ remote_nonnull_string clientid;
+ int type;
+ u_int prefix;
+};
+struct remote_network_get_dhcp_leases_args {
+ remote_nonnull_network net;
+ u_int flags;
+};
+struct remote_network_get_dhcp_leases_ret {
+ struct {
+ u_int leases_len;
+ remote_network_dhcp_lease * leases_val;
+ } leases;
+};
+struct remote_network_get_dhcp_leases_for_mac_args {
+ remote_nonnull_network net;
+ remote_string mac;
+ u_int flags;
+};
+struct remote_network_get_dhcp_leases_for_mac_ret {
+ struct {
+ u_int leases_len;
+ remote_network_dhcp_lease * leases_val;
+ } leases;
+};
enum remote_procedure {
REMOTE_PROC_CONNECT_OPEN = 1,
REMOTE_PROC_CONNECT_CLOSE = 2,
@@ -2628,4 +2658,6 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_CREATE_XML_WITH_FILES = 309,
REMOTE_PROC_DOMAIN_CREATE_WITH_FILES = 310,
REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311,
+ REMOTE_PROC_NETWORK_GET_DHCP_LEASES = 312,
+ REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC = 313,
};
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index ceb1ad8..5a503cf 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -66,6 +66,7 @@ sub fixup_name {
$name =~ s/Fstrim$/FSTrim/;
$name =~ s/Scsi/SCSI/;
$name =~ s/Wwn$/WWN/;
+ $name =~ s/Dhcp$/DHCP/;
return $name;
}
--
1.7.11.7
1:01 p.m.
New subject: [libvirt] [PATCHv3 2/4] net-dhcp-leases: Implement the remote protocol
+static int
+remoteSerializeNetworkDHCPLeases(virNetworkDHCPLeasesPtr *leases,
+ int nleases,
+ remote_network_get_dhcp_leases_ret *ret)
+{
+ size_t i;
+
+ if (nleases > REMOTE_NETWORK_DHCP_LEASES_MAX) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Number of leases is %d, which exceeds max
limit:
%d"),
+ nleases, REMOTE_NETWORK_DHCP_LEASES_MAX);
+ return -1;
+ }
+
+ if (VIR_ALLOC_N(ret->leases.leases_val, nleases) < 0)
+ return -1;
+
+ ret->leases.leases_len = nleases;
+
+ for (i = 0; i < nleases; i++) {
+ virNetworkDHCPLeasesPtr lease = leases[i];
+ remote_network_dhcp_lease *lease_ret =
&(ret->leases.leases_val[i]);
+ lease_ret->expirytime = lease->expirytime;
+ lease_ret->type = lease->type;
+ lease_ret->prefix = lease->prefix;
+
+ if ((VIR_STRDUP(lease_ret->mac, lease->mac) < 0) ||
+ (VIR_STRDUP(lease_ret->ipaddr, lease->ipaddr) < 0) ||
+ (VIR_STRDUP(lease_ret->hostname, lease->hostname) < 0) ||
+ (VIR_STRDUP(lease_ret->clientid, lease->clientid) < 0))
+ goto error;
+ }
+
+ return 0;
+
+error:
+ for (i = 0; i < nleases; i++) {
+ remote_network_dhcp_lease *lease_ret =
&(ret->leases.leases_val[i]);
+ virNetworkDHCPLeaseFree((virNetworkDHCPLeasesPtr)lease_ret);
[1]
+ }
+ return -1;
+}
+
+static int
+remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server
ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_network_get_dhcp_leases_args
*args,
+
remote_network_get_dhcp_leases_ret
*ret)
+{
+ int rv = -1;
+ struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
+ virNetworkDHCPLeasesPtr *leases = NULL;
+ virNetworkPtr net = NULL;
+ int nleases = 0;
+
+ if (!priv->conn) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
+ goto cleanup;
+ }
+
+ if (!(net = get_nonnull_network(priv->conn, args->net)))
+ goto cleanup;
+
+ if ((nleases = virNetworkGetDHCPLeases(net, &leases, args->flags)) <
0)
+ goto cleanup;
+
+ if (remoteSerializeNetworkDHCPLeases(leases, nleases, ret) < 0)
+ goto cleanup;
+ rv = nleases;
+
+cleanup:
+ if (rv < 0)
+ virNetMessageSaveError(rerr);
+ if (leases) {
+ size_t i;
+ for (i = 0; i < nleases; i++) {
+ virNetworkDHCPLeaseFree(leases[i]);
+ }
+ }
+ VIR_FREE(leases);
+ virNetworkFree(net);
+ return rv;
+}
+
+static int
+remoteDispatchNetworkGetDHCPLeasesForMAC(virNetServerPtr server
ATTRIBUTE_UNUSED,
+ virNetServerClientPtr
client,
+ virNetMessagePtr msg
ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr
rerr,
+
remote_network_get_dhcp_leases_for_mac_args *args,
+
remote_network_get_dhcp_leases_for_mac_ret *ret)
+{
+ int rv = -1;
+ struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
> + virNetworkDHCPLeasesPtr *leases = NULL;
> + virNetworkPtr net = NULL;
> + int nleases = 0;
> + const char *mac = NULL;
+
> + if (!priv->conn) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection
not
open"));
> + goto cleanup;
> + }
+
> + if (!(net = get_nonnull_network(priv->conn,
args->net)))
> + goto cleanup;
+
> + mac = args->mac ? *args->mac : NULL;
+
> + if ((nleases = virNetworkGetDHCPLeasesForMAC(net,
mac, &leases,
args->flags)) < 0)
> + goto cleanup;
+
> + if (remoteSerializeNetworkDHCPLeases(leases,
nleases,
+
(remote_network_get_dhcp_leases_ret *)ret) < 0)
[2]
> + goto cleanup;
+
> + rv = nleases;
+
I am a little skeptical about the typecasts involved in [1] and [2]
Specifically for [2], although the APIs are different, the struct is same,
only the name is different. But, what would be the other options?
One option: (Suggested by Osier)
Change remoteSerializeNetworkDHCPLeases to
+remoteSerializeNetworkDHCPLeases(virNetworkDHCPLeasesPtr *leases,
+ int nleases,
+ (void *) ret,
+ int method)
where the datatype for variable in which ret will be copied, will be
decided based on the method (method's value will be taken from an enum
or something similar, e.g: 1->DHCPLeases, 2->DHCPLeasesForMAC)
OR
Second Option: Since the APIs should be independent, make another function:
remoteSerializeNetworkDHCPLeasesForMAC(...), but that will lead to code
redundancy.
Eric, what would you do in these cases?
1:15 p.m.
New subject: [libvirt] [PATCHv3 2/4] net-dhcp-leases: Implement the remote protocol
On Mon, Sep 16, 2013 at 11:31:27PM +0530, Nehal J Wani wrote:
> +static int
> +remoteSerializeNetworkDHCPLeases(virNetworkDHCPLeasesPtr *leases,
> + int nleases,
> + remote_network_get_dhcp_leases_ret *ret)
> +{
> + size_t i;
> +
> + if (nleases > REMOTE_NETWORK_DHCP_LEASES_MAX) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Number of leases is %d, which exceeds max
limit: %d"),
Please make your email client not mangle line breaks when you are
replying. It makes it really hard to follow quoted text.
> + nleases,
REMOTE_NETWORK_DHCP_LEASES_MAX);
> + return -1;
> + }
> +
> + if (VIR_ALLOC_N(ret->leases.leases_val, nleases) < 0)
> + return -1;
> +
> + ret->leases.leases_len = nleases;
> +
> + for (i = 0; i < nleases; i++) {
> + virNetworkDHCPLeasesPtr lease = leases[i];
> + remote_network_dhcp_lease *lease_ret =
&(ret->leases.leases_val[i]);
> + lease_ret->expirytime = lease->expirytime;
> + lease_ret->type = lease->type;
> + lease_ret->prefix = lease->prefix;
> +
> + if ((VIR_STRDUP(lease_ret->mac, lease->mac) < 0) ||
> + (VIR_STRDUP(lease_ret->ipaddr, lease->ipaddr) < 0) ||
> + (VIR_STRDUP(lease_ret->hostname, lease->hostname) < 0) ||
> + (VIR_STRDUP(lease_ret->clientid, lease->clientid) < 0))
> + goto error;
> + }
> +
> + return 0;
> +
> +error:
> + for (i = 0; i < nleases; i++) {
> + remote_network_dhcp_lease *lease_ret =
&(ret->leases.leases_val[i]);
> + virNetworkDHCPLeaseFree((virNetworkDHCPLeasesPtr)lease_ret);
[1]
> + }
> + return -1;
> +}
> +
> +static int
> +remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server
ATTRIBUTE_UNUSED,
> + virNetServerClientPtr client,
> + virNetMessagePtr msg ATTRIBUTE_UNUSED,
> + virNetMessageErrorPtr rerr,
> + remote_network_get_dhcp_leases_args
*args,
> + remote_network_get_dhcp_leases_ret
*ret)
> +{
> + int rv = -1;
> + struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
> + virNetworkDHCPLeasesPtr *leases = NULL;
> + virNetworkPtr net = NULL;
> + int nleases = 0;
> +
> + if (!priv->conn) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection
not
open"));
> + goto cleanup;
> + }
> +
> + if (!(net = get_nonnull_network(priv->conn, args->net)))
> + goto cleanup;
> +
> + if ((nleases = virNetworkGetDHCPLeases(net, &leases, args->flags)) <
0)
> + goto cleanup;
> +
> + if (remoteSerializeNetworkDHCPLeases(leases, nleases, ret) < 0)
> + goto cleanup;
>
> + rv = nleases;
> +
> +cleanup:
> + if (rv < 0)
> + virNetMessageSaveError(rerr);
> + if (leases) {
> + size_t i;
> + for (i = 0; i < nleases; i++) {
> + virNetworkDHCPLeaseFree(leases[i]);
> + }
> + }
> + VIR_FREE(leases);
> + virNetworkFree(net);
> + return rv;
> +}
> +
> +static int
> +remoteDispatchNetworkGetDHCPLeasesForMAC(virNetServerPtr server
ATTRIBUTE_UNUSED,
> + virNetServerClientPtr client,
> + virNetMessagePtr msg
ATTRIBUTE_UNUSED,
> + virNetMessageErrorPtr rerr,
> +
remote_network_get_dhcp_leases_for_mac_args *args,
> +
remote_network_get_dhcp_leases_for_mac_ret *ret)
> +{
> + int rv = -1;
> + struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
> + virNetworkDHCPLeasesPtr *leases = NULL;
> + virNetworkPtr net = NULL;
> + int nleases = 0;
> + const char *mac = NULL;
> +
> + if (!priv->conn) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection
not
open"));
> + goto cleanup;
> + }
> +
> + if (!(net = get_nonnull_network(priv->conn, args->net)))
> + goto cleanup;
> +
> + mac = args->mac ? *args->mac : NULL;
> +
> + if ((nleases = virNetworkGetDHCPLeasesForMAC(net, mac, &leases,
args->flags)) < 0)
> + goto cleanup;
> +
> + if (remoteSerializeNetworkDHCPLeases(leases, nleases,
> +
(remote_network_get_dhcp_leases_ret *)ret) < 0)
[2]
> + goto cleanup;
> +
> + rv = nleases;
> +
I am a little skeptical about the typecasts involved in [1] and [2]
Specifically for [2], although the APIs are different, the struct is same,
only the name is different. But, what would be the other options?
One option: (Suggested by Osier)
Change remoteSerializeNetworkDHCPLeases to
+remoteSerializeNetworkDHCPLeases(virNetworkDHCPLeasesPtr *leases,
+ int nleases,
+ (void *) ret,
+ int method)
where the datatype for variable in which ret will be copied, will be
decided based on the method (method's value will be taken from an enum
or something similar, e.g: 1->DHCPLeases, 2->DHCPLeasesForMAC)
OR
Second Option: Since the APIs should be independent, make another function:
remoteSerializeNetworkDHCPLeasesForMAC(...), but that will lead to code
redundancy.
The commonality between the methods is the 'remote_network_dhcp_lease'
struct. So instead of creating a remoteSerializeNetworkDHCPLeases
which serializes a list of leases. Make a helper method which just
serializes a single "remote_network_dhcp_lease" instance. Push the
list iteration code up into the parent methods.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:08 p.m.
New subject: [libvirt] [PATCHv3 2/4] net-dhcp-leases: Implement the remote protocol
On 09/16/2013 12:01 PM, Nehal J Wani wrote:
> +
> + for (i = 0; i < nleases; i++) {
> + virNetworkDHCPLeasesPtr lease = leases[i];
> + remote_network_dhcp_lease *lease_ret =
&(ret->leases.leases_val[i]);
> +error:
> + for (i = 0; i < nleases; i++) {
> + remote_network_dhcp_lease *lease_ret =
&(ret->leases.leases_val[i]);
> + virNetworkDHCPLeaseFree((virNetworkDHCPLeasesPtr)lease_ret);
[1]
Don't do that. lease_ret is NOT an abi-compatible struct with
virNetworkDHCPLeasesPtr. You'll have to manually free the individual
components, or use xdr_free().
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
1:33 p.m.
New subject: [libvirt] [PATCHv3 2/4] net-dhcp-leases: Implement the remote protocol
On 09/15/2013 11:49 PM, Nehal J Wani wrote:
Implement RPC calls for virNetworkGetDHCPLeases,
virNetworkGetDHCPLeasesForMAC
A lot of my comments below are similar to what I did on Guiseppe's
patch; you may want to closely study:
https://www.redhat.com/archives/libvir-list/2013-September/msg00808.html
diff --git a/daemon/remote.c b/daemon/remote.c
index 2aff7c1..5252893 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -5137,7 +5137,140 @@ cleanup:
return rv;
}
+static int
Lately, we've been preferring a style of 2 blank lines between functions.
+remoteSerializeNetworkDHCPLeases(virNetworkDHCPLeasesPtr *leases,
+ int nleases,
+ remote_network_get_dhcp_leases_ret *ret)
+{
+ size_t i;
+
+ if (nleases > REMOTE_NETWORK_DHCP_LEASES_MAX) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Number of leases is %d, which exceeds max limit:
%d"),
+ nleases, REMOTE_NETWORK_DHCP_LEASES_MAX);
+ return -1;
+ }
+
+ if (VIR_ALLOC_N(ret->leases.leases_val, nleases) < 0)
+ return -1;
+
+ ret->leases.leases_len = nleases;
I wouldn't set this...
+
+ for (i = 0; i < nleases; i++) {
+ virNetworkDHCPLeasesPtr lease = leases[i];
+ remote_network_dhcp_lease *lease_ret = &(ret->leases.leases_val[i]);
+ lease_ret->expirytime = lease->expirytime;
+ lease_ret->type = lease->type;
+ lease_ret->prefix = lease->prefix;
+
+ if ((VIR_STRDUP(lease_ret->mac, lease->mac) < 0) ||
+ (VIR_STRDUP(lease_ret->ipaddr, lease->ipaddr) < 0) ||
+ (VIR_STRDUP(lease_ret->hostname, lease->hostname) < 0) ||
+ (VIR_STRDUP(lease_ret->clientid, lease->clientid) < 0))
+ goto error;
+ }
...until here...
+
+ return 0;
+
+error:
+ for (i = 0; i < nleases; i++) {
+ remote_network_dhcp_lease *lease_ret = &(ret->leases.leases_val[i]);
+ virNetworkDHCPLeaseFree((virNetworkDHCPLeasesPtr)lease_ret);
+ }
...so that the caller doesn't try to iterate over bogus
remote_network_dhcp_lease data on error.
+ return -1;
+}
+
+static int
+remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_network_get_dhcp_leases_args *args,
+ remote_network_get_dhcp_leases_ret *ret)
+{
+ int rv = -1;
+ struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
+ virNetworkDHCPLeasesPtr *leases = NULL;
+ virNetworkPtr net = NULL;
+ int nleases = 0;
+
+ if (!priv->conn) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
+ goto cleanup;
+ }
+
+ if (!(net = get_nonnull_network(priv->conn, args->net)))
+ goto cleanup;
+
+ if ((nleases = virNetworkGetDHCPLeases(net, &leases, args->flags)) < 0)
This doesn't allow a user to efficiently request count-only semantics.
In comparison with our ListAll* apis, doing that would require the use
of an args->need_results member, where you can pass NULL instead of
&leases if the user doesn't care about the results.
+ goto cleanup;
+
+ if (remoteSerializeNetworkDHCPLeases(leases, nleases, ret) < 0)
+ goto cleanup;
+ rv = nleases;
The odd spacing of this non-patched blank line in the middle is the
result of you not using 2 blanks between functions. rv must be 0, not
positive, on success; the RPC client can reconstruct nleases from
ret->leases.leases_val. Then again, if you plan on supporting NULL
leases for count-only operation, you need to add ret->ret as a way to
pass the count in the absence of a return array.
+
+cleanup:
+ if (rv < 0)
+ virNetMessageSaveError(rerr);
+ if (leases) {
+ size_t i;
+ for (i = 0; i < nleases; i++) {
+ virNetworkDHCPLeaseFree(leases[i]);
+ }
+ }
+ VIR_FREE(leases);
+ virNetworkFree(net);
+ return rv;
+}
+
+static int
+remoteDispatchNetworkGetDHCPLeasesForMAC(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_network_get_dhcp_leases_for_mac_args
*args,
+ remote_network_get_dhcp_leases_for_mac_ret
*ret)
+{
+ int rv = -1;
+ struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
+ virNetworkDHCPLeasesPtr *leases = NULL;
+ virNetworkPtr net = NULL;
+ int nleases = 0;
+ const char *mac = NULL;
+
+ if (!priv->conn) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
+ goto cleanup;
+ }
+
+ if (!(net = get_nonnull_network(priv->conn, args->net)))
+ goto cleanup;
+
+ mac = args->mac ? *args->mac : NULL;
args->mac should be non-NULL (since you already reject NULL at the API
level).
+
+ if ((nleases = virNetworkGetDHCPLeasesForMAC(net, mac, &leases, args->flags))
< 0)
+ goto cleanup;
+
+ if (remoteSerializeNetworkDHCPLeases(leases, nleases,
+ (remote_network_get_dhcp_leases_ret *)ret) <
0)
+ goto cleanup;
+
+ rv = nleases;
rv must be 0 on success.
+
+cleanup:
+ if (rv < 0)
+ virNetMessageSaveError(rerr);
+ if (leases) {
+ size_t i;
+ for (i = 0; i < nleases; i++) {
+ virNetworkDHCPLeaseFree(leases[i]);
+ }
+ }
+ VIR_FREE(leases);
+ virNetworkFree(net);
+ return rv;
+}
/*----- Helpers. -----*/
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 62e77a5..92ac8ef 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -6599,6 +6599,154 @@ done:
return rv;
}
+static int
+remoteNetworkGetDHCPLeasesForMAC(virNetworkPtr net,
+ const char *mac,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags)
+{
Why is the ForMAC version first here, but last in all the other hunks
where you add the two functions?
+ int rv = -1;
+ size_t i;
+ struct private_data *priv = net->conn->networkPrivateData;
+ remote_network_get_dhcp_leases_for_mac_args args;
+ remote_network_get_dhcp_leases_for_mac_ret ret;
+
+ virNetworkDHCPLeasesPtr *leases_ret;
Uninitialized...
+ remoteDriverLock(priv);
+
+ make_nonnull_network(&args.net, net);
+ args.mac = mac ? (char **)&mac : NULL;
Again, I think args.mac is typed incorrectly. You should support
args.need_results to allow passing a NULL leases through to the driver.
+ args.flags = flags;
+
+ memset(&ret, 0, sizeof(ret));
+
+ if (call(net->conn, priv, 0, REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC,
+ (xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_args, (char
*)&args,
+ (xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_ret, (char
*)&ret) == -1) {
+ goto done;
+ }
+
+ if (ret.leases.leases_len > REMOTE_NETWORK_DHCP_LEASES_MAX) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Number of leases is %d, which exceeds max limit:
%d"),
+ ret.leases.leases_len, REMOTE_NETWORK_DHCP_LEASES_MAX);
+ return -1;
+ goto cleanup;
Dead goto. The return -1 is wrong. With that removed,
...leases_ret is still uninitialized...
+ }
+
+ if (ret.leases.leases_len &&
No need to populate leases_ret if the user passes leases==NULL.
+ VIR_ALLOC_N(leases_ret, ret.leases.leases_len + 1) < 0)
{
+ goto cleanup;
+ }
+
+ for (i = 0; i < ret.leases.leases_len; i++) {
+ if (VIR_ALLOC(leases_ret[i]) < 0)
+ goto cleanup;
+
+ virNetworkDHCPLeasesPtr lease = leases_ret[i];
+ remote_network_dhcp_lease *lease_ret = &(ret.leases.leases_val[i]);
+ lease->expirytime = lease_ret->expirytime;
+ lease->type = lease_ret->type;
+ lease->prefix = lease_ret->prefix;
+
+ if ((VIR_STRDUP(lease->mac, lease_ret->mac) < 0) ||
+ (VIR_STRDUP(lease->ipaddr, lease_ret->ipaddr) < 0) ||
+ (VIR_STRDUP(lease->hostname, lease_ret->hostname) < 0) ||
+ (VIR_STRDUP(lease->clientid, lease_ret->clientid) < 0))
+ goto cleanup;
I'm not sure if it's worth some transfer semantics instead of
VIR_STRDUP, as in:
lease->mac = lease_ret->mac;
lease_ret->mac = NULL;
On the one hand, it avoids some malloc pressure; on the other, it's a
little bit harder to follow all paths to make sure there are no leaks or
double frees.
+ }
+
+ *leases = leases_ret;
+ leases_ret = NULL;
+ rv = ret.leases.leases_len;
+
+cleanup:
+ if (leases_ret) {
+ for (i = 0; i < ret.leases.leases_len; i++)
+ virNetworkDHCPLeaseFree(leases_ret[i]);
...and you dereference an uninit variable to start freeing random data.
Hello SIGSEGV.
+ }
+ VIR_FREE(leases_ret);
+ xdr_free((xdrproc_t)xdr_remote_network_get_dhcp_leases_for_mac_ret,
+ (char *) &ret);
+
+done:
+ remoteDriverUnlock(priv);
+ return rv;
+}
+
+static int
+remoteNetworkGetDHCPLeases(virNetworkPtr net,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags)
+{
+ int rv = -1;
+ size_t i;
+ struct private_data *priv = net->conn->networkPrivateData;
+ remote_network_get_dhcp_leases_args args;
+ remote_network_get_dhcp_leases_ret ret;
+
+ virNetworkDHCPLeasesPtr *leases_ret;
Same SEGSEGV on uninit data if something else fails.
+ remoteDriverLock(priv);
+
+ make_nonnull_network(&args.net, net);
+ args.flags = flags;
+
+ memset(&ret, 0, sizeof(ret));
+
+ if (call(net->conn, priv, 0, REMOTE_PROC_NETWORK_GET_DHCP_LEASES,
+ (xdrproc_t)xdr_remote_network_get_dhcp_leases_args, (char *)&args,
+ (xdrproc_t)xdr_remote_network_get_dhcp_leases_ret, (char *)&ret) == -1)
{
+ goto done;
+ }
+
+ if (ret.leases.leases_len > REMOTE_NETWORK_DHCP_LEASES_MAX) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Number of leases is %d, which exceeds max limit:
%d"),
+ ret.leases.leases_len, REMOTE_NETWORK_DHCP_LEASES_MAX);
+ return -1;
+ goto cleanup;
Same dead goto.
+ }
+
+ if (ret.leases.leases_len &&
+
VIR_ALLOC_N(leases_ret, ret.leases.leases_len + 1) < 0) {
+ goto cleanup;
+ }
+
+ for (i = 0; i < ret.leases.leases_len; i++) {
+ if (VIR_ALLOC(leases_ret[i]) < 0)
+ goto cleanup;
+
+ virNetworkDHCPLeasesPtr lease = leases_ret[i];
+ remote_network_dhcp_lease *lease_ret = &(ret.leases.leases_val[i]);
+ lease->expirytime = lease_ret->expirytime;
+ lease->type = lease_ret->type;
+ lease->prefix = lease_ret->prefix;
+
+ if ((VIR_STRDUP(lease->mac, lease_ret->mac) < 0) ||
+ (VIR_STRDUP(lease->ipaddr, lease_ret->ipaddr) < 0) ||
+ (VIR_STRDUP(lease->hostname, lease_ret->hostname) < 0) ||
+ (VIR_STRDUP(lease->clientid, lease_ret->clientid) < 0))
+ goto cleanup;
> + }
Same issue about not handling user's NULL leases.
+
+ *leases = leases_ret;
+ leases_ret = NULL;
+ rv = ret.leases.leases_len;
+
+cleanup:
+ if (leases_ret) {
+ for (i = 0; i < ret.leases.leases_len; i++)
+ virNetworkDHCPLeaseFree(leases_ret[i]);
+ }
+ VIR_FREE(leases_ret);
+ xdr_free((xdrproc_t)xdr_remote_network_get_dhcp_leases_ret,
+ (char *) &ret);
+
+done:
+ remoteDriverUnlock(priv);
+ return rv;
+}
+
static void
remoteDomainEventQueue(struct private_data *priv, virDomainEventPtr event)
{
@@ -6958,6 +7106,8 @@ static virNetworkDriver network_driver = {
.networkSetAutostart = remoteNetworkSetAutostart, /* 0.3.0 */
.networkIsActive = remoteNetworkIsActive, /* 0.7.3 */
.networkIsPersistent = remoteNetworkIsPersistent, /* 0.7.3 */
+ .networkGetDHCPLeases = remoteNetworkGetDHCPLeases, /* 1.1.3 */
+ .networkGetDHCPLeasesForMAC = remoteNetworkGetDHCPLeasesForMAC, /* 1.1.3 */
};
static virInterfaceDriver interface_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 85ad9ba..a29646b 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -232,6 +232,11 @@ const REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX = 64;
/* Upper limit on number of job stats */
const REMOTE_DOMAIN_JOB_STATS_MAX = 16;
+/*
+ * Upper limit on the maximum number of leases in one lease file
+ */
+const REMOTE_NETWORK_DHCP_LEASES_MAX = 32768;
+
/* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */
typedef opaque remote_uuid[VIR_UUID_BUFLEN];
@@ -2835,6 +2840,35 @@ struct remote_domain_event_device_removed_msg {
remote_nonnull_string devAlias;
};
+struct remote_network_dhcp_lease {
+ hyper expirytime;
+ remote_nonnull_string mac;
+ remote_nonnull_string ipaddr;
+ remote_nonnull_string hostname;
+ remote_nonnull_string clientid;
+ int type;
+ unsigned int prefix;
+};
+
+struct remote_network_get_dhcp_leases_args {
+ remote_nonnull_network net;
+ unsigned int flags;
Needs a 'int need_results' member to pass NULL.
+};
+
+struct remote_network_get_dhcp_leases_ret {
+ remote_network_dhcp_lease leases<REMOTE_NETWORK_DHCP_LEASES_MAX>;
Needs a 'int ret' member to handle NULL leases.
+};
+
+struct remote_network_get_dhcp_leases_for_mac_args {
+ remote_nonnull_network net;
+ remote_string mac;
This should be remote_nonnull_string (since the API already rejects NULL).
+ unsigned int flags;
+};
+
+struct remote_network_get_dhcp_leases_for_mac_ret {
+ remote_network_dhcp_lease leases<REMOTE_NETWORK_DHCP_LEASES_MAX>;
+};
Same issues about handling NULL leases.
+
/*----- Protocol. -----*/
/* Define the program number, protocol version and procedure numbers here. */
@@ -4998,5 +5032,19 @@ enum remote_procedure {
* @generate: both
* @acl: none
*/
- REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311
+ REMOTE_PROC_DOMAIN_EVENT_DEVICE_REMOVED = 311,
+
+ /**
+ * @generate: none
+ * @priority: high
Is high priority appropriate? Is this something where we are always
guaranteed that we will never need to block or interact with external
processes to get the information, even if we change our implementation
away from dnsmasq to something else?
+ * @acl: network:read
+ */
+ REMOTE_PROC_NETWORK_GET_DHCP_LEASES = 312,
+
+ /**
+ * @generate: none
+ * @priority: high
+ * @acl: network:read
+ */
+ REMOTE_PROC_NETWORK_GET_DHCP_LEASES_FOR_MAC = 313
};
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 4e27aae..9b625e8 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
Be sure to regenerate this after fixing the .x file. You may also have
some (trivial) conflicts, depending on whether other new APIs go in first.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:49 a.m.
New subject: [libvirt] [PATCHv3 3/4] net-dhcp-leases: Private implementation inside network driver
By querying the driver for the path of the leases file for the given virtual
network and parsing it to retrieve info.
src/network/bridge_driver.c:
* Implement networkGetDHCPLeases
* Implement networkGetDHCPLeasesForMAC
* Implement networkGetDHCPLeasesHelper
---
src/network/bridge_driver.c | 177 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 177 insertions(+)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 3a8be90..f5adb97 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -109,6 +109,21 @@ static int networkPlugBandwidth(virNetworkObjPtr net,
virDomainNetDefPtr iface);
static int networkUnplugBandwidth(virNetworkObjPtr net,
virDomainNetDefPtr iface);
+/**
+ * VIR_NETWORK_DHCP_LEASE_LENGTH_MAX:
+ *
+ * Macro providing the maximum length of an entry in the leases file
+ * Refer: http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2013q3/007402.html
+ */
+#define VIR_NETWORK_DHCP_LEASE_LENGTH_MAX 2048
+
+/**
+ * VIR_NETWORK_DHCPLEASE_PARAMS:
+ *
+ * Macro providing the maximum number of parameters in an entry in
+ * the leases file
+ */
+#define VIR_NETWORK_DHCP_LEASE_FIELDS 5
static virNetworkDriverStatePtr driverState = NULL;
@@ -2980,6 +2995,166 @@ cleanup:
return ret;
}
+/* This function parese the leases file of dnsmasq.
+ *
+ * An example of leases file content:
+ *
+ * 1379024255 52:54:00:20:70:3d 192.168.105.240 * *
+ * 1379023351 52:54:00:b1:70:19 192.168.105.201 * *
+ */
+static int
+networkGetDHCPLeasesHelper(virNetworkPtr network,
+ virNetworkObjPtr obj,
+ const char *mac,
+ virNetworkDHCPLeasesPtr **leases)
+{
+ int rv = -1;
+ size_t i = 0;
+ size_t nleases = 0;
+ char *leasefile;
+ char **lease_fields;
+ char dhcpentry[VIR_NETWORK_DHCP_LEASE_LENGTH_MAX];
+ virNetworkDHCPLeasesPtr *leases_ret = NULL;
+ FILE *fp = NULL;
+
+ if (!network) {
+ virReportError(VIR_ERR_NO_NETWORK, "%s",
+ _("no network with matching name"));
+ return -1;
+ }
+
+ /* Retrieve leases file location */
+ leasefile = networkDnsmasqLeaseFileNameDefault(network->name);
+ if (!(fp = fopen(leasefile, "r"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to open leases file: %s"), leasefile);
+ goto cleanup;
+ }
+
+ while (fgets(dhcpentry, sizeof(dhcpentry), fp) != NULL) {
+ virNetworkDHCPLeasesPtr lease = NULL;
+
+ /* Remove newline */
+ dhcpentry[strlen(dhcpentry) - 1] = '\0';
+
+ /* Split the lease line */
+ lease_fields = virStringSplit(dhcpentry, " ",
VIR_NETWORK_DHCP_LEASE_FIELDS);
+
+ if (virStringListLength(lease_fields) != VIR_NETWORK_DHCP_LEASE_FIELDS) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Number of lease params aren't equal to:
%d"),
+ VIR_NETWORK_DHCP_LEASE_FIELDS);
+ goto error;
+ }
+
+ if (mac && STRNEQ(mac, lease_fields[1]))
+ continue;
+
+ if (VIR_EXPAND_N(leases_ret, nleases, 1) < 0)
+ goto error;
+
+ if (VIR_ALLOC(leases_ret[nleases - 1]) < 0)
+ goto error;
+
+ lease = leases_ret[nleases - 1];
+
+ /* Convert expirytime here */
+ if (virStrToLong_ll(lease_fields[0], NULL, 10, &(lease->expirytime)) <
0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to convert lease expiry time to integer:
%s"),
+ lease_fields[0]);
+ goto error;
+ }
+
+ /* Hardcoded, as dnsmasq uses ipv4 */
+ lease->type = VIR_IP_ADDR_TYPE_IPV4;
+ lease->prefix =
virSocketAddrGetIpPrefix(&obj->def->ips->address,
+ &obj->def->ips->netmask,
+ obj->def->ips->prefix);
+
+ if ((VIR_STRDUP(lease->mac, lease_fields[1]) < 0) ||
+ (VIR_STRDUP(lease->ipaddr, lease_fields[2]) < 0) ||
+ (VIR_STRDUP(lease->hostname, lease_fields[3]) < 0) ||
+ (VIR_STRDUP(lease->clientid, lease_fields[4]) < 0))
+ goto error;
+ }
+
+ if (mac && !leases_ret) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("no lease with matching MAC address: %s"), mac);
+ goto error;
+ }
+
+ if (leases_ret) {
+ /* NULL terminated array */
+ ignore_value(VIR_REALLOC_N(leases_ret, nleases + 1));
+ *leases = leases_ret;
+ leases_ret = NULL;
+ rv = nleases;
+ }
+
+cleanup:
+ VIR_FORCE_FCLOSE(fp);
+ VIR_FREE(leasefile);
+ return rv;
+
+error:
+ if (leases_ret) {
+ for (i = 0; i < nleases; i++)
+ virNetworkDHCPLeaseFree(leases_ret[i]);
+ }
+ VIR_FREE(leases_ret);
+ goto cleanup;
+}
+
+static int
+networkGetDHCPLeases(virNetworkPtr network,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags)
+{
+ int rv = -1;
+ virNetworkObjPtr obj;
+
+ virCheckFlags(0, -1);
+
+ if (!(obj = networkObjFromNetwork(network)))
+ return rv;
+
+ if (virNetworkGetDHCPLeasesEnsureACL(network->conn, obj->def) < 0)
+ goto cleanup;
+
+ rv = networkGetDHCPLeasesHelper(network, obj, NULL, leases);
+
+cleanup:
+ if (obj)
+ virNetworkObjUnlock(obj);
+ return rv;
+}
+
+static int
+networkGetDHCPLeasesForMAC(virNetworkPtr network,
+ const char *mac,
+ virNetworkDHCPLeasesPtr **leases,
+ unsigned int flags)
+{
+ int rv = -1;
+ virNetworkObjPtr obj;
+
+ virCheckFlags(0, -1);
+
+ if (!(obj = networkObjFromNetwork(network)))
+ return rv;
+
+ if (virNetworkGetDHCPLeasesForMACEnsureACL(network->conn, obj->def) < 0)
+ goto cleanup;
+
+ rv = networkGetDHCPLeasesHelper(network, obj, mac, leases);
+
+cleanup:
+ if (obj)
+ virNetworkObjUnlock(obj);
+ return rv;
+}
static virNetworkDriver networkDriver = {
"Network",
@@ -3004,6 +3179,8 @@ static virNetworkDriver networkDriver = {
.networkSetAutostart = networkSetAutostart, /* 0.2.1 */
.networkIsActive = networkIsActive, /* 0.7.3 */
.networkIsPersistent = networkIsPersistent, /* 0.7.3 */
+ .networkGetDHCPLeases = networkGetDHCPLeases, /* 1.1.3 */
+ .networkGetDHCPLeasesForMAC = networkGetDHCPLeasesForMAC, /* 1.1.3 */
};
static virStateDriver networkStateDriver = {
--
1.7.11.7
2 p.m.
New subject: [libvirt] [PATCHv3 3/4] net-dhcp-leases: Private implementation inside network driver
On 09/15/2013 11:49 PM, Nehal J Wani wrote:
By querying the driver for the path of the leases file for the given
virtual
network and parsing it to retrieve info.
src/network/bridge_driver.c:
* Implement networkGetDHCPLeases
* Implement networkGetDHCPLeasesForMAC
* Implement networkGetDHCPLeasesHelper
---
src/network/bridge_driver.c | 177 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 177 insertions(+)
}
+/* This function parese the leases file of dnsmasq.
s/parese/parses/
+ *
+ * An example of leases file content:
+ *
+ * 1379024255 52:54:00:20:70:3d 192.168.105.240 * *
+ * 1379023351 52:54:00:b1:70:19 192.168.105.201 * *
+ */
+static int
+networkGetDHCPLeasesHelper(virNetworkPtr network,
+ virNetworkObjPtr obj,
+ const char *mac,
+ virNetworkDHCPLeasesPtr **leases)
+{
+ int rv = -1;
+ size_t i = 0;
+ size_t nleases = 0;
+ char *leasefile;
+ char **lease_fields;
+ char dhcpentry[VIR_NETWORK_DHCP_LEASE_LENGTH_MAX];
+ virNetworkDHCPLeasesPtr *leases_ret = NULL;
+ FILE *fp = NULL;
+
+ if (!network) {
+ virReportError(VIR_ERR_NO_NETWORK, "%s",
+ _("no network with matching name"));
+ return -1;
+ }
Dead code (network is always non-NULL if you get here).
+
+ /* Retrieve leases file location */
+ leasefile = networkDnsmasqLeaseFileNameDefault(network->name);
+ if (!(fp = fopen(leasefile, "r"))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to open leases file: %s"), leasefile);
+ goto cleanup;
+ }
+
+ while (fgets(dhcpentry, sizeof(dhcpentry), fp) != NULL) {
Rather than hand-rolling your file reading, I would suggest using
virFileReadAll(leasefile, len, &buf), and then loop over the returned
buffer. That gives you nicer guarantees about handling things like
interrupts that fgets() might get confused on.
+ virNetworkDHCPLeasesPtr lease = NULL;
+
+ /* Remove newline */
+ dhcpentry[strlen(dhcpentry) - 1] = '\0';
+
+ /* Split the lease line */
+ lease_fields = virStringSplit(dhcpentry, " ",
VIR_NETWORK_DHCP_LEASE_FIELDS);
+
+ if (virStringListLength(lease_fields) != VIR_NETWORK_DHCP_LEASE_FIELDS) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Number of lease params aren't equal to:
%d"),
+ VIR_NETWORK_DHCP_LEASE_FIELDS);
+ goto error;
+ }
+
+ if (mac && STRNEQ(mac, lease_fields[1]))
This may not do what you want - remember, 'mac' is specified by the
user, whereas lease_fields[1] is specified by dnsmasq. While we can
reasonably assume that dnsmasq always uses a canonical MAC
representation (such as 52:54:00:1a:0a:6d), the user may have passed in
a non-canonical form that we can still parse and recognize as the same
address (such as 52:54:00:1A:0a:6D, or even 52:54:0:1a:a:6d). You want
to use virMacAddrCompare() instead of STRNEQ.
+ continue;
Memleak. lease_fields is allocated each iteration of the loop, so it
must be freed each iteration of the loop.
+
+ if (VIR_EXPAND_N(leases_ret, nleases, 1) < 0)
+ goto error;
+
+ if (VIR_ALLOC(leases_ret[nleases - 1]) < 0)
+ goto error;
Rather than trying to allocate it into the destination array, it might
be better to allocate into a local variable, and only on successful
population of the entire element, use VIR_INSERT_ELEMENT to move it into
the array at that time.
+
+ lease = leases_ret[nleases - 1];
+
+ /* Convert expirytime here */
+ if (virStrToLong_ll(lease_fields[0], NULL, 10, &(lease->expirytime)) <
0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to convert lease expiry time to integer:
%s"),
+ lease_fields[0]);
+ goto error;
+ }
+
+ /* Hardcoded, as dnsmasq uses ipv4 */
+ lease->type = VIR_IP_ADDR_TYPE_IPV4;
+ lease->prefix =
virSocketAddrGetIpPrefix(&obj->def->ips->address,
+ &obj->def->ips->netmask,
+ obj->def->ips->prefix);
+
+ if ((VIR_STRDUP(lease->mac, lease_fields[1]) < 0) ||
+ (VIR_STRDUP(lease->ipaddr, lease_fields[2]) < 0) ||
+ (VIR_STRDUP(lease->hostname, lease_fields[3]) < 0) ||
+ (VIR_STRDUP(lease->clientid, lease_fields[4]) < 0))
+ goto error;
+ }
Transfer semantics rather than VIR_STRDUP might save some memory pressure.
+
+ if (mac && !leases_ret) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("no lease with matching MAC address: %s"), mac);
+ goto error;
+ }
+
+ if (leases_ret) {
+ /* NULL terminated array */
+ ignore_value(VIR_REALLOC_N(leases_ret, nleases + 1));
+ *leases = leases_ret;
+ leases_ret = NULL;
+ rv = nleases;
+ }
+
+cleanup:
+ VIR_FORCE_FCLOSE(fp);
+ VIR_FREE(leasefile);
+ return rv;
+
+error:
+ if (leases_ret) {
+ for (i = 0; i < nleases; i++)
+ virNetworkDHCPLeaseFree(leases_ret[i]);
+ }
+ VIR_FREE(leases_ret);
+ goto cleanup;
+}
+
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:49 a.m.
New subject: [libvirt] [PATCHv3 4/4] net-dhcp-leases: Add virsh support
Use virNetworkGetDHCPLeases and virNetworkGetDHCPLeasesForMAC in virsh.
The new feature supports the follwing methods:
1. Retrieve leases info for a given virtual network
2. Retrieve leases info for given network interface
tools/virsh-domain-monitor.c
* Introduce new command : net-dhcp-leases
Example Usage: net-dhcp-leases <network> [mac]
virsh # net-dhcp-leases default
Expiry Time MAC address Protocol IP address Hostname
ClientId
------------------------------------------------------------------------------------------------
13-09-2013 03:45:31 52:54:00:20:70:3d ipv4 192.168.105.240/24 f18
*
13-09-2013 03:32:31 52:54:00:b1:70:19 ipv4 192.168.105.201/24 LDAP
*
tools/virsh.pod
* Document new command
---
tools/virsh-network.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++
tools/virsh.pod | 6 +++
2 files changed, 106 insertions(+)
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 8ddd5ca..9543c64 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -1129,6 +1129,100 @@ cmdNetworkEdit(vshControl *ctl, const vshCmd *cmd)
return ret;
}
+/*
+ * "net-dhcp-leases" command
+ */
+static const vshCmdInfo info_network_dhcp_leases[] = {
+ {.name = "help",
+ .data = N_("Print lease info for a given network")
+ },
+ {.name = "desc",
+ .data = N_("Print lease info for a given network")
+ },
+ {.name = NULL}
+};
+
+static const vshCmdOptDef opts_network_dhcp_leases[] = {
+ {.name = "network",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("network name or uuid")
+ },
+ {.name = "mac",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_NONE,
+ .help = N_("MAC address")
+ },
+ {.name = NULL}
+};
+
+static bool
+cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd)
+{
+ const char *name = NULL;
+ const char *mac = NULL;
+ virNetworkDHCPLeasesPtr *leases = NULL;
+ int nleases = 0;
+ bool ret = false;
+ size_t i;
+ unsigned int flags = 0;
+ virNetworkPtr network = NULL;
+
+ if (vshCommandOptString(cmd, "mac", &mac) < 0)
+ return false;
+
+ if (!(network = vshCommandOptNetworkBy(ctl, cmd, &name,
+ VSH_BYNAME | VSH_BYUUID)))
+ return false;
+
+ nleases = mac ? virNetworkGetDHCPLeasesForMAC(network, mac, &leases, flags)
+ : virNetworkGetDHCPLeases(network, &leases, flags);
+
+ if (nleases < 0) {
+ vshError(ctl, _("Failed to get leases info for %s"), name);
+ goto cleanup;
+ }
+
+ vshPrintExtra(ctl, " %-20s %-20s %-10s %-20s %-12s %s\n%s%s\n",
+ _("Expiry Time"), _("MAC address"),
_("Protocol"),
+ _("IP address"), _("Hostname"),
_("ClientId"),
+ "------------------------------------------------",
+ "------------------------------------------------");
+
+ for (i = 0; i < nleases; i++) {
+ const char *type = NULL;
+ virNetworkDHCPLeasesPtr lease = leases[i];
+ time_t expirytime_tmp = lease->expirytime;
+ struct tm ts;
+ char expirytime[32];
+ ts = *localtime_r(&expirytime_tmp, &ts);
+ strftime(expirytime, sizeof(expirytime), "%d-%m-%Y %H:%M:%S",
&ts);
+
+ switch (lease->type) {
+ case VIR_IP_ADDR_TYPE_IPV4:
+ type = "ipv4";
+ break;
+ case VIR_IP_ADDR_TYPE_IPV6:
+ type = "ipv6";
+ break;
+ }
+
+ vshPrintExtra(ctl, " %-20s %-20s %-10s %s/%-5d %-12s %s\n",
+ expirytime, lease->mac, type, lease->ipaddr,
+ lease->prefix, lease->hostname, lease->clientid);
+ }
+
+ ret = true;
+
+cleanup:
+ if (leases) {
+ for (i = 0; i < nleases; i++)
+ virNetworkDHCPLeaseFree(leases[i]);
+ }
+ VIR_FREE(leases);
+ virNetworkFree(network);
+ return ret;
+}
const vshCmdDef networkCmds[] = {
{.name = "net-autostart",
@@ -1209,5 +1303,11 @@ const vshCmdDef networkCmds[] = {
.info = info_network_uuid,
.flags = 0
},
+ {.name = "net-dhcp-leases",
+ .handler = cmdNetworkDHCPLeases,
+ .opts = opts_network_dhcp_leases,
+ .info = info_network_dhcp_leases,
+ .flags = 0
+ },
{.name = NULL}
};
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 0ae5178..b87f646 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2325,6 +2325,12 @@ If I<--current> is specified, affect the current network
state.
Both I<--live> and I<--config> flags may be given, but I<--current> is
exclusive. Not specifying any flag is the same as specifying I<--current>.
+=item B<net-dhcp-leases> I<network> I<mac>
+
+Get a list of dhcp leases for all network interfaces connected to the given
+virtual I<network> or limited output just for one interface if I<mac> is
+specified.
+
=back
=head1 INTERFACE COMMANDS
--
1.7.11.7
2:28 p.m.
New subject: [libvirt] [PATCHv3 4/4] net-dhcp-leases: Add virsh support
On 09/15/2013 11:49 PM, Nehal J Wani wrote:
Use virNetworkGetDHCPLeases and virNetworkGetDHCPLeasesForMAC in
virsh.
The new feature supports the follwing methods:
1. Retrieve leases info for a given virtual network
2. Retrieve leases info for given network interface
+
+static bool
+cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd)
+{
+ const char *name = NULL;
+ const char *mac = NULL;
+ virNetworkDHCPLeasesPtr *leases = NULL;
+ int nleases = 0;
+ bool ret = false;
+ size_t i;
+ unsigned int flags = 0;
+ virNetworkPtr network = NULL;
+
+ if (vshCommandOptString(cmd, "mac", &mac) < 0)
+ return false;
+
+ if (!(network = vshCommandOptNetworkBy(ctl, cmd, &name,
+ VSH_BYNAME | VSH_BYUUID)))
+ return false;
Just use vshCommandOptNetwork (no need to use the filtering of NetworkBy).
+
+ nleases = mac ? virNetworkGetDHCPLeasesForMAC(network, mac, &leases, flags)
+ : virNetworkGetDHCPLeases(network, &leases, flags);
+
+ if (nleases < 0) {
+ vshError(ctl, _("Failed to get leases info for %s"), name);
+ goto cleanup;
+ }
+
+ vshPrintExtra(ctl, " %-20s %-20s %-10s %-20s %-12s %s\n%s%s\n",
+ _("Expiry Time"), _("MAC address"),
_("Protocol"),
+ _("IP address"), _("Hostname"),
_("ClientId"),
+ "------------------------------------------------",
+ "------------------------------------------------");
+
+ for (i = 0; i < nleases; i++) {
Should you qsort() the list before printing it, to ensure that identical
MAC lines are grouped even if the driver didn't hand them back in any
particular order?
+ const char *type = NULL;
+ virNetworkDHCPLeasesPtr lease = leases[i];
+ time_t expirytime_tmp = lease->expirytime;
+ struct tm ts;
+ char expirytime[32];
+ ts = *localtime_r(&expirytime_tmp, &ts);
+ strftime(expirytime, sizeof(expirytime), "%d-%m-%Y %H:%M:%S",
&ts);
Please use ISO %Y-%m-%d, as it is sortable, and also has the benefit of
being unambiguous in both US and UK.
+
+ switch (lease->type) {
+ case VIR_IP_ADDR_TYPE_IPV4:
+ type = "ipv4";
+ break;
+ case VIR_IP_ADDR_TYPE_IPV6:
+ type = "ipv6";
+ break;
+ }
+
+ vshPrintExtra(ctl, " %-20s %-20s %-10s %s/%-5d %-12s %s\n",
+ expirytime, lease->mac, type, lease->ipaddr,
If a third lease->type is introduced in a future libvirtd, then you risk
passing NULL to printf(%s). Please use NULLSTR(type), so that you don't
crash on non-glibc systems.
+ lease->prefix, lease->hostname,
lease->clientid);
Do we guarantee that ALL string fields of lease are populated with
non-NULL strings (even if the empty string), no matter what? Or should
we revisit the docs in 1/4, and the RPC implementation of 2/4, to allow
for NULL strings for fields that aren't available?
+ }
+
+ ret = true;
+
+cleanup:
+ if (leases) {
+ for (i = 0; i < nleases; i++)
+ virNetworkDHCPLeaseFree(leases[i]);
+ }
+ VIR_FREE(leases);
+ virNetworkFree(network);
+ return ret;
+}
const vshCmdDef networkCmds[] = {
{.name = "net-autostart",
@@ -1209,5 +1303,11 @@ const vshCmdDef networkCmds[] = {
.info = info_network_uuid,
.flags = 0
},
+ {.name = "net-dhcp-leases",
+ .handler = cmdNetworkDHCPLeases,
+ .opts = opts_network_dhcp_leases,
+ .info = info_network_dhcp_leases,
+ .flags = 0
Not your fault, but we should be using trailing commas when doing C99
struct initialization.
+ },
{.name = NULL}
};
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 0ae5178..b87f646 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2325,6 +2325,12 @@ If I<--current> is specified, affect the current network
state.
Both I<--live> and I<--config> flags may be given, but I<--current>
is
exclusive. Not specifying any flag is the same as specifying I<--current>.
+=item B<net-dhcp-leases> I<network> I<mac>
List it as '[I<mac>]', to make it obvious the mac address is optional.
+
+Get a list of dhcp leases for all network interfaces connected to the given
+virtual I<network> or limited output just for one interface if I<mac> is
+specified.
+
=back
=head1 INTERFACE COMMANDS
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4087
days inactive
4087
days old
16 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Nehal J Wani