On Thu, Dec 12, 2013 at 06:00:17PM +0530, Nehal J Wani wrote:
> On Thu, Dec 12, 2013 at 5:14 PM, Daniel P. Berrange <berrange@xxxxxxxxxx> 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'
That'd go away if you remove that check from the public API
and put it in the driver code instead.
> >> 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 ?
If you need to parse it, then yes, but if you don't need to parse
it then don't.
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list