Iimplements the new guest information API by querying requested
information via the guest agent.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
src/qemu/qemu_driver.c | 110 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 110 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1b051a9424..446266e66b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -23190,6 +23190,115 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
return ret;
}
+static unsigned int supportedGuestInfoTypes =
+ VIR_DOMAIN_GUEST_INFO_USERS |
+ VIR_DOMAIN_GUEST_INFO_OS |
+ VIR_DOMAIN_GUEST_INFO_TIMEZONE |
+ VIR_DOMAIN_GUEST_INFO_HOSTNAME |
+ VIR_DOMAIN_GUEST_INFO_FILESYSTEM;
+
+static void
+qemuDomainGetGuestInfoCheckSupport(unsigned int *types)
+{
+ if (*types == 0)
+ *types = supportedGuestInfoTypes;
+
+ *types = *types & supportedGuestInfoTypes;
+}
+
+static int
+qemuDomainGetGuestInfo(virDomainPtr dom,
+ unsigned int types,
+ virTypedParameterPtr *params,
+ int *nparams,
+ unsigned int flags)
+{
+ virQEMUDriverPtr driver = dom->conn->privateData;
+ virDomainObjPtr vm = NULL;
+ qemuAgentPtr agent;
+ int ret = -1;
+ int rv = -1;
+ int maxparams = 0;
+ char *hostname = NULL;
+ virDomainDefPtr def = NULL;
+ virCapsPtr caps = NULL;
+ unsigned int supportedTypes = types;
+
+ virCheckFlags(0, ret);
+ qemuDomainGetGuestInfoCheckSupport(&supportedTypes);
+
+ if (!(vm = qemuDomObjFromDomain(dom)))
+ goto cleanup;
+
+ if (virDomainGetGuestInfoEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
+ goto cleanup;
+
+ if (!qemuDomainAgentAvailable(vm, true))
+ goto endjob;
+
+ agent = qemuDomainObjEnterAgent(vm);
+
+ /* Although the libvirt qemu driver supports all of these guest info types,
+ * some guest agents might be too old to support these commands. If these
+ * info categories were explicitly requested (i.e. 'types' is non-zero),
+ * abort and report an error on any failures, otherwise continue and return
+ * as much info as is supported by the guest agent. */
+ if (supportedTypes & VIR_DOMAIN_GUEST_INFO_USERS) {
+ if (qemuAgentGetUsers(agent, params, nparams, &maxparams) < 0 &&
+ types != 0)
+ goto exitagent;
+ }
+ if (supportedTypes & VIR_DOMAIN_GUEST_INFO_OS) {
+ if (qemuAgentGetOSInfo(agent, params, nparams, &maxparams) < 0
+ && types != 0)
+ goto exitagent;
+ }
+ if (supportedTypes & VIR_DOMAIN_GUEST_INFO_TIMEZONE) {
+ if (qemuAgentGetTimezone(agent, params, nparams, &maxparams) < 0
+ && types != 0)
+ goto exitagent;
+ }
+ if (supportedTypes & VIR_DOMAIN_GUEST_INFO_HOSTNAME) {
+ if (qemuAgentGetHostname(agent, &hostname) < 0) {
+ if (types != 0)
+ goto exitagent;
+ } else {
+ if (virTypedParamsAddString(params, nparams, &maxparams, "hostname",
+ hostname) < 0)
+ goto exitagent;
+ }
+ }
+ if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) {
+ if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+ goto exitagent;
+
+ if (!(def = virDomainDefCopy(vm->def, caps, driver->xmlopt, NULL, false)))
+ goto exitagent;
+
+ if (qemuAgentGetFSInfoParams(agent, params, nparams, &maxparams, def) < 0 &&
+ types != 0)
+ goto exitagent;
+ }
+
+ rv = 0;
+
+ exitagent:
+ qemuDomainObjExitAgent(vm, agent);
+
+ endjob:
+ qemuDomainObjEndAgentJob(vm);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ virDomainDefFree(def);
+ virObjectUnref(caps);
+ VIR_FREE(hostname);
+ return rv;
+}
+
static virHypervisorDriver qemuHypervisorDriver = {
.name = QEMU_DRIVER_NAME,
.connectURIProbe = qemuConnectURIProbe,
@@ -23425,6 +23534,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
.domainCheckpointLookupByName = qemuDomainCheckpointLookupByName, /* 5.6.0 */
.domainCheckpointGetParent = qemuDomainCheckpointGetParent, /* 5.6.0 */
.domainCheckpointDelete = qemuDomainCheckpointDelete, /* 5.6.0 */
+ .domainGetGuestInfo = qemuDomainGetGuestInfo, /* 5.6.0 */
};
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 8/23/19 1:31 PM, Jonathon Jongsma wrote:
> Iimplements the new guest information API by querying requested
> information via the guest agent.
>
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> src/qemu/qemu_driver.c | 110 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 110 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 1b051a9424..446266e66b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -23190,6 +23190,115 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
> return ret;
> }
>
> +static unsigned int supportedGuestInfoTypes =
> + VIR_DOMAIN_GUEST_INFO_USERS |
> + VIR_DOMAIN_GUEST_INFO_OS |
> + VIR_DOMAIN_GUEST_INFO_TIMEZONE |
> + VIR_DOMAIN_GUEST_INFO_HOSTNAME |
> + VIR_DOMAIN_GUEST_INFO_FILESYSTEM;
> +
> +static void
> +qemuDomainGetGuestInfoCheckSupport(unsigned int *types)
> +{
> + if (*types == 0)
> + *types = supportedGuestInfoTypes;
> +
> + *types = *types & supportedGuestInfoTypes;
> +}
> +
> +static int
> +qemuDomainGetGuestInfo(virDomainPtr dom,
> + unsigned int types,
> + virTypedParameterPtr *params,
> + int *nparams,
> + unsigned int flags)
> +{
> + virQEMUDriverPtr driver = dom->conn->privateData;
> + virDomainObjPtr vm = NULL;
> + qemuAgentPtr agent;
> + int ret = -1;
> + int rv = -1;
> + int maxparams = 0;
> + char *hostname = NULL;
> + virDomainDefPtr def = NULL;
> + virCapsPtr caps = NULL;
> + unsigned int supportedTypes = types;
> +
> + virCheckFlags(0, ret);
> + qemuDomainGetGuestInfoCheckSupport(&supportedTypes);
> +
> + if (!(vm = qemuDomObjFromDomain(dom)))
> + goto cleanup;
> +
> + if (virDomainGetGuestInfoEnsureACL(dom->conn, vm->def) < 0)
> + goto cleanup;
> +
> + if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
> + goto cleanup;
> +
> + if (!qemuDomainAgentAvailable(vm, true))
> + goto endjob;
> +
> + agent = qemuDomainObjEnterAgent(vm);
> +
> + /* Although the libvirt qemu driver supports all of these guest info types,
> + * some guest agents might be too old to support these commands. If these
> + * info categories were explicitly requested (i.e. 'types' is non-zero),
> + * abort and report an error on any failures, otherwise continue and return
> + * as much info as is supported by the guest agent. */
> + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_USERS) {
> + if (qemuAgentGetUsers(agent, params, nparams, &maxparams) < 0 &&
> + types != 0)
> + goto exitagent;
> + }
> + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_OS) {
> + if (qemuAgentGetOSInfo(agent, params, nparams, &maxparams) < 0
> + && types != 0)
> + goto exitagent;
> + }
> + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_TIMEZONE) {
> + if (qemuAgentGetTimezone(agent, params, nparams, &maxparams) < 0
> + && types != 0)
> + goto exitagent;
> + }
> + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_HOSTNAME) {
> + if (qemuAgentGetHostname(agent, &hostname) < 0) {
> + if (types != 0)
> + goto exitagent;
> + } else {
> + if (virTypedParamsAddString(params, nparams, &maxparams, "hostname",
> + hostname) < 0)
> + goto exitagent;
> + }
> + }
> + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) {
> + if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> + goto exitagent;
> +
> + if (!(def = virDomainDefCopy(vm->def, caps, driver->xmlopt, NULL, false)))
> + goto exitagent;
> +
> + if (qemuAgentGetFSInfoParams(agent, params, nparams, &maxparams, def) < 0 &&
> + types != 0)
> + goto exitagent;
> + }
> +
> + rv = 0;
> +
> + exitagent:
> + qemuDomainObjExitAgent(vm, agent);
> +
> + endjob:
> + qemuDomainObjEndAgentJob(vm);
> +
> + cleanup:
> + virDomainObjEndAPI(&vm);
> + virDomainDefFree(def);
> + virObjectUnref(caps);
> + VIR_FREE(hostname);
> + return rv;
> +}
> +
> static virHypervisorDriver qemuHypervisorDriver = {
> .name = QEMU_DRIVER_NAME,
> .connectURIProbe = qemuConnectURIProbe,
> @@ -23425,6 +23534,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
> .domainCheckpointLookupByName = qemuDomainCheckpointLookupByName, /* 5.6.0 */
> .domainCheckpointGetParent = qemuDomainCheckpointGetParent, /* 5.6.0 */
> .domainCheckpointDelete = qemuDomainCheckpointDelete, /* 5.6.0 */
> + .domainGetGuestInfo = qemuDomainGetGuestInfo, /* 5.6.0 */
> };
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 8/23/19 6:31 PM, Jonathon Jongsma wrote:
> Iimplements the new guest information API by querying requested
> information via the guest agent.
>
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---
> src/qemu/qemu_driver.c | 110 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 110 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 1b051a9424..446266e66b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -23190,6 +23190,115 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
> return ret;
> }
>
> +static unsigned int supportedGuestInfoTypes =
This can be const since it's never modified.
> + VIR_DOMAIN_GUEST_INFO_USERS |
> + VIR_DOMAIN_GUEST_INFO_OS |
> + VIR_DOMAIN_GUEST_INFO_TIMEZONE |
> + VIR_DOMAIN_GUEST_INFO_HOSTNAME |
> + VIR_DOMAIN_GUEST_INFO_FILESYSTEM;
> +
> +static void
> +qemuDomainGetGuestInfoCheckSupport(unsigned int *types)
> +{
> + if (*types == 0)
> + *types = supportedGuestInfoTypes;
> +
> + *types = *types & supportedGuestInfoTypes;
> +}
> +
> +static int
> +qemuDomainGetGuestInfo(virDomainPtr dom,
> + unsigned int types,
> + virTypedParameterPtr *params,
> + int *nparams,
> + unsigned int flags)
> +{
> + virQEMUDriverPtr driver = dom->conn->privateData;
> + virDomainObjPtr vm = NULL;
> + qemuAgentPtr agent;
> + int ret = -1;
> + int rv = -1;
Huh, this @rv and @ret are used a bit misleadinly :-D
> + int maxparams = 0;
> + char *hostname = NULL;
VIR_AUTOFREE()
> + virDomainDefPtr def = NULL;
> + virCapsPtr caps = NULL;
> + unsigned int supportedTypes = types;
> +
> + virCheckFlags(0, ret);
> + qemuDomainGetGuestInfoCheckSupport(&supportedTypes);
> +
> + if (!(vm = qemuDomObjFromDomain(dom)))
> + goto cleanup;
> +
> + if (virDomainGetGuestInfoEnsureACL(dom->conn, vm->def) < 0)
> + goto cleanup;
> +
> + if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
> + goto cleanup;
> +
> + if (!qemuDomainAgentAvailable(vm, true))
> + goto endjob;
> +
> + agent = qemuDomainObjEnterAgent(vm);
> +
> + /* Although the libvirt qemu driver supports all of these guest info types,
> + * some guest agents might be too old to support these commands. If these
> + * info categories were explicitly requested (i.e. 'types' is non-zero),
> + * abort and report an error on any failures, otherwise continue and return
> + * as much info as is supported by the guest agent. */
> + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_USERS) {
> + if (qemuAgentGetUsers(agent, params, nparams, &maxparams) < 0 &&
This might not do what you think it does. qemuAgentGetUsers() (and the
rest of qemuAgentXXX() for that matter) can fail because of variety of
reasons. We want to continue if and only if command was not found and
halt in all other cases. But I'll leave this as is for now and probably
fix this in a follow up patch. Unless you want to work on that. If you
do, then you might want to take a look how we do that in qemu monitor code:
qemuMonitorJSONHasError(reply, "CommandNotFound"))
> + types != 0)
> + goto exitagent;
> + }
> + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_OS) {
> + if (qemuAgentGetOSInfo(agent, params, nparams, &maxparams) < 0
> + && types != 0)
> + goto exitagent;
> + }
> + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_TIMEZONE) {
> + if (qemuAgentGetTimezone(agent, params, nparams, &maxparams) < 0
> + && types != 0)
> + goto exitagent;
> + }
> + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_HOSTNAME) {
> + if (qemuAgentGetHostname(agent, &hostname) < 0) {
> + if (types != 0)
> + goto exitagent;
> + } else {
> + if (virTypedParamsAddString(params, nparams, &maxparams, "hostname",
> + hostname) < 0)
> + goto exitagent;
> + }
> + }
> + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) {
> + if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> + goto exitagent;
> +
> + if (!(def = virDomainDefCopy(vm->def, caps, driver->xmlopt, NULL, false)))
> + goto exitagent;
> +
No need to create the copy of the domain def (which is really expensive).
> + if (qemuAgentGetFSInfoParams(agent, params, nparams, &maxparams, def) < 0 &&
> + types != 0)
> + goto exitagent;
> + }
> +
> + rv = 0;
> +
> + exitagent:
> + qemuDomainObjExitAgent(vm, agent);
> +
> + endjob:
> + qemuDomainObjEndAgentJob(vm);
> +
> + cleanup:
> + virDomainObjEndAPI(&vm);
> + virDomainDefFree(def);
> + virObjectUnref(caps);
> + VIR_FREE(hostname);
> + return rv;
> +}
> +
> static virHypervisorDriver qemuHypervisorDriver = {
> .name = QEMU_DRIVER_NAME,
> .connectURIProbe = qemuConnectURIProbe,
> @@ -23425,6 +23534,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
> .domainCheckpointLookupByName = qemuDomainCheckpointLookupByName, /* 5.6.0 */
> .domainCheckpointGetParent = qemuDomainCheckpointGetParent, /* 5.6.0 */
> .domainCheckpointDelete = qemuDomainCheckpointDelete, /* 5.6.0 */
> + .domainGetGuestInfo = qemuDomainGetGuestInfo, /* 5.6.0 */
s/6/7/
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, 2019-08-26 at 17:29 +0200, Michal Privoznik wrote:
> On 8/23/19 6:31 PM, Jonathon Jongsma wrote:
> > Iimplements the new guest information API by querying requested
> > information via the guest agent.
> >
> > Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> > ---
> > src/qemu/qemu_driver.c | 110
> > +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 110 insertions(+)
> >
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 1b051a9424..446266e66b 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -23190,6 +23190,115 @@
> > qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
> > return ret;
> > }
> >
> > +static unsigned int supportedGuestInfoTypes =
>
> This can be const since it's never modified.
>
> > + VIR_DOMAIN_GUEST_INFO_USERS |
> > + VIR_DOMAIN_GUEST_INFO_OS |
> > + VIR_DOMAIN_GUEST_INFO_TIMEZONE |
> > + VIR_DOMAIN_GUEST_INFO_HOSTNAME |
> > + VIR_DOMAIN_GUEST_INFO_FILESYSTEM;
> > +
> > +static void
> > +qemuDomainGetGuestInfoCheckSupport(unsigned int *types)
> > +{
> > + if (*types == 0)
> > + *types = supportedGuestInfoTypes;
> > +
> > + *types = *types & supportedGuestInfoTypes;
> > +}
> > +
> > +static int
> > +qemuDomainGetGuestInfo(virDomainPtr dom,
> > + unsigned int types,
> > + virTypedParameterPtr *params,
> > + int *nparams,
> > + unsigned int flags)
> > +{
> > + virQEMUDriverPtr driver = dom->conn->privateData;
> > + virDomainObjPtr vm = NULL;
> > + qemuAgentPtr agent;
> > + int ret = -1;
> > + int rv = -1;
>
> Huh, this @rv and @ret are used a bit misleadinly :-D
>
> > + int maxparams = 0;
> > + char *hostname = NULL;
>
> VIR_AUTOFREE()
>
> > + virDomainDefPtr def = NULL;
> > + virCapsPtr caps = NULL;
> > + unsigned int supportedTypes = types;
> > +
> > + virCheckFlags(0, ret);
> > + qemuDomainGetGuestInfoCheckSupport(&supportedTypes);
> > +
> > + if (!(vm = qemuDomObjFromDomain(dom)))
> > + goto cleanup;
> > +
> > + if (virDomainGetGuestInfoEnsureACL(dom->conn, vm->def) < 0)
> > + goto cleanup;
> > +
> > + if (qemuDomainObjBeginAgentJob(driver, vm,
> > QEMU_AGENT_JOB_QUERY) < 0)
> > + goto cleanup;
> > +
> > + if (!qemuDomainAgentAvailable(vm, true))
> > + goto endjob;
> > +
> > + agent = qemuDomainObjEnterAgent(vm);
> > +
> > + /* Although the libvirt qemu driver supports all of these
> > guest info types,
> > + * some guest agents might be too old to support these
> > commands. If these
> > + * info categories were explicitly requested (i.e. 'types' is
> > non-zero),
> > + * abort and report an error on any failures, otherwise
> > continue and return
> > + * as much info as is supported by the guest agent. */
> > + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_USERS) {
> > + if (qemuAgentGetUsers(agent, params, nparams, &maxparams)
> > < 0 &&
>
> This might not do what you think it does. qemuAgentGetUsers() (and
> the
> rest of qemuAgentXXX() for that matter) can fail because of variety
> of
> reasons. We want to continue if and only if command was not found
> and
> halt in all other cases. But I'll leave this as is for now and
> probably
> fix this in a follow up patch. Unless you want to work on that. If
> you
> do, then you might want to take a look how we do that in qemu monitor
> code:
>
> qemuMonitorJSONHasError(reply, "CommandNotFound"))
I actually debated about whether to differentiate based on the error
response, but ended up choosing not to. But I agree that it probably is
better if we do. Should I treat CommandNotFound and CommandDisabled the
same for this purpose?
I also debated whether to first query the 'guest-info' command (which
returns an array of 'supported_commands') to see which commands are
supported by the guest agent. Then I could use that response to set the
value of 'supportedGuestInfoTypes'. In the end I decided that this
added more complication than benefit and decided to simply execute the
command and see whether it failed. Do you have an opionion on this
approach?
I'll revise the patch in either case.
>
> > + types != 0)
> > + goto exitagent;
> > + }
> > + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_OS) {
> > + if (qemuAgentGetOSInfo(agent, params, nparams, &maxparams)
> > < 0
> > + && types != 0)
> > + goto exitagent;
> > + }
> > + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_TIMEZONE) {
> > + if (qemuAgentGetTimezone(agent, params, nparams,
> > &maxparams) < 0
> > + && types != 0)
> > + goto exitagent;
> > + }
> > + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_HOSTNAME) {
> > + if (qemuAgentGetHostname(agent, &hostname) < 0) {
> > + if (types != 0)
> > + goto exitagent;
> > + } else {
> > + if (virTypedParamsAddString(params, nparams,
> > &maxparams, "hostname",
> > + hostname) < 0)
> > + goto exitagent;
> > + }
> > + }
> > + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) {
> > + if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> > + goto exitagent;
> > +
> > + if (!(def = virDomainDefCopy(vm->def, caps, driver-
> > >xmlopt, NULL, false)))
> > + goto exitagent;
> > +
>
> No need to create the copy of the domain def (which is really
> expensive).
Hmm, perhaps this is a case of reusing existing code without thinking
about it carefully enough. Does that also mean that it's not necessary
to copy in qemuDomainGetFSInfo()?
>
> > + if (qemuAgentGetFSInfoParams(agent, params, nparams,
> > &maxparams, def) < 0 &&
> > + types != 0)
> > + goto exitagent;
> > + }
> > +
> > + rv = 0;
> > +
> > + exitagent:
> > + qemuDomainObjExitAgent(vm, agent);
> > +
> > + endjob:
> > + qemuDomainObjEndAgentJob(vm);
> > +
> > + cleanup:
> > + virDomainObjEndAPI(&vm);
> > + virDomainDefFree(def);
> > + virObjectUnref(caps);
> > + VIR_FREE(hostname);
> > + return rv;
> > +}
> > +
> > static virHypervisorDriver qemuHypervisorDriver = {
> > .name = QEMU_DRIVER_NAME,
> > .connectURIProbe = qemuConnectURIProbe,
> > @@ -23425,6 +23534,7 @@ static virHypervisorDriver
> > qemuHypervisorDriver = {
> > .domainCheckpointLookupByName =
> > qemuDomainCheckpointLookupByName, /* 5.6.0 */
> > .domainCheckpointGetParent = qemuDomainCheckpointGetParent,
> > /* 5.6.0 */
> > .domainCheckpointDelete = qemuDomainCheckpointDelete, /*
> > 5.6.0 */
> > + .domainGetGuestInfo = qemuDomainGetGuestInfo, /* 5.6.0 */
>
> s/6/7/
>
> Michal
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 8/26/19 6:08 PM, Jonathon Jongsma wrote:
> On Mon, 2019-08-26 at 17:29 +0200, Michal Privoznik wrote:
>> On 8/23/19 6:31 PM, Jonathon Jongsma wrote:
>>> Iimplements the new guest information API by querying requested
>>> information via the guest agent.
>>>
>>> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
>>> ---
>>> src/qemu/qemu_driver.c | 110
>>> +++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 110 insertions(+)
>>>
>
> I actually debated about whether to differentiate based on the error
> response, but ended up choosing not to. But I agree that it probably is
> better if we do. Should I treat CommandNotFound and CommandDisabled the
> same for this purpose?
Very good point. Yeah, they should be traeted the same.
>
> I also debated whether to first query the 'guest-info' command (which
> returns an array of 'supported_commands') to see which commands are
> supported by the guest agent. Then I could use that response to set the
> value of 'supportedGuestInfoTypes'. In the end I decided that this
> added more complication than benefit and decided to simply execute the
> command and see whether it failed. Do you have an opionion on this
> approach?
Agreed, it adds needless complexity.
>
> I'll revise the patch in either case.
>
>
>
>>
>>> + types != 0)
>>> + goto exitagent;
>>> + }
>>> + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_OS) {
>>> + if (qemuAgentGetOSInfo(agent, params, nparams, &maxparams)
>>> < 0
>>> + && types != 0)
>>> + goto exitagent;
>>> + }
>>> + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_TIMEZONE) {
>>> + if (qemuAgentGetTimezone(agent, params, nparams,
>>> &maxparams) < 0
>>> + && types != 0)
>>> + goto exitagent;
>>> + }
>>> + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_HOSTNAME) {
>>> + if (qemuAgentGetHostname(agent, &hostname) < 0) {
>>> + if (types != 0)
>>> + goto exitagent;
>>> + } else {
>>> + if (virTypedParamsAddString(params, nparams,
>>> &maxparams, "hostname",
>>> + hostname) < 0)
>>> + goto exitagent;
>>> + }
>>> + }
>>> + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) {
>>> + if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
>>> + goto exitagent;
>>> +
>>> + if (!(def = virDomainDefCopy(vm->def, caps, driver-
>>>> xmlopt, NULL, false)))
>>> + goto exitagent;
>>> +
>>
>> No need to create the copy of the domain def (which is really
>> expensive).
>
> Hmm, perhaps this is a case of reusing existing code without thinking
> about it carefully enough. Does that also mean that it's not necessary
> to copy in qemuDomainGetFSInfo()?
D'oh! Yes it is not necessary, we have jobs to prevent domain object
modifications when the lock is dropped. Let me post a patch for that.
The domain def copying was introduced in v3.0.0-rc1~336 but the commit
message is unclear why the copy is needed. Anyway, we have jobs so that
we don't need to create duplicates.
BTW: jobs are again a complex idea. But put simply: it's an integer
inside qemuDomainObjPrivatePtr (which can be accessed via
vm->privateData) and acquiring a job means setting the integer to a
non-zero value (with optional wait if the integer is set) and releasing
the job then means setting the integer to zero. This means that if two
threads fight over access of the same domain only one will succeed in
setting the integer and the other has to wait until the job is released.
But this means that the domain can be locked and unlocked at will if it
has a job acquired - no one else will modify the domain object (nor its
definition since it's part of domain object).
This is documented in src/qemu/THREADS.txt
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.