Re: [libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-08 Thread Osier Yang

On 06/09/13 22:24, Nehal J Wani wrote:

On Tue, Sep 3, 2013 at 6:51 PM, Osier Yang jy...@redhat.com wrote:

On 01/09/13 21:43, Nehal J Wani wrote:

Define a new API virDomainInterfaceAddresses, which returns
the address information of a running domain's interfaces(s).
If no interface name is specified, it returns the information
of all interfaces, otherwise it only returns the information
of the specificed interface. The address information includes
the MAC and IP addresses.


This API only returns all of the interfaces' address info, it
doesn't take an argument to specify an interface. My bad
though, didn't find it out in previous reviewing.

ACK with the commit log fixed.

Osier

The optional argument interface is supported. I had forgotten to add
it in the example.


I don't see it in the API interface, may be you mean the virsh option.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-06 Thread Nehal J Wani
On Tue, Sep 3, 2013 at 6:51 PM, Osier Yang jy...@redhat.com wrote:
 On 01/09/13 21:43, Nehal J Wani wrote:

 Define a new API virDomainInterfaceAddresses, which returns
 the address information of a running domain's interfaces(s).
 If no interface name is specified, it returns the information
 of all interfaces, otherwise it only returns the information
 of the specificed interface. The address information includes
 the MAC and IP addresses.


 This API only returns all of the interfaces' address info, it
 doesn't take an argument to specify an interface. My bad
 though, didn't find it out in previous reviewing.

 ACK with the commit log fixed.

 Osier

The optional argument interface is supported. I had forgotten to add
it in the example.

-- 
Nehal J Wani
UG3, BTech CS+MS(CL)
IIIT-Hyderabad
http://commandlinewani.blogspot.com

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-05 Thread Nehal J Wani
On Wed, Sep 4, 2013 at 8:05 PM, Osier Yang jy...@redhat.com wrote:
 On 04/09/13 03:13, Eric Blake wrote:

 On 09/03/2013 01:07 PM, Eric Blake wrote:

 On 09/01/2013 07:43 AM, Nehal J Wani wrote:

 Define a new API virDomainInterfaceAddresses, which returns
 the address information of a running domain's interfaces(s).
 If no interface name is specified, it returns the information
 of all interfaces, otherwise it only returns the information
 of the specificed interface. The address information includes
 the MAC and IP addresses.

 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 lease file of dnsmasq
   * DHCP snooping

 But at this stage, it will only work with guest agent, and flags
 won't be supported.

 That worries me a bit.  Ultimately, we want our interfaces to behave as
 sane as possible when flags==0; rather than making the behavior be that
 'flags==0' implies 'only guest agent probe', I'd rather introduce a flag
 right away up front that says 'include guest agent probe in the set of
 attempted methods', and then document that 'flags==0 is shorthand for
 letting the hypervisor choose the best method(s) out of supported
 possibilities)'.  In other words, I want to make sure that this API will
 be similar to virDomainShutdownFlags, where a flags of 0 lets the
 hypervisor choose between methods, a single explicit flag forces the
 hypervisor to use that method alone, and more than one flag can be OR'd
 together to let the hypervisor choose among that subset of flags.

 Hmm.  I'm replying to myself - is that a good sign?

 If the guest agent returns names that are provided by the guest, and
 don't necessarily correspond to the domain XML, then maybe it's best to
 NEVER return guest results by default, but to make the user always
 explicitly request agent interaction.


 Hm, yes, the MAC address returned by guest agent might not
 match what the domain config specifies. It reminds me something,
 both the leases file and snooping will returns the interface name
 like vnetN, which is different with what guest agent returns (like
 ethN or emN). And since the MAC address from guest agent might
 be different with what domain config specifies, we have no way to
 convert it into the names in domain config. That says we will have
 different name styles for guest agent and the other two methods,
 which will need quite documentations to explain.

 That is, even if 'flags==0' is
 used to select between parsing the lease file vs. DHCP snoop results
 (both of which tie perfectly to guest XML naming), the agent approach
 can seriously confuse users if they passed flags==0 and don't know if
 they are getting XML names or guest-provided names.  Which argues that
 guest results should ALWAYS be an explicit non-zero flag, never an
 automatic action.



 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list



