[libvirt] [PATCH v3 0/2] Add .domainGetHostname() support for QEMU driver.

This serie adds a new function into QEMU Guest Agent handlers to use the QEMU command 'guest-get-host-name' to retrieve the domain hostname. This approach requires QEMU-GA running inside the guest, but it is the fastest and easiest way to get this info. Julio Faracco (2): qemu: implementing qemuAgentGetHostname() function. qemu: adding domainGetHostname support for QEMU. src/qemu/qemu_agent.c | 39 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 4 ++++ src/qemu/qemu_driver.c | 40 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+) -- 2.17.1

This commit implements the function qemuAgentGetHostname() that uses the QEMU command 'guest-get-host-name' to retrieve the guest hostname of Virtual Machine. It is a possibility where QEMU-GA is running. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_agent.c | 39 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 4 ++++ 2 files changed, 43 insertions(+) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index bf08871f18..7aba4ed366 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1682,6 +1682,45 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus, return 0; } +int +qemuAgentGetHostname(qemuAgentPtr mon, + char **hostname) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data = NULL; + + cmd = qemuAgentMakeCommand("guest-get-host-name", + NULL); + + if (!cmd) + return ret; + + if (qemuAgentCommand(mon, cmd, &reply, true, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + goto cleanup; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed return value")); + goto cleanup; + } + + if (VIR_STRDUP(*hostname, + virJSONValueObjectGetString(data, "host-name")) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'host-name' missing in reply of guest-get-host-name")); + goto cleanup; + } + + ret = 0; + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} int qemuAgentGetTime(qemuAgentPtr mon, diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 6dd9c702dd..4354b7e0cf 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -105,6 +105,10 @@ int qemuAgentUpdateCPUInfo(unsigned int nvcpus, qemuAgentCPUInfoPtr cpuinfo, int ncpuinfo); +int +qemuAgentGetHostname(qemuAgentPtr mon, + char **hostname); + int qemuAgentGetTime(qemuAgentPtr mon, long long *seconds, unsigned int *nseconds); -- 2.17.1

On 08/21/2018 10:39 PM, Julio Faracco wrote:
This commit implements the function qemuAgentGetHostname() that uses the QEMU command 'guest-get-host-name' to retrieve the guest hostname
QEMU guest agent
of Virtual Machine. It is a possibility where QEMU-GA is running.
of the virtual machine running the QEMU-GA.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_agent.c | 39 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 4 ++++ 2 files changed, 43 insertions(+)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index bf08871f18..7aba4ed366 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1682,6 +1682,45 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus, return 0; }
+int +qemuAgentGetHostname(qemuAgentPtr mon, + char **hostname) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data = NULL; + + cmd = qemuAgentMakeCommand("guest-get-host-name", + NULL); + + if (!cmd) + return ret; + + if (qemuAgentCommand(mon, cmd, &reply, true, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + goto cleanup;
FWIW: The v2 dropped the qemuAgentCheckError... But I note two others exist - perhaps worthy of a followup patch or two?
+ + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed return value")); + goto cleanup; + } + + if (VIR_STRDUP(*hostname, + virJSONValueObjectGetString(data, "host-name")) <= 0) {
In this case the error would be overwritten in the case were VIR_STRDUP returns -1. Let's separate them and fetch into a local const char * - there's more examples that take that option... and yes, I think qemuAgentGetFSInfo is wrong for the same reason (yet another possible followup patch)... IOW: if (!(field = virJSONValueObjectGetString(data, "host-name"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'host-name' missing in guest-get-host-name reply")); goto cleanup; } if (VIR_STRDUP(*hostname, field) < 0) goto cleanup; As long as you're OK with it, I can merge something like this in and push after the 4.7.0 freeze into 4.8.0. Reviewed-by: John Ferlan <jferlan@redhat.com> John [...]

This commit add the support to use the function qemuAgentGetHostname() for obtain the domain hostname using QEMU-GA command. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_driver.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 21e9e87ddd..5f53cbea15 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19351,6 +19351,45 @@ qemuConnectGetCPUModelNames(virConnectPtr conn, return virCPUGetModels(arch, models); } +static char * +qemuDomainGetHostname(virDomainPtr dom, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + qemuAgentPtr agent; + char *hostname = NULL; + + virCheckFlags(0, NULL); + + if (!(vm = qemuDomObjFromDomain(dom))) + return NULL; + + 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; + + if (!qemuDomainAgentAvailable(vm, true)) + goto endjob; + + agent = qemuDomainObjEnterAgent(vm); + ignore_value(qemuAgentGetHostname(agent, &hostname)); + qemuDomainObjExitAgent(vm, agent); + + endjob: + qemuDomainObjEndAgentJob(vm); + + cleanup: + virDomainObjEndAPI(&vm); + return hostname; +} + + static int qemuDomainGetTime(virDomainPtr dom, long long *seconds, @@ -21955,6 +21994,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */ .domainFSFreeze = qemuDomainFSFreeze, /* 1.2.5 */ .domainFSThaw = qemuDomainFSThaw, /* 1.2.5 */ + .domainGetHostname = qemuDomainGetHostname, /* 4.7.0 */ .domainGetTime = qemuDomainGetTime, /* 1.2.5 */ .domainSetTime = qemuDomainSetTime, /* 1.2.5 */ .nodeGetFreePages = qemuNodeGetFreePages, /* 1.2.6 */ -- 2.17.1

On 08/21/2018 10:39 PM, Julio Faracco wrote:
This commit add the support to use the function qemuAgentGetHostname()
s/add the/adds/
for obtain the domain hostname using QEMU-GA command.
s/for/to/
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_driver.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 21e9e87ddd..5f53cbea15 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19351,6 +19351,45 @@ qemuConnectGetCPUModelNames(virConnectPtr conn, return virCPUGetModels(arch, models); }
Nit - we prefer 2 blank lines between functions for new code.
+static char * +qemuDomainGetHostname(virDomainPtr dom, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + qemuAgentPtr agent; + char *hostname = NULL; + + virCheckFlags(0, NULL); + + if (!(vm = qemuDomObjFromDomain(dom))) + return NULL; + + 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; + + if (!qemuDomainAgentAvailable(vm, true)) + goto endjob; + + agent = qemuDomainObjEnterAgent(vm); + ignore_value(qemuAgentGetHostname(agent, &hostname)); + qemuDomainObjExitAgent(vm, agent); + + endjob: + qemuDomainObjEndAgentJob(vm); + + cleanup: + virDomainObjEndAPI(&vm); + return hostname; +} + +
Ironically the 2 blanks lines were done here ;-)
static int qemuDomainGetTime(virDomainPtr dom, long long *seconds, @@ -21955,6 +21994,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */ .domainFSFreeze = qemuDomainFSFreeze, /* 1.2.5 */ .domainFSThaw = qemuDomainFSThaw, /* 1.2.5 */ + .domainGetHostname = qemuDomainGetHostname, /* 4.7.0 */
This'll be 4.8.0. Again, simple enough for me to adjust before pushing once 4.8.0 opens. You'll still need to post the news.xml changes once it opens and maybe you can work up patches to work through those things I pointed out in the review of patch 1. Reviewed-by: John Ferlan <jferlan@redhat.com> John
.domainGetTime = qemuDomainGetTime, /* 1.2.5 */ .domainSetTime = qemuDomainSetTime, /* 1.2.5 */ .nodeGetFreePages = qemuNodeGetFreePages, /* 1.2.6 */

Thanks for your review @John. I will wait for 4.8.0 to push all changes in one shot (including news.xml). Julio Cesar Faracco Em qua, 29 de ago de 2018 às 17:15, John Ferlan <jferlan@redhat.com> escreveu:
On 08/21/2018 10:39 PM, Julio Faracco wrote:
This commit add the support to use the function qemuAgentGetHostname()
s/add the/adds/
for obtain the domain hostname using QEMU-GA command.
s/for/to/
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_driver.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 21e9e87ddd..5f53cbea15 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19351,6 +19351,45 @@ qemuConnectGetCPUModelNames(virConnectPtr conn, return virCPUGetModels(arch, models); }
Nit - we prefer 2 blank lines between functions for new code.
+static char * +qemuDomainGetHostname(virDomainPtr dom, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + qemuAgentPtr agent; + char *hostname = NULL; + + virCheckFlags(0, NULL); + + if (!(vm = qemuDomObjFromDomain(dom))) + return NULL; + + 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; + + if (!qemuDomainAgentAvailable(vm, true)) + goto endjob; + + agent = qemuDomainObjEnterAgent(vm); + ignore_value(qemuAgentGetHostname(agent, &hostname)); + qemuDomainObjExitAgent(vm, agent); + + endjob: + qemuDomainObjEndAgentJob(vm); + + cleanup: + virDomainObjEndAPI(&vm); + return hostname; +} + +
Ironically the 2 blanks lines were done here ;-)
static int qemuDomainGetTime(virDomainPtr dom, long long *seconds, @@ -21955,6 +21994,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */ .domainFSFreeze = qemuDomainFSFreeze, /* 1.2.5 */ .domainFSThaw = qemuDomainFSThaw, /* 1.2.5 */ + .domainGetHostname = qemuDomainGetHostname, /* 4.7.0 */
This'll be 4.8.0.
Again, simple enough for me to adjust before pushing once 4.8.0 opens.
You'll still need to post the news.xml changes once it opens and maybe you can work up patches to work through those things I pointed out in the review of patch 1.
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
.domainGetTime = qemuDomainGetTime, /* 1.2.5 */ .domainSetTime = qemuDomainSetTime, /* 1.2.5 */ .nodeGetFreePages = qemuNodeGetFreePages, /* 1.2.6 */
participants (2)
-
John Ferlan
-
Julio Faracco