Re: [RFC] net-dhcp-leases: virNetworkDHCPLeases struct design

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

 



On Fri, Oct 11, 2013 at 1:49 AM, Nehal J Wani <nehaljw.kkd1@xxxxxxxxx> wrote:
> On Fri, Oct 11, 2013 at 12:36 AM, Eric Blake <eblake@xxxxxxxxxx> 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]