[libvirt] [PATCHv7 3/4] domifaddr: Implement the API for qemu

[Daniel P. Berrange]

On Mon, Jan 05, 2015 at 10:54:35PM +0530, Nehal J Wani wrote:
> By querying the qemu guest agent with the QMP command
> "guest-network-get-interfaces" and converting the received JSON
> output to structured objects.
> 
> Although "ifconfig" is deprecated, IP aliases created by "ifconfig"
> are supported by this API. The legacy syntax of an IP alias is:
> "<ifname>:<alias-name>". Since we want all aliases to be clubbed
> under parent interface, simply stripping ":<alias-name>" suffices.
> Note that IP aliases formed by "ip" aren't visible to "ifconfig",
> and aliases created by "ip" do not have any specific name. But
> we are lucky, as qemu guest agent detects aliases created by both.
> 
> src/qemu/qemu_agent.h:
>   * Define qemuAgentGetInterfaces
> 
> src/qemu/qemu_agent.c:
>   * Implement qemuAgentGetInterface
> 
> src/qemu/qemu_driver.c:
>   * New function qemuGetDHCPInterfaces
>   * New function qemuDomainInterfaceAddresses
> 
> src/remote_protocol-sructs:
>   * Define new structs
> 
> tests/qemuagenttest.c:
>   * Add new test: testQemuAgentGetInterfaces
>     Test cases for IP aliases, 0 or multiple ipv4/ipv6 address(es)
> 
> Signed-off-by: Nehal J Wani <nehaljw.kkd1 at gmail.com>
> ---
>  These are very minor changes, and rest of the patches in the series haven't
>  been reviewed yet. Hence sending fix in the same version. :)
>  src/qemu/qemu_agent.c  | 202 +++++++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_agent.h  |   4 +
>  src/qemu/qemu_driver.c | 159 ++++++++++++++++++++++++++++++++++++++
>  tests/qemuagenttest.c  | 188 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 553 insertions(+)

> +static int
> +qemuDomainInterfaceAddresses(virDomainPtr dom,
> +                             virDomainInterfacePtr **ifaces,
> +                             unsigned int flags)
> +{
> +    virQEMUDriverPtr driver = dom->conn->privateData;
> +    qemuDomainObjPrivatePtr priv = NULL;
> +    virDomainObjPtr vm = NULL;
> +    int ret = -1;
> +
> +    /* We won't allow agent interaction in default behavior */
> +    if (flags == 0)
> +        flags = VIR_DOMAIN_INTERFACE_ADDRESSES_LEASE;
> +
> +    virCheckFlags(VIR_DOMAIN_INTERFACE_ADDRESSES_LEASE |
> +                  VIR_DOMAIN_INTERFACE_ADDRESSES_AGENT, -1);

[snip]

> +    if (flags & VIR_DOMAIN_INTERFACE_ADDRESSES_LEASE) {
> +        ret = qemuGetDHCPInterfaces(dom, vm, ifaces);
> +        if (ret > 0)
> +            goto cleanup;
> +    }
> +
> +    if (flags & VIR_DOMAIN_INTERFACE_ADDRESSES_AGENT) {

On second thoughts, I'm not convinced this logic is the right
approach.  With this, if you don't specify any flags, no code
runs at all. Also, if qemuGetDHCPInterfaces raises a fatal
error we loose the error report & run the agent code instead
which kind of sucks.

I think we want todo something like

  bool tryLeases;
  bool tryAgent;

  if (flags) {
    tryLease =  (flags & VIR_DOMAIN_INTERFACE_ADDRESSES_LEASE);
    tryAgent = (flags & VIR_DOMAIN_INTERFACE_ADDRESSES_AGENT);
  } else {
    tryLease == ...if guest has any NICs on libvirt virtual network...
    tryAgent = priv->agent != NULL;
  }


  if (tryLease) {
      return qemuGetDHCPInterfaces(...);
  }
  if (tryAgent) {
      ...agent code...
  }

  virReportError("No IP address data source found");


> +        if (priv->agentError) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("QEMU guest agent is not "
> +                             "available due to an error"));
> +            goto cleanup;
> +        }
> +
> +        if (!priv->agent) {
> +            virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> +                           _("QEMU guest agent is not configured"));
> +            goto cleanup;
> +        }
> +
> +        if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
> +            goto cleanup;
> +
> +        qemuDomainObjEnterAgent(vm);
> +        ret = qemuAgentGetInterfaces(priv->agent, ifaces);
> +        qemuDomainObjExitAgent(vm);
> +
> +        qemuDomainObjEndJob(driver, vm);
> +    }
> +
> + cleanup:
> +    if (vm)
> +        virObjectUnlock(vm);
> +    return ret;
> +}
> +
> +static int
> +qemuGetDHCPInterfaces(virDomainPtr dom,
> +                      virDomainObjPtr vm,
> +                      virDomainInterfacePtr **ifaces)
> +{
> +    int rv = -1;
> +    int n_leases = 0;
> +    size_t i, j;
> +    size_t ifaces_count = 0;
> +    virNetworkPtr network;
> +    char macaddr[VIR_MAC_STRING_BUFLEN];
> +    virDomainInterfacePtr iface = NULL;
> +    virNetworkDHCPLeasePtr *leases = NULL;
> +    virDomainInterfacePtr *ifaces_ret = NULL;
> +
> +    if (dom->conn->networkDriver &&
> +        dom->conn->networkDriver->networkGetDHCPLeases) {
> +
> +        for (i = 0; i < vm->def->nnets; i++) {
> +            virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr);
> +            network = virNetworkLookupByName(dom->conn,
> +                                             vm->def->nets[i]->data.network.name);

You need to check  nets[i]->type == VIR_DOMAIN_NET_NETWORK before
calling this.


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