Daniel, would like to throw some light on the discussion?

-- 
Nehal J Wani

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-05 Thread Daniel P. Berrange
On Tue, Sep 03, 2013 at 01:13:20PM -0600, Eric Blake wrote:
 On 09/03/2013 01:07 PM, Eric Blake wrote:
  On 09/01/2013 07:43 AM, Nehal J Wani wrote:
  Define a new API virDomainInterfaceAddresses, which returns
  the address information of a running domain's interfaces(s).
  If no interface name is specified, it returns the information
  of all interfaces, otherwise it only returns the information
  of the specificed interface. The address information includes
  the MAC and IP addresses.
 
  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 lease file of dnsmasq
* DHCP snooping
 
  But at this stage, it will only work with guest agent, and flags
  won't be supported.
  
  That worries me a bit.  Ultimately, we want our interfaces to behave as
  sane as possible when flags==0; rather than making the behavior be that
  'flags==0' implies 'only guest agent probe', I'd rather introduce a flag
  right away up front that says 'include guest agent probe in the set of
  attempted methods', and then document that 'flags==0 is shorthand for
  letting the hypervisor choose the best method(s) out of supported
  possibilities)'.  In other words, I want to make sure that this API will
  be similar to virDomainShutdownFlags, where a flags of 0 lets the
  hypervisor choose between methods, a single explicit flag forces the
  hypervisor to use that method alone, and more than one flag can be OR'd
  together to let the hypervisor choose among that subset of flags.
 
 Hmm.  I'm replying to myself - is that a good sign?
 
 If the guest agent returns names that are provided by the guest, and
 don't necessarily correspond to the domain XML, then maybe it's best to
 NEVER return guest results by default, but to make the user always
 explicitly request agent interaction.  That is, even if 'flags==0' is
 used to select between parsing the lease file vs. DHCP snoop results
 (both of which tie perfectly to guest XML naming), the agent approach
 can seriously confuse users if they passed flags==0 and don't know if
 they are getting XML names or guest-provided names.  Which argues that
 guest results should ALWAYS be an explicit non-zero flag, never an
 automatic action.

Yes, that sounds like a good idea. The other reason to return DHCP
snoop results by default is that those will work for any guest
without requiring an agent installed.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-05 Thread Daniel P. Berrange
On Wed, Sep 04, 2013 at 10:35:34PM +0800, Osier Yang wrote:
 On 04/09/13 03:13, Eric Blake wrote:
 On 09/03/2013 01:07 PM, Eric Blake wrote:
 On 09/01/2013 07:43 AM, Nehal J Wani wrote:
 Define a new API virDomainInterfaceAddresses, which returns
 the address information of a running domain's interfaces(s).
 If no interface name is specified, it returns the information
 of all interfaces, otherwise it only returns the information
 of the specificed interface. The address information includes
 the MAC and IP addresses.
 
 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 lease file of dnsmasq
* DHCP snooping
 
 But at this stage, it will only work with guest agent, and flags
 won't be supported.
 That worries me a bit.  Ultimately, we want our interfaces to behave as
 sane as possible when flags==0; rather than making the behavior be that
 'flags==0' implies 'only guest agent probe', I'd rather introduce a flag
 right away up front that says 'include guest agent probe in the set of
 attempted methods', and then document that 'flags==0 is shorthand for
 letting the hypervisor choose the best method(s) out of supported
 possibilities)'.  In other words, I want to make sure that this API will
 be similar to virDomainShutdownFlags, where a flags of 0 lets the
 hypervisor choose between methods, a single explicit flag forces the
 hypervisor to use that method alone, and more than one flag can be OR'd
 together to let the hypervisor choose among that subset of flags.
 Hmm.  I'm replying to myself - is that a good sign?
 
 If the guest agent returns names that are provided by the guest, and
 don't necessarily correspond to the domain XML, then maybe it's best to
 NEVER return guest results by default, but to make the user always
 explicitly request agent interaction.
 
 Hm, yes, the MAC address returned by guest agent might not
 match what the domain config specifies. It reminds me something,
 both the leases file and snooping will returns the interface name
 like vnetN, which is different with what guest agent returns (like
 ethN or emN). And since the MAC address from guest agent might
 be different with what domain config specifies, we have no way to
 convert it into the names in domain config. That says we will have
 different name styles for guest agent and the other two methods,
 which will need quite documentations to explain.

