[libvirt] [PATCH v3] driver: Include source as a flag to virDomainGetHostname

Michal Privoznik mprivozn at redhat.com
Thu Jan 9 12:47:25 UTC 2020


On 12/27/19 9:36 PM, Julio Faracco wrote:
> There is a lots of possibilities to retrieve hostname information from
> domain. Libvirt could use lease information from dnsmasq to get current
> hostname too. QEMU supports QEMU-agent but it can use lease source. See
> 'domifaddr' as an example.
> 
> This commit still adds lease options for QEMU. It will get the first
> hostname available from domain networks.
> 
> This case, QEMU driver has a default section inside switch to keep
> compatibility. So, if someone call 'domhostname' without specifying
> source, it will get the default option. Some other drivers like LXC and
> OpenVZ supports only one type, virCheckFlags will handle this case.
> 
> Signed-off-by: Julio Faracco <jcfaracco at gmail.com>
> ---
> v1-v2: Moving sources into flags.
> v2-v3: Applying Michal's suggestions.
> ---
>   docs/manpages/virsh.rst          |  7 ++-
>   include/libvirt/libvirt-domain.h |  5 ++
>   src/lxc/lxc_driver.c             | 78 ++++++++++++++++++++++++++++++++
>   src/qemu/qemu_driver.c           | 76 +++++++++++++++++++++++++++----
>   tools/virsh-completer-domain.c   | 17 +++++++
>   tools/virsh-completer-domain.h   |  4 ++
>   tools/virsh-domain.c             | 26 ++++++++++-
>   7 files changed, 201 insertions(+), 12 deletions(-)
> 
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index fea0527caf..ba483d4d00 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -1797,10 +1797,15 @@ domhostname
>   
>   .. code-block::
>   
> -   domhostname domain
> +   domhostname domain [--source lease|agent]
>   
>   Returns the hostname of a domain, if the hypervisor makes it available.
>   
> +The *--source* argument specifies what data source to use for the
> +hostnames, currently 'lease' to read DHCP leases or 'agent' to query
> +the guest OS via an agent. If unspecified, driver returns the default
> +method available (some drivers support only one type of source).
> +
>   
>   domid
>   -----
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index e60003978a..666c1875cc 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -4833,6 +4833,11 @@ typedef struct _virTypedParameter virMemoryParameter;
>    */
>   typedef virMemoryParameter *virMemoryParameterPtr;
>   
> +typedef enum {
> +    VIR_DOMAIN_GET_HOSTNAME_LEASE = (1 << 0), /* Parse DHCP lease file */
> +    VIR_DOMAIN_GET_HOSTNAME_AGENT = (1 << 1), /* Query qemu guest agent */
> +} virDomainGetHostnameFlags;
> +

This can go closer to the function declaration.

>   typedef enum {
>       VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LEASE = 0, /* Parse DHCP lease file */
>       VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_AGENT = 1, /* Query qemu guest agent */
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 780c6ed4a2..2dac730e70 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -5291,6 +5291,83 @@ lxcDomainGetCPUStats(virDomainPtr dom,
>   }
>   
>   
> +static char *
> +lxcDomainGetHostname(virDomainPtr dom,
> +                     unsigned int flags)
> +{
> +    virLXCDriverPtr driver = dom->conn->privateData;
> +    virDomainObjPtr vm = NULL;
> +    char macaddr[VIR_MAC_STRING_BUFLEN];
> +    g_autoptr(virConnect) conn = NULL;
> +    virNetworkDHCPLeasePtr *leases = NULL;
> +    int n_leases;
> +    size_t i, j;
> +    char *hostname = NULL;
> +
> +    virCheckFlags(VIR_DOMAIN_GET_HOSTNAME_LEASE, NULL);
> +
> +    if (!(vm = lxcDomObjFromDomain(dom)))
> +        return NULL;
> +
> +    if (virDomainGetHostnameEnsureACL(dom->conn, vm->def) < 0)
> +        goto cleanup;
> +
> +    if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_QUERY) < 0)
> +        goto cleanup;
> +
> +    if (virDomainObjCheckActive(vm) < 0)
> +        goto endjob;
> +
> +    if (!(conn = virGetConnectNetwork()))
> +        goto endjob;
> +
> +    for (i = 0; i < vm->def->nnets; i++) {
> +        g_autoptr(virNetwork) network = NULL;
> +
> +        if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
> +            continue;
> +
> +        virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr);
> +        network = virNetworkLookupByName(conn,
> +                                         vm->def->nets[i]->data.network.name);
> +
> +        if (!network)
> +            goto endjob;
> +
> +        if ((n_leases = virNetworkGetDHCPLeases(network, macaddr,
> +                                                &leases, 0)) < 0)
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("There is no available hostname %d"),
> +                           flags);
> +
> +        for (j = 0; j < n_leases; j++) {
> +            virNetworkDHCPLeasePtr lease = leases[j];
> +            if (lease->hostname) {
> +                hostname = g_strdup(lease->hostname);
> +
> +                for (j = 0; j < n_leases; j++)
> +                    virNetworkDHCPLeaseFree(leases[j]);
> +
> +                VIR_FREE(leases);
> +
> +                goto endjob;

Huh, you fixed qemu version of this code but not LXC one.

> +            }
> +
> +            virNetworkDHCPLeaseFree(lease);
> +        }
> +
> +        VIR_FREE(leases);
> +    }
> +
> + endjob:
> +    virLXCDomainObjEndJob(driver, vm);
> +
> + cleanup:
> +    virDomainObjEndAPI(&vm);
> +    return hostname;

There's another problem I realized here. We document that upon error 
NULL is returned. But if the domain is still booting up an hasn't done 
any DHPC, then there is no lease record just yet and NULL is returned 
without any error message. But in order to produce something sensible we 
will need new error code too.

