[libvirt] [PATCH v4 0/5] Introduce flags to virDomainGetHostname()

v4 of: https://www.redhat.com/archives/libvir-list/2019-December/msg01453.html diff to v3: - I've split Julio's one patch into 4 smaller ones, - I've fixed issues I've raised in v3, like new error code (patch 1/5 is completely new that's why I'm authoring it), fixed the completer and some memleaks. Julio Faracco (4): Introduce source flags to virDomainGetHostname() qemu: Implement virDomainGetHostnameFlags lxc: Implement virDomainGetHostnameFlags virsh: Expose virDomainGetHostnameFlags Michal Prívozník (1): virerror: Make it easier to add new error number docs/manpages/virsh.rst | 7 +- include/libvirt/libvirt-domain.h | 6 ++ include/libvirt/virterror.h | 1 + scripts/apibuild.py | 6 ++ src/libvirt-domain.c | 9 ++- src/lxc/lxc_driver.c | 79 +++++++++++++++++++ src/qemu/qemu_driver.c | 125 +++++++++++++++++++++++++++---- src/remote/remote_daemon.c | 1 + src/util/virerror.c | 7 +- tools/virsh-completer-domain.c | 19 +++++ tools/virsh-completer-domain.h | 4 + tools/virsh-domain.c | 37 ++++++++- tools/virsh-domain.h | 8 ++ 13 files changed, 286 insertions(+), 23 deletions(-) -- 2.24.1

In v5.0.0-rc1~94 we switched from one huge switch() to an array for translating error numbers into error messages. However, the array is declared to have VIR_ERR_NUMBER_LAST items which makes it impossible to spot this place by compile checking when adding new error number. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- scripts/apibuild.py | 6 ++++++ src/util/virerror.c | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/scripts/apibuild.py b/scripts/apibuild.py index 2f7314b379..595c004a4c 100755 --- a/scripts/apibuild.py +++ b/scripts/apibuild.py @@ -1657,6 +1657,12 @@ class CParser: token = ("name", "virloginit") return token + elif token[0] == "name" and token[1] == "G_STATIC_ASSERT": + # skip whole line + while token is not None and token[0] != "sep" or token[1] != ";": + token = self.token() + return self.token() + elif token[0] == "name": if self.type == "": self.type = token[1] diff --git a/src/util/virerror.c b/src/util/virerror.c index fd2f77329f..aac6ee3597 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -910,7 +910,7 @@ typedef struct { } virErrorMsgTuple; -const virErrorMsgTuple virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = { +static const virErrorMsgTuple virErrorMsgStrings[] = { [VIR_ERR_OK] = { NULL, NULL }, [VIR_ERR_INTERNAL_ERROR] = { N_("internal error"), @@ -1235,6 +1235,8 @@ const virErrorMsgTuple virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = { N_("network port not found: %s") }, }; +G_STATIC_ASSERT(G_N_ELEMENTS(virErrorMsgStrings) == VIR_ERR_NUMBER_LAST); + /** * virErrorMsg: -- 2.24.1

