Re: [PATCH v10 3/4] domifaddr: Implement the API for qemu

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 16, 2015 at 08:42:49AM -0400, John Ferlan wrote:
> 
> 
> On 03/11/2015 07:09 AM, Daniel P. Berrange wrote:
> > From: Nehal J Wani <nehaljw.kkd1@xxxxxxxxx>
> > 
> > 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@xxxxxxxxx>
> > ---
> >  src/qemu/qemu_agent.c  | 204 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/qemu/qemu_agent.h  |   4 +
> >  src/qemu/qemu_driver.c | 175 ++++++++++++++++++++++++++++++++++++++++++
> >  tests/qemuagenttest.c  | 188 +++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 571 insertions(+)
> > 
> 
> ACK - there's a NIT and a comment that follows, I'll let you decide how
> to handle.
> 
> John
> > diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> > index 5fcc40f..8155006 100644
> > --- a/src/qemu/qemu_agent.c
> > +++ b/src/qemu/qemu_agent.c
> > @@ -1953,3 +1953,207 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,
> >      virJSONValueFree(reply);
> >      return ret;
> >  }
> > +
> > +/*
> > + * qemuAgentGetInterfaces:
> > + * @mon: Agent monitor
> > + * @ifaces: pointer to an array of pointers pointing to interface objects
> > + *
> > + * Issue guest-network-get-interfaces to guest agent, which returns a
> > + * list of interfaces of a running domain along with their IP and MAC
> > + * addresses.
> > + *
> > + * Returns: number of interfaces on success, -1 on error.
> > + */
> > +int
> > +qemuAgentGetInterfaces(qemuAgentPtr mon,
> > +                       virDomainInterfacePtr **ifaces)
> > +{
> > +    int ret = -1;
> > +    size_t i, j;
> > +    int size = -1;
> > +    virJSONValuePtr cmd = NULL;
> > +    virJSONValuePtr reply = NULL;
> > +    virJSONValuePtr ret_array = NULL;
> > +    size_t ifaces_count = 0;
> > +    size_t addrs_count = 0;
> > +    virDomainInterfacePtr *ifaces_ret = NULL;
> > +    virHashTablePtr ifaces_store = NULL;
> > +    char **ifname = NULL;
> > +
> > +    /* Hash table to handle the interface alias */
> > +    if (!(ifaces_store = virHashCreate(ifaces_count, NULL))) {
> > +        virHashFree(ifaces_store);
> > +        return -1;
> > +    }
> > +
> > +    if (!(cmd = qemuAgentMakeCommand("guest-network-get-interfaces", NULL)))
> > +        goto cleanup;
> > +
> > +    if (qemuAgentCommand(mon, cmd, &reply, false, VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0 ||
> > +        qemuAgentCheckError(cmd, reply) < 0) {
> > +        goto cleanup;
> > +    }
> > +
> > +    if (!(ret_array = virJSONValueObjectGet(reply, "return"))) {
> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +                       _("qemu agent didn't provide 'return' field"));
> > +        goto cleanup;
> > +    }
> > +
> > +    if ((size = virJSONValueArraySize(ret_array)) < 0) {
> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +                       _("qemu agent didn't return an array of interfaces"));
> > +        goto cleanup;
> > +    }
> > +
> > +    for (i = 0; i < size; i++) {
> > +        virJSONValuePtr tmp_iface = virJSONValueArrayGet(ret_array, i);
> > +        virJSONValuePtr ip_addr_arr = NULL;
> > +        const char *hwaddr, *ifname_s, *name = NULL;
> > +        int ip_addr_arr_size;
> > +        virDomainInterfacePtr iface = NULL;
> > +
> > +        /* Shouldn't happen but doesn't hurt to check neither */
> > +        if (!tmp_iface) {
> > +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +                           _("something has went really wrong"));
> > +            goto error;
> > +        }
> > +
> > +        /* interface name is required to be presented */
> > +        name = virJSONValueObjectGetString(tmp_iface, "name");
> > +        if (!name) {
> > +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +                           _("qemu agent didn't provide 'name' field"));
> > +            goto error;
> > +        }
> > +
> > +        /* Handle interface alias (<ifname>:<alias>) */
> > +        ifname = virStringSplit(name, ":", 2);
> > +        ifname_s = ifname[0];
> > +
> > +        iface = virHashLookup(ifaces_store, ifname_s);
> > +
> > +        /* If the hash table doesn't contain this iface, add it */
> > +        if (!iface) {
> > +            if (VIR_EXPAND_N(ifaces_ret, ifaces_count, 1) < 0)
> > +                goto error;
> > +
> > +            if (VIR_ALLOC(ifaces_ret[ifaces_count - 1]) < 0)
> > +                goto error;
> > +
> > +            if (virHashAddEntry(ifaces_store, ifname_s,
> > +                                ifaces_ret[ifaces_count - 1]) < 0)
> > +                goto error;
> > +
> > +            iface = ifaces_ret[ifaces_count - 1];
> > +            iface->naddrs = 0;
> > +
> > +            if (VIR_STRDUP(iface->name, ifname_s) < 0)
> > +                goto error;
> > +
> > +            hwaddr = virJSONValueObjectGetString(tmp_iface, "hardware-address");
> > +            if (!hwaddr) {
> > +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +                               _("qemu agent didn't provide"
> > +                                 " 'hardware-address' field"));
> > +                goto error;
> > +            }
> > +
> > +            if (VIR_STRDUP(iface->hwaddr, hwaddr) < 0)
> > +                goto error;
> > +        }
> > +
> > +        /* Has to be freed for each interface. */
> > +        virStringFreeList(ifname);
> > +
> > +        /* as well as IP address which - moreover -
> > +         * can be presented multiple times */
> > +        ip_addr_arr = virJSONValueObjectGet(tmp_iface, "ip-addresses");
> > +        if (!ip_addr_arr)
> > +            continue;
> > +
> 
> NIT: Extra line here
> 
> > +
> > +        if ((ip_addr_arr_size = virJSONValueArraySize(ip_addr_arr)) < 0)
> > +            /* Mmm, empty 'ip-address'? */
> > +            goto error;
> > +
> > +        /* If current iface already exists, continue with the count */
> > +        addrs_count = iface->naddrs;
> > +
> > +        for (j = 0; j < ip_addr_arr_size; j++) {
> > +            const char *type, *addr;
> > +            virJSONValuePtr ip_addr_obj = virJSONValueArrayGet(ip_addr_arr, j);
> > +            virDomainIPAddressPtr ip_addr;
> > +
> > +            if (VIR_EXPAND_N(iface->addrs, addrs_count, 1)  < 0)
> > +                goto error;
> > +
> > +            ip_addr = &iface->addrs[addrs_count - 1];
> > +
> > +            /* Shouldn't happen but doesn't hurt to check neither */
> > +            if (!ip_addr_obj) {
> > +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +                               _("something has went really wrong"));
> 
> Cannot remember if this text caught my attention the first time, but it
> is humorous...

I'll put a more sensible error message in there.


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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]