[libvirt] RFC: Introduce API to query IP addresses for given domain

Osier Yang jyang at redhat.com
Tue Jul 2 13:41:00 UTC 2013


On 28/06/13 18:26, Nehal J. Wani wrote:
> Hello, fellow developers!
>        I am a GSoC candidate this year working forlibvirt.org  <http://libvirt.org>. My project is "Introduce API to query IP addresses for given domain".
> The discussion regarding this feature had started here:http://www.mail-archive.com/libvir-list@redhat.com/msg51857.html  and then Michal had sent a patch too (refer:http://www.mail-archive.com/libvir-list@redhat.com/msg57141.html). But it was not pushed upstream due to lack of extensibility.
>
> So far I've come up with an API and I want to get your opinion before I start writing the rest so I don't follow the wrong direction.
>
> Following are the valid commands:
>
> domifaddr <domain-name>
> domifaddr <domain-name> <interface-name>
> domifaddr <domain-name> <interface-name> <method>
> domifaddr <domain-name> <method>

with guessing 'domifaddr <domain-name>' will get all interfaces' info, 
this looks okay, but it's not the most important thing.

>
> methods:
> (i) Querying qemu-guest-agent
> (ii) Getting info from dnsmasq.leases file
> (iii) Using the nwfilter to snoop the traffic

not important AT THIS stage. but btw, does nwfilter support both arp and 
dhcp snooping?

>
> If no method is mentioned, qemu-guest-agent will be used.

yeah, using guest agent should be the default.

> Previous attempts by Michal had used structs and xml. Structs bring in restrictions and xml has to be parsed. Hence I am not planning to continue with either of these.
>
> As a start, I would like to know your comments about my API which queries the qemu-guest-agent and returns the results in virTypedParameter **params.
>
> Format(JSON) in which the qemu guest agent returns info:
>
> [{"name":"lo",
> 	"ip-addresses":
> 		[{"ip-address-type":"ipv4","ip-address":"127.0.0.1","prefix":8},
>           	{"ip-address-type":"ipv6","ip-address":"::1","prefix":128}],
>          "hardware-address":"00:00:00:00:00:00"},
> {"name":"eth0",
> 	"ip-addresses":
>          	[{"ip-address-type":"ipv4","ip-address":"192.168.122.42","prefix":24},
>           	{"ip-address-type":"ipv6","ip-address":"fe80::5054:ff:fe09:d240","prefix":64}],
>          "hardware-address":"52:54:00:09:d2:40"}]
>
> //Possible 1-D Structure (A little hassle to maintain)
>
> params[0] = {"iface-count",int,2}

i think this is no need. as long as you returns the number of interfaces 
as return value of the api, or *nparams.

> params[1] = {"iface-name",string,"lo"}
> params[2] = {"iface-hwaddr",string,"00:00:00:00:00:00"}
> params[3] = {"iface-addr-count",int,2}
> params[4] = {"iface-addr-type",string,"ipv4"}
> params[5] = {"iface-addr",string,"127.0.0.1"}
> params[6] = {"iface-addr-prefix",int,8}
> params[7] = {"iface-addr-type",string,"ipv6"}
> params[8] = {"iface-addr",string,"::1"}
> params[9] = {"iface-addr-prefix",int,128}

hm..  this is not well orgnized. param field must be unique, otherwise 
there are conflicts when getting the param value, assuming that i want 
to get value of field "iface-addr" by virTypedParamsGetString, what it 
should get? what will it get? [1]

since there could be multiple addrs for a interface, so we may need to 
introduce a new param type to have a array param value for the multiple 
addresses.

and if you can have a enum for adress types (they are fixed, 
ipv4/ipv6),  and then the type of "iface-addr-type" should be 'int'.

> ....
> ....
> ....
>
> //2D Structure: (Not very hasslefree, but easier to maintain as one interface per row)
>
> params[0] = {"iface-name",string,"lo"}{"iface-hwaddr",string,"00:00:00:00:00:00"}{"iface-addr-type",string,"ipv4"}{"iface-addr",string,"127.0.0.1"}{"iface-addr-prefix",int,8}{"iface-addr-type",string,"ipv6"}{"iface-addr",string,"::1"}{"iface-addr-prefix",int,128}
> params[1] = {"iface-name",string,"eth0"}{"iface-hwaddr",string,"52:54:00:09:d2:40"}{"iface-addr-type",string,"ipv4"}{"iface-addr",string,"192.168.122.42"}{"iface-addr-prefix",int,8}{"iface-addr-type",string,"ipv6"}{"iface-addr",string,"fe80::5054:ff:fe09:d240"}{"iface-addr-prefix",int,64}

