On Fri, Oct 11, 2013 at 1:49 AM, Nehal J Wani <nehaljw.k...@gmail.com> wrote: > On Fri, Oct 11, 2013 at 12:36 AM, Eric Blake <ebl...@redhat.com> wrote: >> On 10/08/2013 06:39 AM, Nehal J Wani wrote: >>> Continued from >>> https://www.redhat.com/archives/libvir-list/2013-October/msg00325.html >>> >> >>> Since no one likes the idea of having a union, but we do want to >>> support dhcpv6, following is the new design: >>> >>> struct _virNetworkDHCPLeases { >>> char *interface; /* Network interface name (non-null) */ >>> long long expirytime; /* Seconds since epoch (non-null) */ >> >> (non-null) makes no sense for an integer. On the other hand, should we >> document a sentinel for unknown (or infinite) lease expiration? >> > dnsmasq will return the value 0 for expirytime in case of infinite expiration. > >>> int type; /* virIPAddrType (non-null) */ >> >> (non-null) makes no sense for an integer. >> >>> unsigned int prefix; /* IP address prefix (non-null) */ >> >> (non-null) makes no sense for an integer; should we document that 0 >> means unknown? >> >>> char *mac; /* MAC address (mostly non-null, >>> except rare cases) */ >> >> I guess it could be NULL for the generic virNetworkDHCPLeases query; but >> I don't see how it can possibly be NULL for the specific per-MAC >> virNetworkDHCPLeases ForMAC(). Probably also worth documenting that it >> is in ASCII form (not raw bytes). If you implement it as remote_string >> on the RPC side, then you are guaranteeing that it is non-NULL; are we >> hurting ourselves if we make that guarantee? > > Well, in the discussion with dnsmasq developers, Simon had said, "if > you're interested in the MAC addresses of clients, the very latest > dnsmasq code can determine that in most cases." > > Since it says 'most cases', and we don't want to take risks, I was > being skeptical in keeping it NON-NULL. > > In the case for virNetworkDHCPLeasesForMAC(), if the user passes NULL > for@mac, the API will automatically throw an error: > > + /* 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; > + } > > > > So, rewriting: > > /** > * _virNetworkDHCPLeases: > * For DHCPv4, the information returned: > * - Network Interface Name > * - Expiry Time > * - MAC address (can be NULL, only in rare cases) > * - IAID (NULL) > * - IPv4 address (with type and prefix) > * - Hostname (can be NULL) > * - Client ID (can be NULL) > * > * For DHCPv6, the information returned: > * - Network Interface Name > * - Expiry Time > * - MAC address (can be NULL, only in rare cases) > * - IAID (can be NULL, only in rare cases) > * - IPv6 address (with type and prefix) > * - Hostname (can be NULL) > * - Client DUID > * > * Note: @mac, @iaid, @ipaddr, @clientid are in ASCII form, not raw bytes. > * Note: @expirytime can 0, in case the lease is for infinite time. > */ > struct _virNetworkDHCPLeases { > char *interface; /* Network interface name */ > long long expirytime; /* Seconds since epoch */ > int type; /* virIPAddrType */ > unsigned int prefix; /* IP address prefix */ > char *mac; /* MAC address */ > char *iaid; /* IAID */ > char *ipaddr; /* IP address */ > char *hostname; /* Hostname */ > char *clientid; /* Client ID or DUID */ > }; > > /* Remote struct */ > struct remote_network_dhcp_lease { > remote_nonnull_string interface; > hyper expirytime; > int type; > unsigned int prefix; > remote_string mac; > remote_string iaid; > remote_nonnull_string ipaddr; > remote_string hostname; > remote_string clientid; > }; >
danpb, laine, would you like to share your views? -- Nehal J Wani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
