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
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