no, this is not the right way to go, there is no param fields for 
params[i], i think you still don't understand how typed parameter works. 
assuming you are a user of your api may help understand what problems 
you will face with not good design, e.g [1].


>
> Function definitions that I intend to use are:
>
> static int
> remoteDispatchDomainInterfacesAddresses(
>      virNetServerPtr server ATTRIBUTE_UNUSED,
>      virNetServerClientPtr client,
>      virNetMessagePtr msg ATTRIBUTE_UNUSED,
>      virNetMessageErrorPtr rerr,
>      remote_domain_interfaces_addresses_args *args,
>      remote_domain_interfaces_addresses_ret *ret)
>
> int virDomainInterfacesAddresses (virDomainPtr dom,
>                                   virTypedParameterPtr *params,
>                                    int *nparams,
>                                    unsigned int flags);

[2] no need to list this, we only have to figure out the design of the 
public api.

>
>
> typedef int
> (*virDrvDomainInterfacesAddresses)   (virDomainPtr dom,
>                                        virTypedParameterPtr *params,
>                                        int *nparams,
>                                        unsigned int flags);

like [2]

and how to let the api know whether to get info of all interfaces or a 
specified one? also you need to define the flags for different method in 
proposal.



>   
>
>
> int
> virDomainInterfacesAddresses (virDomainPtr dom,
>                               virTypedParameterPtr *params,
>                               int *nparams,
>                               unsigned int flags)
>
> int
> qemuAgentGetInterfaces(qemuAgentPtr mon,
>                         virTypedParameterPtr *params,
>                         int *nparams)
>
>
> int qemuAgentGetInterfaces(qemuAgentPtr mon,
>                             virTypedParameterPtr *params,
>                             int *nparams);
>
>
> static int
> qemuDomainInterfacesAddresses(virDomainPtr dom,
>                                virTypedParameterPtr *params,
>                                int *nparams,
>                                unsigned int flags)
>
> static int
> remoteDomainInterfacesAddresses(virDomainPtr dom,
>                                 virTypedParameterPtr *params,
>                                 int *nparams,
>                                  unsigned int flags)

like [2]

>
> static bool
> cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd)
>
> Also, It will be helpful to know whether we want the client to first query for the number of parameters and then send the request or have the server side allocate appropriate memory and return the result once in for all. In the latter case, I'll be using something of the kind virTypedParameterPtr ***params.

instead of such long text to explain how the api works. better to 
describe the api and flags, etc by comments, and with less text to 
explain if it's necessaey. more eASY for reviewing and you can directly 
use them in later patches. e.g

/**
  * virDomainGetBlockIoTune:
  * @dom: pointer to domain object
  * @disk: path to the block device, or device shorthand
  * @params: Pointer to blkio parameter object
  *          (return value, allocated by the caller)
  * @nparams: Pointer to number of blkio parameters
  * @flags: bitwise-OR of virDomainModificationImpact and 
virTypedParameterFlags
  *
  * Get all block IO tunable parameters for a given device.  On input,
  * @nparams gives the size of the @params array; on output, @nparams
  * gives how many slots were filled with parameter information, which
  * might be less but will not exceed the input value.
  *
  * As a special case, calling with @params as NULL and @nparams as 0
  * on input will cause @nparams on output to contain the number of
  * parameters supported by the hypervisor, either for the given @disk
  * (note that block devices of different types might support different
  * parameters), or if @disk is NULL, for all possible disks. The
  * caller should then allocate @params array,
  * i.e. (sizeof(@virTypedParameter) * @nparams) bytes and call the API
  * again.  See virDomainGetMemoryParameters() for more details.
  *
  * The @disk parameter is either an unambiguous source name of the
  * block device (the <source file='...'/> sub-element, such as
  * "/path/to/image"), or the device target shorthand (the <target
  * dev='...'/> sub-element, such as "xvda").  Valid names can be found
  * by calling virDomainGetXMLDesc() and inspecting elements
  * within //domain/devices/disk.  This parameter cannot be NULL
  * unless @nparams is 0 on input.
  *
  * Returns -1 in case of error, 0 in case of success.
  */
int virDomainGetBlockIoTune(virDomainPtr dom,
                             const char *disk,
                             virTypedParameterPtr params,
                             int *nparams,
                             unsigned int flags)



>
> Thanking You,
> Nehal J. Wani
> UG2, BTech CS+MS(CL)
> IIIT-Hyderabad
> http://commanlinewani.blogspot.com
>
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130702/9383671b/attachment-0001.htm>


More information about the libvir-list mailing list