[libvirt] [PATCH v5 0/4] Add .domainGetHostname() support for QEMU driver.
This serie adds a new function into QEMU Guest Agent handler 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. This serie has some suggestion made by John Ferlan for v3. Specially, some improvements to error handling. v5: Contains news suggested by Han Han on v2. Contains improvements suggested by John on v3. Removes some tabs reported on v4. Julio Faracco (4): qemu: implementing qemuAgentGetHostname() function. qemu: adding domainGetHostname support for QEMU docs: Add QEMU-GA get hostname feature into news.xml qemu: unlink the error report from VIR_STRDUP. docs/news.xml | 10 +++++++ src/qemu/qemu_agent.c | 66 ++++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_agent.h | 4 +++ src/qemu/qemu_driver.c | 42 +++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 6 deletions(-) -- 2.17.1
This commit implements the function qemuAgentGetHostname() that uses the QEMU guest agent command 'guest-get-host-name' to retrieve the guest hostname of virtual machine running the QEMU-GA. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_agent.c | 47 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 4 ++++ 2 files changed, 51 insertions(+) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index bf08871f18..ac728becef 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1683,6 +1683,53 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus, } +int +qemuAgentGetHostname(qemuAgentPtr mon, + char **hostname) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data = NULL; + const char *result = 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 (qemuAgentCheckError(cmd, reply) < 0) + goto cleanup; + + if (!(data = virJSONValueObjectGet(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed return value")); + goto cleanup; + } + + if (!(result = 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, result) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + int qemuAgentGetTime(qemuAgentPtr mon, long long *seconds, 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 09/05/2018 12:20 AM, Julio Faracco wrote:
This commit implements the function qemuAgentGetHostname() that uses the QEMU guest agent command 'guest-get-host-name' to retrieve the guest hostname of virtual machine running the QEMU-GA.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_agent.c | 47 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 4 ++++ 2 files changed, 51 insertions(+)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index bf08871f18..ac728becef 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1683,6 +1683,53 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus, }
+int +qemuAgentGetHostname(qemuAgentPtr mon, + char **hostname) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data = NULL; + const char *result = 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 (qemuAgentCheckError(cmd, reply) < 0) + goto cleanup;
Hmmm - I hope my comments weren't misconstrued. I think your v3 to drop the call is correct since qemuAgentCommand already does this. My point there was that there were a couple of other calls that added a call to qemuAgentCheckError after qemuAgentCommand, but that doesn't seem "right" based on mkletzan's commit 5b3492fadb. In any case, I'll remove it in my branch and wait for your 'OK' before pushing the series. Reviewed-by: John Ferlan <jferlan@redhat.com> John
I don't remember if I sent any patch to include the check. But I remember that I saw that @mkletzan put the check right after the command execution. So, when you run a command the function automatically check for errors reported. Nice! Well, @jferlan you have my Ok. ;-) -- Julio Cesar Faracco Em qua, 5 de set de 2018 às 14:08, John Ferlan <jferlan@redhat.com> escreveu:
On 09/05/2018 12:20 AM, Julio Faracco wrote:
This commit implements the function qemuAgentGetHostname() that uses the QEMU guest agent command 'guest-get-host-name' to retrieve the guest hostname of virtual machine running the QEMU-GA.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_agent.c | 47 +++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 4 ++++ 2 files changed, 51 insertions(+)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index bf08871f18..ac728becef 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1683,6 +1683,53 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus, }
+int +qemuAgentGetHostname(qemuAgentPtr mon, + char **hostname) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data = NULL; + const char *result = 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 (qemuAgentCheckError(cmd, reply) < 0) + goto cleanup;
Hmmm - I hope my comments weren't misconstrued. I think your v3 to drop the call is correct since qemuAgentCommand already does this.
My point there was that there were a couple of other calls that added a call to qemuAgentCheckError after qemuAgentCommand, but that doesn't seem "right" based on mkletzan's commit 5b3492fadb.
In any case, I'll remove it in my branch and wait for your 'OK' before pushing the series.
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
This commit adds 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 | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 07ea5473b6..2f8d6915e1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19353,6 +19353,46 @@ 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, @@ -19399,6 +19439,7 @@ qemuDomainGetTime(virDomainPtr dom, return ret; } + static int qemuDomainSetTime(virDomainPtr dom, long long seconds, @@ -21957,6 +21998,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */ .domainFSFreeze = qemuDomainFSFreeze, /* 1.2.5 */ .domainFSThaw = qemuDomainFSThaw, /* 1.2.5 */ + .domainGetHostname = qemuDomainGetHostname, /* 4.8.0 */ .domainGetTime = qemuDomainGetTime, /* 1.2.5 */ .domainSetTime = qemuDomainSetTime, /* 1.2.5 */ .nodeGetFreePages = qemuNodeGetFreePages, /* 1.2.6 */ -- 2.17.1
On 09/05/2018 12:20 AM, Julio Faracco wrote:
This commit adds support to use the function qemuAgentGetHostname() for obtain the domain hostname using QEMU-GA command.
I'll fix the "for" to be "to"...
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_driver.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
QEMU-GA supports get geust hostname command. This commit includes a specific entry to inform this new feature for QEMU driver to 4.8.0 release. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- docs/news.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 9a17b2f612..d67327699d 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -37,6 +37,16 @@ <section title="New features"> </section> <section title="Improvements"> + <change> + <summary> + qemu: Retrieve guest hostname through QEMU Guest Agent command + </summary> + <description> + QEMU is now able to retrieve the guest hostname using a new QEMU-GA + command called 'guest-get-host-name'. Virsh users can execute + 'domhostname' for QEMU driver. + </description> + </change> </section> <section title="Bug fixes"> </section> -- 2.17.1
On 09/05/2018 12:20 AM, Julio Faracco wrote:
QEMU-GA supports get geust hostname command. This commit includes a specific entry to inform this new feature for QEMU driver to 4.8.0 release.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- docs/news.xml | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 9a17b2f612..d67327699d 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -37,6 +37,16 @@ <section title="New features"> </section> <section title="Improvements"> + <change> + <summary> + qemu: Retrieve guest hostname through QEMU Guest Agent command + </summary> + <description> + QEMU is now able to retrieve the guest hostname using a new QEMU-GA + command called 'guest-get-host-name'. Virsh users can execute + 'domhostname' for QEMU driver.
s/./ for domains configured to use the Guest Agent.
+ </description> + </change>
I can change before pushing... Reviewed-by: John Ferlan <jferlan@redhat.com> John
</section> <section title="Bug fixes"> </section>
The function to retrieve the file system info using QEMU-GA is using some conditionals to retrieve the info. This is wrong because the error of some conditionals will be raised if VIR_STRDUP return errors and not if some problem occurred with JSON. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_agent.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index ac728becef..36dc18d72f 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1836,6 +1836,7 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info, virJSONValuePtr data; virDomainFSInfoPtr *info_ret = NULL; virPCIDeviceAddress pci_address; + const char *result = NULL; cmd = qemuAgentMakeCommand("guest-get-fsinfo", NULL); if (!cmd) @@ -1881,28 +1882,34 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info, if (VIR_ALLOC(info_ret[i]) < 0) goto cleanup; - if (VIR_STRDUP(info_ret[i]->mountpoint, - virJSONValueObjectGetString(entry, "mountpoint")) < 0) { + if (!(result = virJSONValueObjectGetString(entry, "mountpoint"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'mountpoint' missing in reply of " "guest-get-fsinfo")); goto cleanup; } - if (VIR_STRDUP(info_ret[i]->name, - virJSONValueObjectGetString(entry, "name")) < 0) { + if (VIR_STRDUP(info_ret[i]->mountpoint, result) < 0) + goto cleanup; + + if (!(result = virJSONValueObjectGetString(entry, "name"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'name' missing in reply of guest-get-fsinfo")); goto cleanup; } - if (VIR_STRDUP(info_ret[i]->fstype, - virJSONValueObjectGetString(entry, "type")) < 0) { + if (VIR_STRDUP(info_ret[i]->name, result) < 0) + goto cleanup; + + if (!(result = virJSONValueObjectGetString(entry, "type"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'type' missing in reply of guest-get-fsinfo")); goto cleanup; } + if (VIR_STRDUP(info_ret[i]->fstype, result) < 0) + goto cleanup; + if (!(entry = virJSONValueObjectGet(entry, "disk"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'disk' missing in reply of guest-get-fsinfo")); -- 2.17.1
On 09/05/2018 12:20 AM, Julio Faracco wrote:
The function to retrieve the file system info using QEMU-GA is using some conditionals to retrieve the info. This is wrong because the error of some conditionals will be raised if VIR_STRDUP return errors and not if some problem occurred with JSON.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_agent.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index ac728becef..36dc18d72f 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1836,6 +1836,7 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info, virJSONValuePtr data; virDomainFSInfoPtr *info_ret = NULL; virPCIDeviceAddress pci_address; + const char *result = NULL;
This can move inside the for loop... I can move before pushing... Reviewed-by: John Ferlan <jferlan@redhat.com> John
The '.' at the end of the summary is not necessary On Wed, Sep 05, 2018 at 01:20:56AM -0300, Julio Faracco wrote:
The function to retrieve the file system info using QEMU-GA is using some conditionals to retrieve the info. This is wrong because the error of some conditionals will be raised if VIR_STRDUP return errors and not if some problem occurred with JSON.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_agent.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
This patch is unrelated to the rest of the series and can be pushed separately. Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (3)
-
John Ferlan -
Julio Faracco -
Ján Tomko