Such is life. We just have to document these naming limitations.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-04 Thread Osier Yang

On 04/09/13 03:07, Eric Blake wrote:

On 09/01/2013 07:43 AM, Nehal J Wani wrote:

Define a new API virDomainInterfaceAddresses, which returns
the address information of a running domain's interfaces(s).
If no interface name is specified, it returns the information
of all interfaces, otherwise it only returns the information
of the specificed interface. The address information includes
the MAC and IP addresses.

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 lease file of dnsmasq
   * DHCP snooping

But at this stage, it will only work with guest agent, and flags
won't be supported.

That worries me a bit.  Ultimately, we want our interfaces to behave as
sane as possible when flags==0; rather than making the behavior be that
'flags==0' implies 'only guest agent probe', I'd rather introduce a flag
right away up front that says 'include guest agent probe in the set of
attempted methods', and then document that 'flags==0 is shorthand for
letting the hypervisor choose the best method(s) out of supported
possibilities)'.  In other words, I want to make sure that this API will
be similar to virDomainShutdownFlags, where a flags of 0 lets the
hypervisor choose between methods, a single explicit flag forces the
hypervisor to use that method alone, and more than one flag can be OR'd
together to let the hypervisor choose among that subset of flags.


I can't find any reason to say this is not good. Except that I'm wondering
which the upper layer developer prefer more, the more straightforward
style we used for the flags of old APIs (flags==0 defaults to a specific
flag), or the style like this (flags==0 defaults to letting the hypervisor
choosing it, and even choosing between subset of the OR'ed flags)?





+char *addr;  /* IP address */
+unsigned int prefix; /* IP address prefix */

Do we ever intend to support non-CIDR IPv4 addresses (where the mask is
not contiguous bits)?  Or are we intentionally documenting that the
prefix will always be possible because we always require the mask to be
a contiguous prefix?


We don't have the way to require it, it's returned either from guest
agent, leases file, and traffic snooping, unless we reject if they
returns non-contiguous bits for the subnet mask, but that doesn't
make sense.

So I'm wondering if we should introduce a new member for the netmask,
though qemu guest agent doesn't return anything for the netmask
(I guess it always assumes the netmask bits are contiguous), for
the other 2 methods, we still might faces the non-contiguous netmask
bits.




@@ -1238,6 +1243,7 @@ struct _virDriver {
  virDrvDomainInterfaceStats domainInterfaceStats;
  virDrvDomainSetInterfaceParameters domainSetInterfaceParameters;
  virDrvDomainGetInterfaceParameters domainGetInterfaceParameters;
+virDrvDomainInterfaceAddressesdomainInterfaceAddresses;

Spacing is off; only one space needed.


+ *
+ * cleanup:
+ * if (ifaces)
+ * for (i = 0; i  ifaces_count; i++)
+ * virDomainInterfaceFree(ifaces[i]);
+ * VIR_FREE(ifaces);

VIR_FREE() is not a public function; this comment should instead mention
free(); because it is in a comment, it will not trigger our syntax check
rules.


+int
+virDomainInterfaceAddresses(virDomainPtr dom,
+virDomainInterfacePtr **ifaces,
+unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(dom, ifaces=%p, flags=%x, ifaces, flags);
+
+virResetLastError();
+
+if (!VIR_IS_CONNECTED_DOMAIN(dom)) {
+virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+virDispatchError(NULL);
+return -1;
+}
+
+conn = dom-conn;

Putting on my security hat - anything that requires guest interaction
should probably not be permitted from a read-only connection (because a
malicious guest coupled with a read-only caller purposefully exploiting
the guest's intentional bad behavior might open up a denial of service
against read-write clients).  Again, all the more reason why I think you
should require non-zero flags before permitting guest agent interaction,
so that we can then add a security check here (if you pass flags=0, you
get the default set of actions that are safe for a read-only client; but
if you pass flags=..._AGENT, then we can prevent the call from
succeeding on a read-only client).

Overall, I'm happy with what we finally settled on for the API, and my
questions now only involve whether we need a non-zero flag before
allowing agent interaction.



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-04 Thread Osier Yang

On 04/09/13 03:13, Eric Blake wrote:

On 09/03/2013 01:07 PM, Eric Blake wrote:

On 09/01/2013 07:43 AM, Nehal J Wani wrote:

Define a new API virDomainInterfaceAddresses, which returns
the address information of a running domain's interfaces(s).
If no interface name is specified, it returns the information
of all interfaces, otherwise it only returns the information
of the specificed interface. The address information includes
the MAC and IP addresses.

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 lease file of dnsmasq
   * DHCP snooping

But at this stage, it will only work with guest agent, and flags
won't be supported.

That worries me a bit.  Ultimately, we want our interfaces to behave as
sane as possible when flags==0; rather than making the behavior be that
'flags==0' implies 'only guest agent probe', I'd rather introduce a flag
right away up front that says 'include guest agent probe in the set of
attempted methods', and then document that 'flags==0 is shorthand for
letting the hypervisor choose the best method(s) out of supported
possibilities)'.  In other words, I want to make sure that this API will
be similar to virDomainShutdownFlags, where a flags of 0 lets the
hypervisor choose between methods, a single explicit flag forces the
hypervisor to use that method alone, and more than one flag can be OR'd
together to let the hypervisor choose among that subset of flags.

Hmm.  I'm replying to myself - is that a good sign?

If the guest agent returns names that are provided by the guest, and
don't necessarily correspond to the domain XML, then maybe it's best to
NEVER return guest results by default, but to make the user always
explicitly request agent interaction.


Hm, yes, the MAC address returned by guest agent might not
match what the domain config specifies. It reminds me something,
both the leases file and snooping will returns the interface name
like vnetN, which is different with what guest agent returns (like
ethN or emN). And since the MAC address from guest agent might
be different with what domain config specifies, we have no way to
convert it into the names in domain config. That says we will have
different name styles for guest agent and the other two methods,
which will need quite documentations to explain.


That is, even if 'flags==0' is
used to select between parsing the lease file vs. DHCP snoop results
(both of which tie perfectly to guest XML naming), the agent approach
can seriously confuse users if they passed flags==0 and don't know if
they are getting XML names or guest-provided names.  Which argues that
guest results should ALWAYS be an explicit non-zero flag, never an
automatic action.



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-03 Thread Osier Yang

On 01/09/13 21:43, Nehal J Wani wrote:

Define a new API virDomainInterfaceAddresses, which returns
the address information of a running domain's interfaces(s).
If no interface name is specified, it returns the information
of all interfaces, otherwise it only returns the information
of the specificed interface. The address information includes
the MAC and IP addresses.


This API only returns all of the interfaces' address info, it
doesn't take an argument to specify an interface. My bad
though, didn't find it out in previous reviewing.

ACK with the commit log fixed.

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-03 Thread Eric Blake
On 09/02/2013 03:41 AM, Nehal J Wani wrote:

 In agreement with Guido's suggestion on 5/5 of the previous version
 (https://www.redhat.com/archives/libvir-list/2013-August/msg01271.html),
 the enums are also to exposed to python bindings, for which,
 /include/libvirt/libvirt.h.in will have to be modified a bit. Diff has
 been attached.

Yes, the constants need to be exposed in python bindings.  However,


  
  typedef enum {
 -VIR_IP_ADDR_TYPE_IPV4,
 -VIR_IP_ADDR_TYPE_IPV6,
 +VIR_IP_ADDR_TYPE_IPV4 = (1  0),
 +VIR_IP_ADDR_TYPE_IPV6 = (1  1),

This is not the correct way to do so.  These are not bitmasks, but
should be sequential:

VIR_IP_ADDR_TYPE_IPV4 = 0,
VIR_IP_ADDR_TYPE_IPV6 = 1,

/* next one would be 2, ... */

  
  #ifdef VIR_ENUM_SENTINELS
 -VIR_IP_ADDR_TYPE_LAST
 +VIR_IP_ADDR_TYPE_LAST = (1  2),

and this should NOT have an explicit value (the _LAST enum is magic, and
should always be 1 more than the previous value; it is also protected
behind #ifdef because there are some contexts that should not be exposed
to the _LAST value).  I don't remember off the top of my head, but think
that python bindings are one of the places where we intentionally don't
expose _LAST values.

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

Re: [libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-03 Thread Eric Blake
On 09/01/2013 07:43 AM, Nehal J Wani wrote:
 Define a new API virDomainInterfaceAddresses, which returns
 the address information of a running domain's interfaces(s).
 If no interface name is specified, it returns the information
 of all interfaces, otherwise it only returns the information
 of the specificed interface. The address information includes
 the MAC and IP addresses.
 
 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 lease file of dnsmasq
   * DHCP snooping
 
 But at this stage, it will only work with guest agent, and flags
 won't be supported.

That worries me a bit.  Ultimately, we want our interfaces to behave as
sane as possible when flags==0; rather than making the behavior be that
'flags==0' implies 'only guest agent probe', I'd rather introduce a flag
right away up front that says 'include guest agent probe in the set of
attempted methods', and then document that 'flags==0 is shorthand for
letting the hypervisor choose the best method(s) out of supported
possibilities)'.  In other words, I want to make sure that this API will
be similar to virDomainShutdownFlags, where a flags of 0 lets the
hypervisor choose between methods, a single explicit flag forces the
hypervisor to use that method alone, and more than one flag can be OR'd
together to let the hypervisor choose among that subset of flags.

 +char *addr;  /* IP address */
 +unsigned int prefix; /* IP address prefix */

Do we ever intend to support non-CIDR IPv4 addresses (where the mask is
not contiguous bits)?  Or are we intentionally documenting that the
prefix will always be possible because we always require the mask to be
a contiguous prefix?

 @@ -1238,6 +1243,7 @@ struct _virDriver {
  virDrvDomainInterfaceStats domainInterfaceStats;
  virDrvDomainSetInterfaceParameters domainSetInterfaceParameters;
  virDrvDomainGetInterfaceParameters domainGetInterfaceParameters;
 +virDrvDomainInterfaceAddressesdomainInterfaceAddresses;

Spacing is off; only one space needed.

 + *
 + * cleanup:
 + * if (ifaces)
 + * for (i = 0; i  ifaces_count; i++)
 + * virDomainInterfaceFree(ifaces[i]);
 + * VIR_FREE(ifaces);

VIR_FREE() is not a public function; this comment should instead mention
free(); because it is in a comment, it will not trigger our syntax check
rules.

 +int
 +virDomainInterfaceAddresses(virDomainPtr dom,
 +virDomainInterfacePtr **ifaces,
 +unsigned int flags)
 +{
 +virConnectPtr conn;
 +
 +VIR_DOMAIN_DEBUG(dom, ifaces=%p, flags=%x, ifaces, flags);
 +
 +virResetLastError();
 +
 +if (!VIR_IS_CONNECTED_DOMAIN(dom)) {
 +virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
 +virDispatchError(NULL);
 +return -1;
 +}
 +
 +conn = dom-conn;

Putting on my security hat - anything that requires guest interaction
should probably not be permitted from a read-only connection (because a
malicious guest coupled with a read-only caller purposefully exploiting
the guest's intentional bad behavior might open up a denial of service
against read-write clients).  Again, all the more reason why I think you
should require non-zero flags before permitting guest agent interaction,
so that we can then add a security check here (if you pass flags=0, you
get the default set of actions that are safe for a read-only client; but
if you pass flags=..._AGENT, then we can prevent the call from
succeeding on a read-only client).

Overall, I'm happy with what we finally settled on for the API, and my
questions now only involve whether we need a non-zero flag before
allowing agent interaction.

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

Re: [libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-03 Thread Eric Blake
On 09/03/2013 01:07 PM, Eric Blake wrote:
 On 09/01/2013 07:43 AM, Nehal J Wani wrote:
 Define a new API virDomainInterfaceAddresses, which returns
 the address information of a running domain's interfaces(s).
 If no interface name is specified, it returns the information
 of all interfaces, otherwise it only returns the information
 of the specificed interface. The address information includes
 the MAC and IP addresses.

 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 lease file of dnsmasq
   * DHCP snooping

 But at this stage, it will only work with guest agent, and flags
 won't be supported.
 
 That worries me a bit.  Ultimately, we want our interfaces to behave as
 sane as possible when flags==0; rather than making the behavior be that
 'flags==0' implies 'only guest agent probe', I'd rather introduce a flag
 right away up front that says 'include guest agent probe in the set of
 attempted methods', and then document that 'flags==0 is shorthand for
 letting the hypervisor choose the best method(s) out of supported
 possibilities)'.  In other words, I want to make sure that this API will
 be similar to virDomainShutdownFlags, where a flags of 0 lets the
 hypervisor choose between methods, a single explicit flag forces the
 hypervisor to use that method alone, and more than one flag can be OR'd
 together to let the hypervisor choose among that subset of flags.

Hmm.  I'm replying to myself - is that a good sign?

If the guest agent returns names that are provided by the guest, and
don't necessarily correspond to the domain XML, then maybe it's best to
NEVER return guest results by default, but to make the user always
explicitly request agent interaction.  That is, even if 'flags==0' is
used to select between parsing the lease file vs. DHCP snoop results
(both of which tie perfectly to guest XML naming), the agent approach
can seriously confuse users if they passed flags==0 and don't know if
they are getting XML names or guest-provided names.  Which argues that
guest results should ALWAYS be an explicit non-zero flag, never an
automatic action.

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

Re: [libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-02 Thread Nehal J Wani
On Sun, Sep 1, 2013 at 7:13 PM, Nehal J Wani nehaljw.k...@gmail.com wrote:
 Define a new API virDomainInterfaceAddresses, which returns
 the address information of a running domain's interfaces(s).
 If no interface name is specified, it returns the information
 of all interfaces, otherwise it only returns the information
 of the specificed interface. The address information includes
 the MAC and IP addresses.

 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 lease file of dnsmasq
   * DHCP snooping

 But at this stage, it will only work with guest agent, and flags
 won't be supported.

 include/libvirt/libvirt.h.in:
   * Define virDomainInterfaceAddresses, virDomainInterfaceFree
   * Define structs virDomainInterface, virDomainIPAddress

 python/generator.py:
   * Skip the auto-generation for virDomainInterfaceAddresses
 and virDomainInterfaceFree

 src/driver.h:
   * Define domainInterfaceAddresses

 src/libvirt.c:
   * Implement virDomainInterfaceAddresses
   * Implement virDomainInterfaceFree

 src/libvirt_public.syms:
   * Export the new symbols

 ---
  include/libvirt/libvirt.h.in |  32 
  python/generator.py  |   3 ++
  src/driver.h |   6 +++
  src/libvirt.c| 115 
 +++
  src/libvirt_public.syms  |   6 +++
  5 files changed, 162 insertions(+)

 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index a47e33c..1a34d02 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -2044,6 +2044,38 @@ int 
 virDomainGetInterfaceParameters (virDomainPtr dom,
   
 virTypedParameterPtr params,
   int *nparams, 
 unsigned int flags);

 +typedef enum {
 +VIR_IP_ADDR_TYPE_IPV4,
 +VIR_IP_ADDR_TYPE_IPV6,
 +
 +#ifdef VIR_ENUM_SENTINELS
 +VIR_IP_ADDR_TYPE_LAST
 +#endif
 +} virIPAddrType;
 +
 +typedef struct _virDomainInterfaceIPAddress virDomainIPAddress;
 +typedef virDomainIPAddress *virDomainIPAddressPtr;
 +struct _virDomainInterfaceIPAddress {
 +int type;/* virIPAddrType */
 +char *addr;  /* IP address */
 +unsigned int prefix; /* IP address prefix */
 +};
 +
 +typedef struct _virDomainInterface virDomainInterface;
 +typedef virDomainInterface *virDomainInterfacePtr;
 +struct _virDomainInterface {
 +char *name; /* interface name */
 +char *hwaddr;   /* hardware address */
 +unsigned int naddrs;/* number of items in @addrs */
 +virDomainIPAddressPtr addrs;/* array of IP addresses */
 +};
 +
 +int virDomainInterfaceAddresses(virDomainPtr dom,
 +virDomainInterfacePtr **ifaces,
 +unsigned int flags);
 +
 +void virDomainInterfaceFree(virDomainInterfacePtr iface);
 +
  /* Management of domain block devices */

  int virDomainBlockPeek (virDomainPtr dom,
 diff --git a/python/generator.py b/python/generator.py
 index fb321c6..50f779b 100755
 --- a/python/generator.py
 +++ b/python/generator.py
 @@ -458,6 +458,7 @@ skip_impl = (
  'virNodeGetMemoryParameters',
  'virNodeSetMemoryParameters',
  'virNodeGetCPUMap',
 +'virDomainInterfaceAddresses',
  'virDomainMigrate3',
  'virDomainMigrateToURI3',
  )
 @@ -560,6 +561,8 @@ skip_function = (
  virTypedParamsGetString,
  virTypedParamsGetUInt,
  virTypedParamsGetULLong,
 +
 +virDomainInterfaceFree, # Only useful in C
  )

  lxc_skip_function = (
 diff --git a/src/driver.h b/src/driver.h
 index be64333..eb4927b 100644
 --- a/src/driver.h
 +++ b/src/driver.h
 @@ -518,6 +518,11 @@ typedef int
unsigned int flags);

  typedef int
 +(*virDrvDomainInterfaceAddresses)(virDomainPtr dom,
 +  virDomainInterfacePtr **ifaces,
 +  unsigned int flags);
 +
 +typedef int
  (*virDrvDomainMemoryStats)(virDomainPtr domain,
 struct _virDomainMemoryStat *stats,
 unsigned int nr_stats,
 @@ -1238,6 +1243,7 @@ struct _virDriver {
  virDrvDomainInterfaceStats domainInterfaceStats;
  virDrvDomainSetInterfaceParameters domainSetInterfaceParameters;
  virDrvDomainGetInterfaceParameters domainGetInterfaceParameters;
 +virDrvDomainInterfaceAddressesdomainInterfaceAddresses;
  virDrvDomainMemoryStats domainMemoryStats;
  virDrvDomainBlockPeek domainBlockPeek;
  virDrvDomainMemoryPeek domainMemoryPeek;
 diff --git a/src/libvirt.c b/src/libvirt.c
 index 07a3fd5..82c117f 100644
 --- a/src/libvirt.c
 +++ b/src/libvirt.c
 

Re: [libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-02 Thread Daniel P. Berrange
On Sun, Sep 01, 2013 at 07:13:31PM +0530, Nehal J Wani wrote:
 Define a new API virDomainInterfaceAddresses, which returns
 the address information of a running domain's interfaces(s).
 If no interface name is specified, it returns the information
 of all interfaces, otherwise it only returns the information
 of the specificed interface. The address information includes
 the MAC and IP addresses.
 
 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 lease file of dnsmasq
   * DHCP snooping
 
 But at this stage, it will only work with guest agent, and flags
 won't be supported.
 
 include/libvirt/libvirt.h.in:
   * Define virDomainInterfaceAddresses, virDomainInterfaceFree
   * Define structs virDomainInterface, virDomainIPAddress
 
 python/generator.py:
   * Skip the auto-generation for virDomainInterfaceAddresses
 and virDomainInterfaceFree
 
 src/driver.h:
   * Define domainInterfaceAddresses
 
 src/libvirt.c:
   * Implement virDomainInterfaceAddresses
   * Implement virDomainInterfaceFree
 
 src/libvirt_public.syms:
   * Export the new symbols
 
 ---
  include/libvirt/libvirt.h.in |  32 
  python/generator.py  |   3 ++
  src/driver.h |   6 +++
  src/libvirt.c| 115 
 +++
  src/libvirt_public.syms  |   6 +++
  5 files changed, 162 insertions(+)

ACK

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs

2013-09-01 Thread Nehal J Wani
Define a new API virDomainInterfaceAddresses, which returns
the address information of a running domain's interfaces(s).
If no interface name is specified, it returns the information
of all interfaces, otherwise it only returns the information
of the specificed interface. The address information includes
the MAC and IP addresses.

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 lease file of dnsmasq
  * DHCP snooping

But at this stage, it will only work with guest agent, and flags
won't be supported.

include/libvirt/libvirt.h.in:
  * Define virDomainInterfaceAddresses, virDomainInterfaceFree
  * Define structs virDomainInterface, virDomainIPAddress

python/generator.py:
  * Skip the auto-generation for virDomainInterfaceAddresses
and virDomainInterfaceFree

src/driver.h:
  * Define domainInterfaceAddresses

src/libvirt.c:
  * Implement virDomainInterfaceAddresses
  * Implement virDomainInterfaceFree

src/libvirt_public.syms:
  * Export the new symbols

---
 include/libvirt/libvirt.h.in |  32 
 python/generator.py  |   3 ++
 src/driver.h |   6 +++
 src/libvirt.c| 115 +++
 src/libvirt_public.syms  |   6 +++
 5 files changed, 162 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index a47e33c..1a34d02 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2044,6 +2044,38 @@ int virDomainGetInterfaceParameters 
(virDomainPtr dom,
  virTypedParameterPtr 
params,
  int *nparams, 
unsigned int flags);
 
+typedef enum {
+VIR_IP_ADDR_TYPE_IPV4,
+VIR_IP_ADDR_TYPE_IPV6,
+
+#ifdef VIR_ENUM_SENTINELS
+VIR_IP_ADDR_TYPE_LAST
+#endif
+} virIPAddrType;
+
+typedef struct _virDomainInterfaceIPAddress virDomainIPAddress;
+typedef virDomainIPAddress *virDomainIPAddressPtr;
+struct _virDomainInterfaceIPAddress {
+int type;/* virIPAddrType */
+char *addr;  /* IP address */
+unsigned int prefix; /* IP address prefix */
+};
+
+typedef struct _virDomainInterface virDomainInterface;
+typedef virDomainInterface *virDomainInterfacePtr;
+struct _virDomainInterface {
+char *name; /* interface name */
+char *hwaddr;   /* hardware address */
+unsigned int naddrs;/* number of items in @addrs */
+virDomainIPAddressPtr addrs;/* array of IP addresses */
+};
+
+int virDomainInterfaceAddresses(virDomainPtr dom,
+virDomainInterfacePtr **ifaces,
+unsigned int flags);
+
+void virDomainInterfaceFree(virDomainInterfacePtr iface);
+
 /* Management of domain block devices */
 
 int virDomainBlockPeek (virDomainPtr dom,
diff --git a/python/generator.py b/python/generator.py
index fb321c6..50f779b 100755
--- a/python/generator.py
+++ b/python/generator.py
@@ -458,6 +458,7 @@ skip_impl = (
 'virNodeGetMemoryParameters',
 'virNodeSetMemoryParameters',
 'virNodeGetCPUMap',
+'virDomainInterfaceAddresses',
 'virDomainMigrate3',
 'virDomainMigrateToURI3',
 )
@@ -560,6 +561,8 @@ skip_function = (
 virTypedParamsGetString,
 virTypedParamsGetUInt,
 virTypedParamsGetULLong,
+
+virDomainInterfaceFree, # Only useful in C
 )
 
 lxc_skip_function = (
diff --git a/src/driver.h b/src/driver.h
index be64333..eb4927b 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -518,6 +518,11 @@ typedef int
   unsigned int flags);
 
 typedef int
+(*virDrvDomainInterfaceAddresses)(virDomainPtr dom,
+  virDomainInterfacePtr **ifaces,
+  unsigned int flags);
+
+typedef int
 (*virDrvDomainMemoryStats)(virDomainPtr domain,
struct _virDomainMemoryStat *stats,
unsigned int nr_stats,
@@ -1238,6 +1243,7 @@ struct _virDriver {
 virDrvDomainInterfaceStats domainInterfaceStats;
 virDrvDomainSetInterfaceParameters domainSetInterfaceParameters;
 virDrvDomainGetInterfaceParameters domainGetInterfaceParameters;
+virDrvDomainInterfaceAddressesdomainInterfaceAddresses;
 virDrvDomainMemoryStats domainMemoryStats;
 virDrvDomainBlockPeek domainBlockPeek;
 virDrvDomainMemoryPeek domainMemoryPeek;
diff --git a/src/libvirt.c b/src/libvirt.c
index 07a3fd5..82c117f 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -8643,6 +8643,96 @@ error:
 return -1;
 }
 
+ /**
+ * virDomainInterfaceAddresses:
+ * @dom: domain object
+ * @ifaces: pointer to an array of pointers pointing to interface objects
+ * @flags: