Re: [libvirt] [PATCHv5 1/4] net-dhcp-leases: Implement the public APIs

On Thu, Dec 12, 2013 at 5:14 PM, Daniel P. Berrange <berrange(a)redhat.com&gt; wrote: > On Tue, Nov 26, 2013 at 02:35:58AM +0530, Nehal J Wani wrote: >> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in >> index 5aad75c..20ea40a 100644 >> --- a/include/libvirt/libvirt.h.in >> +++ b/include/libvirt/libvirt.h.in >> + >> +typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases; >> +typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr; >> +struct _virNetworkDHCPLeases { > > s/Leases/Lease/ - each struct only stores a single lease > >> + char *interface; /* Network interface name */ >> + long long expirytime; /* Seconds since epoch */ >> + int type; /* virIPAddrType */ >> + char *mac; /* MAC address */ >> + char *iaid; /* IAID */ >> + char *ipaddr; /* IP address */ >> + unsigned int prefix; /* IP address prefix */ >> + char *hostname; /* Hostname */ >> + char *clientid; /* Client ID or DUID */ >> +}; > >> @@ -2019,6 +2022,7 @@ libvirt_setuid_rpc_client_la_SOURCES = \ >> util/virtypedparam.c \ >> util/viruri.c \ >> util/virutil.c \ >> + util/virmacaddr.c \ >> util/viruuid.c \ >> conf/domain_event.c \ >> rpc/virnetsocket.c \ > > Don't add this here. The code didn't compile without making the above addition. It kept throwing the error: ../src/.libs/libvirt-setuid-rpc-client.a(libvirt_setuid_rpc_client_la-libvirt.o): In function `virNetworkGetDHCPLeasesForMAC': /home/Wani/Hobby/libvirt/src/libvirt.c:22187: undefined reference to `virMacAddrParse'

>> diff --git a/src/libvirt.c b/src/libvirt.c >> index eff44eb..9a0b872 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" > >> +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(); >> + >> + if (leases) >> + *leases = NULL; >> + >> + virCheckNonNullArgGoto(mac, error); >> + >> + if (!VIR_IS_CONNECTED_NETWORK(network)) { >> + virLibNetworkError(VIR_ERR_INVALID_NETWORK, __FUNCTION__); >> + virDispatchError(NULL); >> + return -1; >> + } >> + >> + /* Validate the MAC address */ >> + if (virMacAddrParse(mac, &addr) < 0) { >> + virReportInvalidArg(mac, "%s", >> + _("Given MAC Address doesn't comply " >> + "with the standard (IEEE 802) format")); >> + goto error; >> + } > > Don't do this here - it is the job of driver APIs todo semantic > validation of parameters. Do you mean, I need to put this check inside networkGetDHCPLeasesForMAC in src/network/bridge_driver.c ?