Re: [libvirt] [PATCHv5 1/5] domifaddr: Implement the public APIs
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;
+ virDrvDomainInterfaceAddresses domainInterfaceAddresses;
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
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)