> +}
> +
> +
>   static int
>   lxcNodeGetFreePages(virConnectPtr conn,
>                       unsigned int npages,
> @@ -5436,6 +5513,7 @@ static virHypervisorDriver lxcHypervisorDriver = {
>       .domainSetMetadata = lxcDomainSetMetadata, /* 1.1.3 */
>       .domainGetMetadata = lxcDomainGetMetadata, /* 1.1.3 */
>       .domainGetCPUStats = lxcDomainGetCPUStats, /* 1.2.2 */
> +    .domainGetHostname = lxcDomainGetHostname, /* 6.0.0 */
>       .nodeGetMemoryParameters = lxcNodeGetMemoryParameters, /* 0.10.2 */
>       .nodeSetMemoryParameters = lxcNodeSetMemoryParameters, /* 0.10.2 */
>       .domainSendProcessSignal = lxcDomainSendProcessSignal, /* 1.0.1 */
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index ec8faf384c..d8ed859f70 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -20280,9 +20280,19 @@ qemuDomainGetHostname(virDomainPtr dom,
>       virQEMUDriverPtr driver = dom->conn->privateData;
>       virDomainObjPtr vm = NULL;
>       qemuAgentPtr agent;
> +    char macaddr[VIR_MAC_STRING_BUFLEN];
> +    g_autoptr(virConnect) conn = NULL;
> +    virNetworkDHCPLeasePtr *leases = NULL;
> +    int n_leases;
> +    size_t i, j;
>       char *hostname = NULL;
>   
> -    virCheckFlags(0, NULL);
> +    virCheckFlags(VIR_DOMAIN_GET_HOSTNAME_LEASE |
> +                  VIR_DOMAIN_GET_HOSTNAME_AGENT, NULL);
> +
> +    VIR_EXCLUSIVE_FLAGS_RET(VIR_DOMAIN_GET_HOSTNAME_LEASE,
> +                            VIR_DOMAIN_GET_HOSTNAME_AGENT,
> +                            NULL);
>   
>       if (!(vm = qemuDomainObjFromDomain(dom)))
>           return NULL;
> @@ -20290,18 +20300,66 @@ qemuDomainGetHostname(virDomainPtr dom,
>       if (virDomainGetHostnameEnsureACL(dom->conn, vm->def) < 0)
>           goto cleanup;
>   
> -    if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
> -        goto cleanup;
> -
>       if (virDomainObjCheckActive(vm) < 0)
>           goto endjob;

Which job you want to end here? None was started.

>   
> -    if (!qemuDomainAgentAvailable(vm, true))
> -        goto endjob;
> +    switch (flags) {
> +    default:
> +    case VIR_DOMAIN_GET_HOSTNAME_AGENT:
> +        if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
> +            goto cleanup;
>   
> -    agent = qemuDomainObjEnterAgent(vm);
> -    ignore_value(qemuAgentGetHostname(agent, &hostname));
> -    qemuDomainObjExitAgent(vm, agent);
> +        if (!qemuDomainAgentAvailable(vm, true))
> +            goto endjob;
> +
> +        agent = qemuDomainObjEnterAgent(vm);
> +        ignore_value(qemuAgentGetHostname(agent, &hostname));
> +        qemuDomainObjExitAgent(vm, agent);
> +
> +        break;
> +    case VIR_DOMAIN_GET_HOSTNAME_LEASE:
> +        if (!(conn = virGetConnectNetwork()))
> +            goto endjob;
> +
> +        for (i = 0; i < vm->def->nnets; i++) {
> +            g_autoptr(virNetwork) network = NULL;
> +
> +            if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK)
> +                continue;
> +
> +            virMacAddrFormat(&(vm->def->nets[i]->mac), macaddr);
> +            network = virNetworkLookupByName(conn,
> +                                             vm->def->nets[i]->data.network.name);
> +
> +            if (!network)
> +                goto endjob;
> +
> +            if ((n_leases = virNetworkGetDHCPLeases(network, macaddr,
> +                                                    &leases, 0)) < 0)
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                               _("There is no available hostname %d"),
> +                               flags);
> +
> +            for (j = 0; j < n_leases; j++) {
> +                virNetworkDHCPLeasePtr lease = leases[j];
> +                if (lease->hostname) {
> +                    hostname = g_strdup(lease->hostname);
> +
> +                    for (j = 0; j < n_leases; j++)
> +                        virNetworkDHCPLeaseFree(leases[j]);
> +
> +                    VIR_FREE(leases);
> +
> +                    goto endjob;
> +                }
> +
> +                virNetworkDHCPLeaseFree(lease);
> +            }
> +
> +            VIR_FREE(leases);
> +        }
> +        break;
> +    }
>   
>    endjob:
>       qemuDomainObjEndAgentJob(vm);
> diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
> index 0311ee50d0..67e67d485a 100644
> --- a/tools/virsh-completer-domain.c
> +++ b/tools/virsh-completer-domain.c
> @@ -296,3 +296,20 @@ virshDomainShutdownModeCompleter(vshControl *ctl,
>   
>       return virshCommaStringListComplete(mode, modes);
>   }
> +
> +
> +char **
> +virshDomainGetHosnameSourcesCompleter(vshControl *ctl,
> +                                      const vshCmd *cmd,
> +                                      unsigned int flags)
> +{
> +    const char *sources[] = {"lease", "agent", NULL};
> +    const char *source = NULL;
> +
> +    virCheckFlags(0, NULL);
> +
> +    if (vshCommandOptStringQuiet(ctl, cmd, "source", &source) < 0)
> +        return NULL;
> +
> +    return virshCommaStringListComplete(source, sources);

This will need some fixing too.

I will post v4 soo since I have all the fixes in a local branch.

Michal




More information about the libvir-list mailing list