Re: [PATCH v8 1/4] domifaddr: Implement the public APIs

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

 



On 03/05/2015 07:45 AM, Daniel P. Berrange wrote:
> On Mon, Jan 26, 2015 at 12:08:46AM +0530, Nehal J Wani wrote:
>> Define helper function virDomainInterfaceFree, which allows
>> the upper layer application to free the domain interface object
>> conveniently.
>>
>> The API is going to provide multiple methods by flags, e.g.
>>   * Query guest agent
>>   * Parse DHCP lease file
>>

>> +++ b/include/libvirt/libvirt-domain.h
>> @@ -3682,5 +3682,32 @@ typedef struct _virTypedParameter virMemoryParameter;
>>   */
>>  typedef virMemoryParameter *virMemoryParameterPtr;
>>  
>> +typedef enum {
>> +    VIR_DOMAIN_INTERFACE_ADDRESSES_LEASE = (1 << 0), /* Parse DHCP lease file */
>> +    VIR_DOMAIN_INTERFACE_ADDRESSES_AGENT = (1 << 1), /* Query qemu guest agent */
>> +} virDomainInterfaceAddressesFlags;

This feels like it should not be named 'Flags', because it seems like
you can never request multiple modes at once.


> 
> I can't help but have a nagging feeling this ambiguos return data means our
> API design is wrong. Instead of having a bitwise OR of flags to select the
> data source, should we perhaps use a straight enum, so the application always
> specifies exactly which data source to use ?
> 
> eg an API that is
> 
>   enum {
>     VIR_DOMAIN_INTERFACE_ADDRESSES_SOURCE_LEASES,
>     VIR_DOMAIN_INTERFACE_ADDRESSES_SOURCE_AGENT,
>     VIR_DOMAIN_INTERFACE_ADDRESSES_SOURCE_SNOOP,
>   };
> 

Yep - that's exactly my concern, and the right solution (that is,
_LEASES is 0, _AGENT is 1, _SNOOP is 2, and for now we have a _LAST at 3
that can later bump to 4 if we add another method).

>   virDomainInterfaceAddresses(virDomainPtr dom,
>                               int source,
>                               virDomainInterfacePtr **ifaces,
> 			      unsigned int flags);
> 
> so the caller always knows how to interpret the returned data for
> mac addr andname. Leave the flags field unused, for future help
> if needed.

Yes, that sounds like the right direction.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
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]