On Thu, Jan 09, 2020 at 01:45:56PM +0100, Michal Privoznik wrote:
In v5.0.0-rc1~94 we switched from one huge switch() to an array for translating error numbers into error messages. However, the array is declared to have VIR_ERR_NUMBER_LAST items which makes it impossible to spot this place by compile checking when adding new error number.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- scripts/apibuild.py | 6 ++++++ src/util/virerror.c | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/scripts/apibuild.py b/scripts/apibuild.py index 2f7314b379..595c004a4c 100755 --- a/scripts/apibuild.py +++ b/scripts/apibuild.py @@ -1657,6 +1657,12 @@ class CParser: token = ("name", "virloginit") return token
+ elif token[0] == "name" and token[1] == "G_STATIC_ASSERT": + # skip whole line + while token is not None and token[0] != "sep" or token[1] != ";":
This will trigger a TypeError exception if token is None because of operator precedence. diff --git a/scripts/apibuild.py b/scripts/apibuild.py index 595c004a4c..00dd510304 100755 --- a/scripts/apibuild.py +++ b/scripts/apibuild.py @@ -1659,7 +1659,8 @@ class CParser: elif token[0] == "name" and token[1] == "G_STATIC_ASSERT": # skip whole line - while token is not None and token[0] != "sep" or token[1] != ";": + while token is not None and (token[0] != "sep" or + token[1] != ";"): token = self.token() return self.token()
+ token = self.token() + return self.token() + elif token[0] == "name": if self.type == "": self.type = token[1] diff --git a/src/util/virerror.c b/src/util/virerror.c index fd2f77329f..aac6ee3597 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -910,7 +910,7 @@ typedef struct { } virErrorMsgTuple;
-const virErrorMsgTuple virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = { +static const virErrorMsgTuple virErrorMsgStrings[] = { [VIR_ERR_OK] = { NULL, NULL }, [VIR_ERR_INTERNAL_ERROR] = { N_("internal error"), @@ -1235,6 +1235,8 @@ const virErrorMsgTuple virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = { N_("network port not found: %s") }, };
+G_STATIC_ASSERT(G_N_ELEMENTS(virErrorMsgStrings) == VIR_ERR_NUMBER_LAST); +
Safe for freeze: Reviewed-by: Erik Skultety <eskultet@redhat.com>

From: Julio Faracco <jcfaracco@gmail.com> 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. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- include/libvirt/libvirt-domain.h | 6 ++++++ include/libvirt/virterror.h | 1 + src/libvirt-domain.c | 9 +++++---- src/remote/remote_daemon.c | 1 + src/util/virerror.c | 3 +++ 5 files changed, 16 insertions(+), 4 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index c1b9a9d1d0..44f6b62913 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1567,6 +1567,12 @@ int virDomainSetMemoryStatsPeriod (virDomainPtr domain, int virDomainGetMaxVcpus (virDomainPtr domain); int virDomainGetSecurityLabel (virDomainPtr domain, virSecurityLabelPtr seclabel); + +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; + char * virDomainGetHostname (virDomainPtr domain, unsigned int flags); int virDomainGetSecurityLabelList (virDomainPtr domain, diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 54f4f8190d..b7aa2a0ec3 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -332,6 +332,7 @@ typedef enum { VIR_ERR_INVALID_NETWORK_PORT = 105, /* invalid network port object */ VIR_ERR_NETWORK_PORT_EXIST = 106, /* the network port already exist */ VIR_ERR_NO_NETWORK_PORT = 107, /* network port not found */ + VIR_ERR_NO_HOSTNAME = 108, /* no domain's hostname found */ # ifdef VIR_ENUM_SENTINELS VIR_ERR_NUMBER_LAST diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index eb66999f07..d0304e174f 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11025,12 +11025,13 @@ virDomainGetDiskErrors(virDomainPtr dom, /** * virDomainGetHostname: * @domain: a domain object - * @flags: extra flags; not used yet, so callers should always pass 0 + * @flags: bitwise-OR of virDomainGetHostnameFlags * - * Get the hostname for that domain. + * Get the hostname for that domain. If no hostname is found, + * then an error is raised with VIR_ERR_NO_HOSTNAME code. * - * Dependent on hypervisor used, this may require a guest agent to be - * available. + * Dependent on hypervisor and @flags used, this may require a + * guest agent to be available. * * Returns the hostname which must be freed by the caller, or * NULL if there was an error. diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index cd55b2c39e..1c224f8050 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -296,6 +296,7 @@ static int daemonErrorLogFilter(virErrorPtr err, int priority) case VIR_ERR_NO_DOMAIN_METADATA: case VIR_ERR_NO_SERVER: case VIR_ERR_NO_CLIENT: + case VIR_ERR_NO_HOSTNAME: return VIR_LOG_DEBUG; } diff --git a/src/util/virerror.c b/src/util/virerror.c index aac6ee3597..0f3ee1faaa 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1233,6 +1233,9 @@ static const virErrorMsgTuple virErrorMsgStrings[] = { [VIR_ERR_NO_NETWORK_PORT] = { N_("network port not found"), N_("network port not found: %s") }, + [VIR_ERR_NO_HOSTNAME] = { + N_("no hostname found"), + N_("no hostname found: %s") }, }; G_STATIC_ASSERT(G_N_ELEMENTS(virErrorMsgStrings) == VIR_ERR_NUMBER_LAST); -- 2.24.1

On Thu, Jan 09, 2020 at 01:45:57PM +0100, Michal Privoznik wrote:
From: Julio Faracco <jcfaracco@gmail.com>
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.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Julio Faracco <jcfaracco@gmail.com> ---
Reviewed-by: Erik Skultety <eskultet@redhat.com>

From: Julio Faracco <jcfaracco@gmail.com> We have to keep the default - querying the agent if no flag is set. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_driver.c | 125 +++++++++++++++++++++++++++++++++++------ 1 file changed, 109 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1fc662b3c8..6b33342be8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20257,25 +20257,16 @@ qemuConnectGetCPUModelNames(virConnectPtr conn, } -static char * -qemuDomainGetHostname(virDomainPtr dom, - unsigned int flags) +static int +qemuDomainGetHostnameAgent(virQEMUDriverPtr driver, + virDomainObjPtr vm, + char **hostname) { - virQEMUDriverPtr driver = dom->conn->privateData; - virDomainObjPtr vm = NULL; qemuAgentPtr agent; - char *hostname = NULL; - - virCheckFlags(0, NULL); - - if (!(vm = qemuDomainObjFromDomain(dom))) - return NULL; - - if (virDomainGetHostnameEnsureACL(dom->conn, vm->def) < 0) - goto cleanup; + int ret = -1; if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0) - goto cleanup; + return -1; if (virDomainObjCheckActive(vm) < 0) goto endjob; @@ -20284,11 +20275,113 @@ qemuDomainGetHostname(virDomainPtr dom, goto endjob; agent = qemuDomainObjEnterAgent(vm); - ignore_value(qemuAgentGetHostname(agent, &hostname)); + ignore_value(qemuAgentGetHostname(agent, hostname)); qemuDomainObjExitAgent(vm, agent); + ret = 0; endjob: qemuDomainObjEndAgentJob(vm); + return ret; +} + + +static int +qemuDomainGetHostnameLease(virQEMUDriverPtr driver, + virDomainObjPtr vm, + char **hostname) +{ + char macaddr[VIR_MAC_STRING_BUFLEN]; + g_autoptr(virConnect) conn = NULL; + virNetworkDHCPLeasePtr *leases = NULL; + int n_leases; + size_t i, j; + int ret = -1; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + return -1; + + if (virDomainObjCheckActive(vm) < 0) + goto endjob; + + if (!(conn = virGetConnectNetwork())) + goto endjob; + + for (i = 0; i < vm->def->nnets; i++) { + g_autoptr(virNetwork) network = NULL; + virDomainNetDefPtr net = vm->def->nets[i]; + + if (net->type != VIR_DOMAIN_NET_TYPE_NETWORK) + continue; + + virMacAddrFormat(&net->mac, macaddr); + network = virNetworkLookupByName(conn, net->data.network.name); + + if (!network) + goto endjob; + + if ((n_leases = virNetworkGetDHCPLeases(network, macaddr, + &leases, 0)) < 0) + goto endjob; + + for (j = 0; j < n_leases; j++) { + virNetworkDHCPLeasePtr lease = leases[j]; + if (lease->hostname && !*hostname) + *hostname = g_strdup(lease->hostname); + + virNetworkDHCPLeaseFree(lease); + } + + VIR_FREE(leases); + + if (*hostname) + goto endjob; + } + + ret = 0; + endjob: + qemuDomainObjEndJob(driver, vm); + return ret; +} + + +static char * +qemuDomainGetHostname(virDomainPtr dom, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + char *hostname = 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 (!(flags & VIR_DOMAIN_GET_HOSTNAME_LEASE)) + flags |= VIR_DOMAIN_GET_HOSTNAME_AGENT; + + if (!(vm = qemuDomainObjFromDomain(dom))) + return NULL; + + if (virDomainGetHostnameEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_GET_HOSTNAME_AGENT) { + if (qemuDomainGetHostnameAgent(driver, vm, &hostname) < 0) + goto cleanup; + } else if (flags & VIR_DOMAIN_GET_HOSTNAME_LEASE) { + if (qemuDomainGetHostnameLease(driver, vm, &hostname) < 0) + goto cleanup; + } + + if (!hostname) { + virReportError(VIR_ERR_NO_HOSTNAME, + _("no hostname found for domain %s"), + vm->def->name); + goto cleanup; + } cleanup: virDomainObjEndAPI(&vm); -- 2.24.1

On Thu, Jan 09, 2020 at 01:45:58PM +0100, Michal Privoznik wrote:
From: Julio Faracco <jcfaracco@gmail.com>
We have to keep the default - querying the agent if no flag is set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- ...
+ +static int +qemuDomainGetHostnameLease(virQEMUDriverPtr driver, + virDomainObjPtr vm, + char **hostname) ...
+ + if ((n_leases = virNetworkGetDHCPLeases(network, macaddr, + &leases, 0)) < 0) + goto endjob; + + for (j = 0; j < n_leases; j++) { + virNetworkDHCPLeasePtr lease = leases[j]; + if (lease->hostname && !*hostname) + *hostname = g_strdup(lease->hostname);
Not a big deal, but why not doing a break once you extract the first hostname? Reviewed-by: Erik Skultety <eskultet@redhat.com>

On 1/10/20 2:05 PM, Erik Skultety wrote:
On Thu, Jan 09, 2020 at 01:45:58PM +0100, Michal Privoznik wrote:
From: Julio Faracco <jcfaracco@gmail.com>
We have to keep the default - querying the agent if no flag is set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- ...
+ +static int +qemuDomainGetHostnameLease(virQEMUDriverPtr driver, + virDomainObjPtr vm, + char **hostname) ...
+ + if ((n_leases = virNetworkGetDHCPLeases(network, macaddr, + &leases, 0)) < 0) + goto endjob; + + for (j = 0; j < n_leases; j++) { + virNetworkDHCPLeasePtr lease = leases[j]; + if (lease->hostname && !*hostname) + *hostname = g_strdup(lease->hostname);
Not a big deal, but why not doing a break once you extract the first hostname?
We need to free all leases to avoid memleak. Instead of doing that in two separate loops (one where we'd look for the hostname, the other just for freeing), I find it simpler if done in a single loop.
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Michal

On Fri, Jan 10, 2020 at 03:19:16PM +0100, Michal Privoznik wrote:
On 1/10/20 2:05 PM, Erik Skultety wrote:
On Thu, Jan 09, 2020 at 01:45:58PM +0100, Michal Privoznik wrote:
From: Julio Faracco <jcfaracco@gmail.com>
We have to keep the default - querying the agent if no flag is set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- ...
+ +static int +qemuDomainGetHostnameLease(virQEMUDriverPtr driver, + virDomainObjPtr vm, + char **hostname) ...
+ + if ((n_leases = virNetworkGetDHCPLeases(network, macaddr, + &leases, 0)) < 0) + goto endjob; + + for (j = 0; j < n_leases; j++) { + virNetworkDHCPLeasePtr lease = leases[j]; + if (lease->hostname && !*hostname) + *hostname = g_strdup(lease->hostname);
Not a big deal, but why not doing a break once you extract the first hostname?
We need to free all leases to avoid memleak. Instead of doing that in two separate loops (one where we'd look for the hostname, the other just for freeing), I find it simpler if done in a single loop.
Aaah, shoot, you're right, please ignore my comment then. Erik

From: Julio Faracco <jcfaracco@gmail.com> Since there is no guest agent in LXC world (yet), we can implement _LEASE flag only. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_driver.c | 79 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 780c6ed4a2..bf1f8f8190 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5291,6 +5291,84 @@ 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; + virDomainNetDefPtr net = vm->def->nets[i]; + + if (net->type != VIR_DOMAIN_NET_TYPE_NETWORK) + continue; + + virMacAddrFormat(&net->mac, macaddr); + network = virNetworkLookupByName(conn, net->data.network.name); + + if (!network) + goto endjob; + + if ((n_leases = virNetworkGetDHCPLeases(network, macaddr, + &leases, 0)) < 0) + goto endjob; + + for (j = 0; j < n_leases; j++) { + virNetworkDHCPLeasePtr lease = leases[j]; + + if (lease->hostname && !hostname) + hostname = g_strdup(lease->hostname); + + virNetworkDHCPLeaseFree(lease); + } + + VIR_FREE(leases); + + if (hostname) + goto endjob; + } + + if (!hostname) { + virReportError(VIR_ERR_NO_HOSTNAME, + _("no hostname found for domain %s"), + vm->def->name); + goto endjob; + } + + endjob: + virLXCDomainObjEndJob(driver, vm); + + cleanup: + virDomainObjEndAPI(&vm); + return hostname; +} + + static int lxcNodeGetFreePages(virConnectPtr conn, unsigned int npages, @@ -5436,6 +5514,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 */ -- 2.24.1

On Thu, Jan 09, 2020 at 01:45:59PM +0100, Michal Privoznik wrote:
From: Julio Faracco <jcfaracco@gmail.com>
Since there is no guest agent in LXC world (yet), we can implement _LEASE flag only.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
+ + for (j = 0; j < n_leases; j++) { + virNetworkDHCPLeasePtr lease = leases[j]; + + if (lease->hostname && !hostname) + hostname = g_strdup(lease->hostname);
Same comment with using a break. Reviewed-by: Erik Skultety <eskultet@redhat.com>

From: Julio Faracco <jcfaracco@gmail.com> Our virsh already has 'domhostname' command. Add '--source' argument to it so that users can chose between 'lease' and 'agent' sources. Also, implement completer for the argument. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/manpages/virsh.rst | 7 ++++++- tools/virsh-completer-domain.c | 19 +++++++++++++++++ tools/virsh-completer-domain.h | 4 ++++ tools/virsh-domain.c | 37 +++++++++++++++++++++++++++++++++- tools/virsh-domain.h | 8 ++++++++ 5 files changed, 73 insertions(+), 2 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 4522259657..95a20aef9c 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/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c index 6da603048e..4472ee08f2 100644 --- a/tools/virsh-completer-domain.c +++ b/tools/virsh-completer-domain.c @@ -316,3 +316,22 @@ virshDomainInterfaceAddrSourceCompleter(vshControl *ctl G_GNUC_UNUSED, return ret; } + + +char ** +virshDomainHostnameSourceCompleter(vshControl *ctl G_GNUC_UNUSED, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int flags) +{ + char **ret = NULL; + size_t i; + + virCheckFlags(0, NULL); + + ret = g_new0(typeof(*ret), VIRSH_DOMAIN_HOSTNAME_SOURCE_LAST + 1); + + for (i = 0; i < VIRSH_DOMAIN_HOSTNAME_SOURCE_LAST; i++) + ret[i] = g_strdup(virshDomainHostnameSourceTypeToString(i)); + + return ret; +} diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h index 79beec2cfe..b00b05e3bd 100644 --- a/tools/virsh-completer-domain.h +++ b/tools/virsh-completer-domain.h @@ -58,3 +58,7 @@ char ** virshDomainInterfaceAddrSourceCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); + +char ** virshDomainHostnameSourceCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 9315755990..0b6a9f2fbd 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11741,20 +11741,55 @@ static const vshCmdInfo info_domhostname[] = { static const vshCmdOptDef opts_domhostname[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), + {.name = "source", + .type = VSH_OT_STRING, + .flags = VSH_OFLAG_NONE, + .completer = virshDomainHostnameSourceCompleter, + .help = N_("address source: 'lease' or 'agent'")}, {.name = NULL} }; +VIR_ENUM_IMPL(virshDomainHostnameSource, + VIRSH_DOMAIN_HOSTNAME_SOURCE_LAST, + "agent", + "lease"); + static bool cmdDomHostname(vshControl *ctl, const vshCmd *cmd) { char *hostname; virDomainPtr dom; bool ret = false; + const char *sourcestr = NULL; + int flags = 0; /* Use default value. Drivers can have its own default. */ if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - hostname = virDomainGetHostname(dom, 0); + if (vshCommandOptStringReq(ctl, cmd, "source", &sourcestr) < 0) + goto error; + + if (sourcestr) { + int source = virshDomainHostnameSourceTypeFromString(sourcestr); + + if (source < 0) { + vshError(ctl, _("Unknown data source '%s'"), sourcestr); + goto error; + } + + switch ((virshDomainHostnameSource) source) { + case VIRSH_DOMAIN_HOSTNAME_SOURCE_AGENT: + flags |= VIR_DOMAIN_GET_HOSTNAME_AGENT; + break; + case VIRSH_DOMAIN_HOSTNAME_SOURCE_LEASE: + flags |= VIR_DOMAIN_GET_HOSTNAME_LEASE; + break; + case VIRSH_DOMAIN_HOSTNAME_SOURCE_LAST: + break; + } + } + + hostname = virDomainGetHostname(dom, flags); if (hostname == NULL) { vshError(ctl, "%s", _("failed to get hostname")); goto error; diff --git a/tools/virsh-domain.h b/tools/virsh-domain.h index 02996d51b1..0d59c579d4 100644 --- a/tools/virsh-domain.h +++ b/tools/virsh-domain.h @@ -30,4 +30,12 @@ typedef struct virshDomainEventCallback virshDomainEventCallback; extern virshDomainEventCallback virshDomainEventCallbacks[]; +typedef enum { + VIRSH_DOMAIN_HOSTNAME_SOURCE_AGENT, + VIRSH_DOMAIN_HOSTNAME_SOURCE_LEASE, + VIRSH_DOMAIN_HOSTNAME_SOURCE_LAST +} virshDomainHostnameSource; + +VIR_ENUM_DECL(virshDomainHostnameSource); + extern const vshCmdDef domManagementCmds[]; -- 2.24.1

On Thu, Jan 09, 2020 at 01:46:00PM +0100, Michal Privoznik wrote:
From: Julio Faracco <jcfaracco@gmail.com>
Our virsh already has 'domhostname' command. Add '--source' argument to it so that users can chose between 'lease' and 'agent' sources. Also, implement completer for the argument.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>

On Thu, Jan 09, 2020 at 01:45:55PM +0100, Michal Privoznik wrote:
v4 of:
https://www.redhat.com/archives/libvir-list/2019-December/msg01453.html
diff to v3: - I've split Julio's one patch into 4 smaller ones, - I've fixed issues I've raised in v3, like new error code (patch 1/5 is completely new that's why I'm authoring it), fixed the completer and some memleaks.
I'm tempted to say Safe for freeze for patches 2-5, but since you're adding an enhancement, I'm afraid this will have to wait until 6.1.0 even though it's a straightforward change, sorry for a late review :(. Erik
participants (2)
-
Erik Skultety
-
Michal Privoznik