[libvirt] [PATCH 0/7] add virDomainGetGuestInfo()
This series adds several bits of guest information provided by a new API function virDomainGetGuestInfo(). There is an implementation for qemu using the guest agent. In particular, it adds information about logged-in users, guest OS, and timezone. I had previously submitted a patch series with a dedicated API function for querying guest users that returned an array of structures describing the users. It was suggested that I convert his to a more futureproof design using typed parameters and also combine it with other bits of guest information similar to virDomainListGetStats(). In fact, there were several bugs that requested this information be added to the 'bulk stats' API, but Peter Krempa suggested adding a new API for all data queried from the guest agent (see https://bugzilla.redhat.com/show_bug.cgi?id=1705514). This is that API proposal. It follows the stats API quite closely, and tries to return data in similar ways (for example, the "users.N.*" field name scheme was inspired by various stats fields). I plan to follow this series up with a patch that adds fsinfo to the returned information, but wanted to get comments on this approach now. Here's an example of the output of the virsh command added in this patch: virsh # guestinfo win7 users.count : 1 users.0.name : test users.0.domain : test-PC users.0.login_time : 1564664122706 os.id : mswindows os.name : Microsoft Windows os.pretty-name : Windows 7 Professional os.version : Microsoft Windows 77 os.version-id : os.machine : x86_64 os.variant : client os.variant-id : client os.kernel-release : 7601 os.kernel-version : 6.1 timezone.name : Central Daylight Time timezone.offset : -18000 hostname : TEST-PC Jonathon Jongsma (7): lib: add virDomainGetGuestInfo() remote: implement virDomainGetGuestInfo qemu_agent: add helper for getting guest users qemu_agent: add helper function for querying OS info qemu_agent: add helper for querying timezone info qemu: Implement virDomainGetGuestInfo() virsh: add 'guestinfo' command include/libvirt/libvirt-domain.h | 13 + src/driver-hypervisor.h | 8 + src/libvirt-domain.c | 105 ++++++++ src/libvirt_public.syms | 1 + src/qemu/qemu_agent.c | 190 +++++++++++++++ src/qemu/qemu_agent.h | 4 + src/qemu/qemu_driver.c | 77 ++++++ src/remote/remote_daemon_dispatch.c | 41 ++++ src/remote/remote_driver.c | 53 ++++ src/remote/remote_protocol.x | 21 +- src/remote_protocol-structs | 12 + tests/qemuagenttest.c | 366 ++++++++++++++++++++++++++++ tools/virsh-domain.c | 53 ++++ 13 files changed, 943 insertions(+), 1 deletion(-) -- 2.21.0
This API is intended to aggregate several guest agent information queries and is ispired by stats API virDomainListGetStats(). It is anticipated that this information will be provided by a guest agent running within the domain. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- include/libvirt/libvirt-domain.h | 13 ++++ src/driver-hypervisor.h | 8 +++ src/libvirt-domain.c | 105 +++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 127 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index f160ee88b5..93e0d36ff9 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -4902,4 +4902,17 @@ int virDomainGetLaunchSecurityInfo(virDomainPtr domain, int *nparams, unsigned int flags); +typedef enum { + VIR_DOMAIN_GUEST_INFO_USERS = (1 << 0), /* return active users */ + VIR_DOMAIN_GUEST_INFO_OS = (1 << 1), /* return OS information */ + VIR_DOMAIN_GUEST_INFO_TIMEZONE = (1 << 2), /* return timezone information */ + VIR_DOMAIN_GUEST_INFO_HOSTNAME = (1 << 3), /* return hostname information */ +} virDomainGuestInfoTypes; + +int virDomainGetGuestInfo(virDomainPtr domain, + unsigned int types, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags); + #endif /* LIBVIRT_DOMAIN_H */ diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index c1632ae4c6..58eb731e85 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1359,6 +1359,13 @@ typedef int (*virDrvDomainCheckpointDelete)(virDomainCheckpointPtr checkpoint, unsigned int flags); +typedef int +(*virDrvDomainGetGuestInfo)(virDomainPtr domain, + unsigned int types, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags); + typedef struct _virHypervisorDriver virHypervisorDriver; typedef virHypervisorDriver *virHypervisorDriverPtr; @@ -1617,4 +1624,5 @@ struct _virHypervisorDriver { virDrvDomainCheckpointLookupByName domainCheckpointLookupByName; virDrvDomainCheckpointGetParent domainCheckpointGetParent; virDrvDomainCheckpointDelete domainCheckpointDelete; + virDrvDomainGetGuestInfo domainGetGuestInfo; }; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 2fe9bb8e91..7cd17f346d 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12212,6 +12212,111 @@ virDomainSetVcpu(virDomainPtr domain, return -1; } +/** + * virDomainGetGuestInfo: + * @domain: pointer to domain object + * @types: types of information to return, binary-OR of virDomainGuestInfoTypes + * @params: location to store the guest info parameters + * @nparams: number of items in @params + * @flags: currently unused, set to 0 + * + * Queries the guest agent for the various information about the guest system. + * The reported data depends on the guest agent implementation. the information + * is returned as an array of typed parameters containing the individual + * parameters. The parameter name for each information field consists of a + * dot-separated strign containing the name of the requested group followed by + * a group-specific description of the statistic value. + * + * The information groups are enabled using the @types parameter which is a + * binary-OR of enum virDomainGuestInfoTypes. The following groups are available + * (although not necessarily implemented for each hypervisor): + * + * VIR_DOMAIN_GUEST_INFO_USERS: + * returns information about users that are currently logged in within the + * guest domain. The typed parameter keys are in this format: + * + * "users.count" - the number of active users on this domain as an + * unsigned int + * "users.<num>.name - username of the user as a string + * "users.<num>.domain - domain of the user as a string (may only be + * present on certain guest types) + * "users.<num>.login_time - the login time of a user in milliseconds + * since the epoch as unsigned long long + * + * VIR_DOMAIN_GUEST_INFO_OS: + * Return information about the operating system running within the guest. The + * typed parameter keys are in this format: + * + * "os.id" - a string identifying the operating system + * "os.name" - the name of the operating system, suitable for presentation + * to a user, as a string + * "os.pretty-name" - a pretty name for the operating system, suitable for + * presentation to a user, as a string + * "os.version" - the version of the operating system suitable for + * presentation to a user, as a string + * "os.version-id" - the version id of the operating system suitable for + * processing by scripts, as a string + * "os.kernel-release" - the release of the operating system kernel, as a + * string + * "os.kernel-version" - the version of the operating system kernel, as a + * string + * "os.machine" - the machine hardware name as a string + * "os.variant" - a specific variant or edition of the operating system + * suitable for presentation to a user, as a string + * "os.variant-id" - the id for a specific variant or edition of the + * operating system, as a string + * + * VIR_DOMAIN_GUEST_INFO_TIMEZONE: + * Returns information about the timezone within the domain. The typed + * parameter keys are in this format: + * + * "timezone.name" - the name of the timezone as a string + * "timezone.offset" - the offset to UTC in seconds as an int + * + * VIR_DOMAIN_GUEST_INFO_FILESYSTEM: + * FIXME + * + * Using 0 for @types returns all information groups supported by the given + * hypervisor. + * + * This API requires the VM to run. The caller is responsible for calling + * virTypedParamsFree to free memory returned in @params. + * + * Returns 0 on success, -1 on error. + */ +int virDomainGetGuestInfo(virDomainPtr domain, + unsigned int types, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(domain, "types=%u, params=%p, nparams=%p, flags=0x%x", + types, params, nparams, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckReadOnlyGoto(domain->conn->flags, error); + + virCheckNonNullArgGoto(params, error); + virCheckNonNullArgGoto(nparams, error); + + if (domain->conn->driver->domainGetGuestInfo) { + int ret; + ret = domain->conn->driver->domainGetGuestInfo(domain, types, + params, nparams, flags); + + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} /** * virDomainSetBlockThreshold: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 54256b6317..e196fd11d9 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -850,6 +850,7 @@ LIBVIRT_5.6.0 { virDomainCheckpointLookupByName; virDomainCheckpointRef; virDomainListAllCheckpoints; + virDomainGetGuestInfo; } LIBVIRT_5.5.0; # .... define new API here using predicted next version number .... -- 2.21.0
Add daemon and client code to serialize/deserialize virDomainGetGuestInfo(). Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/remote/remote_daemon_dispatch.c | 41 ++++++++++++++++++++++ src/remote/remote_driver.c | 53 +++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 21 +++++++++++- src/remote_protocol-structs | 12 +++++++ 4 files changed, 126 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 444c03a644..5bd0172da7 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -7485,3 +7485,44 @@ remoteSerializeDomainDiskErrors(virDomainDiskErrorPtr errors, } return -1; } + +static int +remoteDispatchDomainGetGuestInfo(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_guest_info_args *args, + remote_domain_get_guest_info_ret *ret) +{ + int rv = -1; + struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virDomainPtr dom = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if (virDomainGetGuestInfo(dom, args->types, ¶ms, &nparams, args->flags) < 0) + goto cleanup; + + if (virTypedParamsSerialize(params, nparams, + (virTypedParameterRemotePtr *) &ret->params.params_val, + &ret->params.params_len, + VIR_TYPED_PARAM_STRING_OKAY) < 0) + goto cleanup; + + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virObjectUnref(dom); + + return rv; +} diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 72c2336b7a..dbfdbbe97d 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8169,6 +8169,58 @@ remoteNetworkPortGetParameters(virNetworkPortPtr port, return rv; } +static int +remoteDomainGetGuestInfo(virDomainPtr dom, + unsigned int types, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + int rv = -1; + struct private_data *priv = dom->conn->privateData; + remote_domain_get_guest_info_args args; + remote_domain_get_guest_info_ret ret; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, dom); + + args.types = types; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + + if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_GUEST_INFO, + (xdrproc_t)xdr_remote_domain_get_guest_info_args, (char *)&args, + (xdrproc_t)xdr_remote_domain_get_guest_info_ret, (char *)&ret) == -1) + goto done; + + if (ret.params.params_len > REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Too many params in guestinfo: %d for limit %d"), + ret.params.params_len, REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX); + goto cleanup; + } + + if (params) { + if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX, + params, + nparams) < 0) + goto cleanup; + } + + rv = 0; + + cleanup: + xdr_free((xdrproc_t)xdr_remote_domain_get_guest_info_ret, + (char *) &ret); + + done: + remoteDriverUnlock(priv); + return rv; +} /* get_nonnull_domain and get_nonnull_network turn an on-wire * (name, uuid) pair into virDomainPtr or virNetworkPtr object. @@ -8596,6 +8648,7 @@ static virHypervisorDriver hypervisor_driver = { .domainCheckpointLookupByName = remoteDomainCheckpointLookupByName, /* 5.6.0 */ .domainCheckpointGetParent = remoteDomainCheckpointGetParent, /* 5.6.0 */ .domainCheckpointDelete = remoteDomainCheckpointDelete, /* 5.6.0 */ + .domainGetGuestInfo = remoteDomainGetGuestInfo, /* 5.6.0 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 118369e2b3..75c2bc69ff 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -269,6 +269,9 @@ const REMOTE_NODE_SEV_INFO_MAX = 64; /* Upper limit on number of launch security information entries */ const REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX = 64; +/* Upper limit on number of parameters describing a guest */ +const REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX = 2048; + /* * Upper limit on list of network port parameters */ @@ -3723,6 +3726,16 @@ struct remote_domain_checkpoint_delete_args { unsigned int flags; }; +struct remote_domain_get_guest_info_args { + remote_nonnull_domain dom; + unsigned int types; + unsigned int flags; +}; + +struct remote_domain_get_guest_info_ret { + remote_typed_param params<REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX>; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -6584,5 +6597,11 @@ enum remote_procedure { * @generate: both * @acl: domain:checkpoint */ - REMOTE_PROC_DOMAIN_CHECKPOINT_DELETE = 417 + REMOTE_PROC_DOMAIN_CHECKPOINT_DELETE = 417, + + /** + * @generate: none + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_GET_GUEST_INFO = 418 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index a42b4a9671..616c3d5d52 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -3105,6 +3105,17 @@ struct remote_domain_checkpoint_delete_args { remote_nonnull_domain_checkpoint checkpoint; u_int flags; }; +struct remote_domain_get_guest_info_args { + remote_nonnull_domain dom; + u_int types; + u_int flags; +}; +struct remote_domain_get_guest_info_ret { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -3523,4 +3534,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_CHECKPOINT_LOOKUP_BY_NAME = 415, REMOTE_PROC_DOMAIN_CHECKPOINT_GET_PARENT = 416, REMOTE_PROC_DOMAIN_CHECKPOINT_DELETE = 417, + REMOTE_PROC_DOMAIN_GET_GUEST_INFO = 418, }; -- 2.21.0
This function fetches the list of logged-in users from the qemu agent and adds them to a list of typed parameters so that they can be used internally in libvirt. Also add some basic tests for the function. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_agent.c | 92 +++++++++++++++++++++++ src/qemu/qemu_agent.h | 2 + tests/qemuagenttest.c | 168 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 262 insertions(+) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 361db299a5..de31f33dba 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2240,3 +2240,95 @@ qemuAgentSetUserPassword(qemuAgentPtr mon, VIR_FREE(password64); return ret; } + +int +qemuAgentGetUsers(qemuAgentPtr mon, + virTypedParameterPtr *params, + int *nparams, + int *maxparams) +{ + int ret = -1; + size_t i; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data = NULL; + size_t ndata; + const char *strvalue; + + if (!(cmd = qemuAgentMakeCommand("guest-get-users", NULL))) + return -1; + + if (qemuAgentCommand(mon, cmd, &reply, true, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + goto cleanup; + + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest-get-users reply was missing return data")); + goto cleanup; + } + + if (!virJSONValueIsArray(data)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed guest-get-users data array")); + goto cleanup; + } + + ndata = virJSONValueArraySize(data); + + if (virTypedParamsAddUInt(params, nparams, maxparams, + "users.count", ndata) < 0) + goto cleanup; + + for (i = 0; i < ndata; i++) { + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; + virJSONValuePtr entry = virJSONValueArrayGet(data, i); + + if (!entry) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("array element missing in guest-get-users return " + "value")); + goto cleanup; + } + + if (!(strvalue = virJSONValueObjectGetString(entry, "user"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'user' missing in reply of guest-get-users")); + goto cleanup; + } + + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "users.%zu.name", i); + if (virTypedParamsAddString(params, nparams, maxparams, + param_name, strvalue) < 0) + goto cleanup; + + /* 'domain' is only present for windows guests */ + if ((strvalue = virJSONValueObjectGetString(entry, "domain"))) { + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "users.%zu.domain", i); + if (virTypedParamsAddString(params, nparams, maxparams, + param_name, strvalue) < 0) + goto cleanup; + } + + double logintime; + if (virJSONValueObjectGetNumberDouble(entry, "login-time", &logintime) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'login-time' missing in reply of guest-get-users")); + goto cleanup; + } + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "users.%zu.login_time", i); + if (virTypedParamsAddULLong(params, nparams, maxparams, + param_name, logintime * 1000) < 0) + goto cleanup; + } + + ret = ndata; + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 6ae9fe54da..05621b521a 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -120,3 +120,5 @@ int qemuAgentSetUserPassword(qemuAgentPtr mon, const char *user, const char *password, bool crypted); + +int qemuAgentGetUsers(qemuAgentPtr mon, virTypedParameterPtr *params, int *nparams, int *maxparams); diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 2f79986207..1477c0539c 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -902,6 +902,173 @@ testQemuAgentGetInterfaces(const void *data) return ret; } +static const char testQemuAgentUsersResponse[] = + "{\"return\": " + " [" + " {\"user\": \"test\"," + " \"login-time\": 1561739203.584038" + " }," + " {\"user\": \"test2\"," + " \"login-time\": 1561739229.190697" + " }" + " ]" + "}"; + +static const char testQemuAgentUsersResponse2[] = + "{\"return\": " + " [" + " {\"user\": \"test\"," + " \"domain\": \"DOMAIN\"," + " \"login-time\": 1561739203.584038" + " }" + " ]" + "}"; + +static int getUserInfo(virTypedParameterPtr params, int nparams, size_t nth, + const char **username, const char **domain, + unsigned long long *logintime) +{ + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; + + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "users.%zu.name", nth); + if (username && + virTypedParamsGetString(params, nparams, param_name, username) < 0) + return -1; + + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "users.%zu.domain", nth); + if (domain && + virTypedParamsGetString(params, nparams, param_name, domain) < 0) + return -1; + + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "users.%zu.login_time", nth); + if (logintime && + virTypedParamsGetULLong(params, nparams, param_name, logintime) < 0) + return -1; + + return 0; +} + +static int +testQemuAgentUsers(const void *data) +{ + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt); + virTypedParameterPtr params = NULL; + int nparams = 0; + int maxparams = 0; + int ret = -1; + const char *username = NULL; + const char *domain = NULL; + unsigned long long logintime = 0; + unsigned int count; + + if (!test) + return -1; + + if (qemuMonitorTestAddAgentSyncResponse(test) < 0) + goto cleanup; + + if (qemuMonitorTestAddItem(test, "guest-get-users", + testQemuAgentUsersResponse) < 0) + goto cleanup; + + /* get users */ + if (qemuAgentGetUsers(qemuMonitorTestGetAgent(test), + ¶ms, &nparams, &maxparams) < 0) + goto cleanup; + + if (virTypedParamsGetUInt(params, nparams, "users.count", &count) < 0) + goto cleanup; + if (count != 2) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Expected '2' users, got '%u'", count); + goto cleanup; + } + + getUserInfo(params, nparams, 0, &username, NULL, &logintime); + if (!username) { + virReportError(VIR_ERR_INTERNAL_ERROR, "Missing username"); + goto cleanup; + } + if (STRNEQ(username, "test")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Expected user name 'test', got '%s'", username); + goto cleanup; + } + if (logintime != 1561739203584) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Expected login time of '1561739203584', got '%llu'", + logintime); + goto cleanup; + } + + getUserInfo(params, nparams, 1, &username, NULL, &logintime); + if (STRNEQ(username, "test2")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Expected user name 'test2', got '%s'", username); + goto cleanup; + } + if (logintime != 1561739229190) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Expected login time of '1561739229190', got '%llu'", + logintime); + goto cleanup; + } + + if (qemuMonitorTestAddAgentSyncResponse(test) < 0) + goto cleanup; + + if (qemuMonitorTestAddItem(test, "guest-get-users", + testQemuAgentUsersResponse2) < 0) + goto cleanup; + + virTypedParamsFree(params, nparams); + params = NULL; + nparams = 0; + maxparams = 0; + + /* get users with domain */ + if (qemuAgentGetUsers(qemuMonitorTestGetAgent(test), + ¶ms, &nparams, &maxparams) < 0) + goto cleanup; + + if (virTypedParamsGetUInt(params, nparams, "users.count", &count) < 0) + goto cleanup; + if (count != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Expected '1' user, got '%u'", count); + goto cleanup; + } + + getUserInfo(params, nparams, 0, &username, &domain, &logintime); + if (STRNEQ(username, "test")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Expected user name 'test', got '%s'", username); + goto cleanup; + } + if (logintime != 1561739203584) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Expected login time of '1561739203584', got '%llu'", + logintime); + goto cleanup; + } + if (STRNEQ(domain, "DOMAIN")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Expected domain 'DOMAIN', got '%s'", domain); + goto cleanup; + } + ret = 0; + + cleanup: + virTypedParamsFree(params, nparams); + qemuMonitorTestFree(test); + return ret; +} + + static int mymain(void) { @@ -931,6 +1098,7 @@ mymain(void) DO_TEST(CPU); DO_TEST(ArbitraryCommand); DO_TEST(GetInterfaces); + DO_TEST(Users); DO_TEST(Timeout); /* Timeout should always be called last */ -- 2.21.0
This function queries the guest operating system information and adds the returned information to an array of typed parameters with field names intended to be returned in virDomainGetGuestInfo(). Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_agent.c | 52 ++++++++++++++++++ src/qemu/qemu_agent.h | 1 + tests/qemuagenttest.c | 122 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index de31f33dba..7418749459 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2332,3 +2332,55 @@ qemuAgentGetUsers(qemuAgentPtr mon, return ret; } +int +qemuAgentGetOSInfo(qemuAgentPtr mon, + virTypedParameterPtr *params, + int *nparams, + int *maxparams) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data = NULL; + const char *result; + + if (!(cmd = qemuAgentMakeCommand("guest-get-osinfo", NULL))) + return -1; + + if (qemuAgentCommand(mon, cmd, &reply, true, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + goto cleanup; + + if (!(data = virJSONValueObjectGetObject(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest-get-osinfo reply was missing return data")); + goto cleanup; + } + +# define OSINFO_ADD_PARAM(agent_string_, param_string_) \ + do { \ + if ((result = virJSONValueObjectGetString(data, agent_string_))) { \ + if (virTypedParamsAddString(params, nparams, maxparams, \ + param_string_, result) < 0) { \ + goto cleanup; \ + } \ + } \ + } while (0) + OSINFO_ADD_PARAM("id", "os.id"); + OSINFO_ADD_PARAM("name", "os.name"); + OSINFO_ADD_PARAM("pretty-name", "os.pretty-name"); + OSINFO_ADD_PARAM("version", "os.version"); + OSINFO_ADD_PARAM("version-id", "os.version-id"); + OSINFO_ADD_PARAM("machine", "os.machine"); + OSINFO_ADD_PARAM("variant", "os.variant"); + OSINFO_ADD_PARAM("variant-id", "os.variant-id"); + OSINFO_ADD_PARAM("kernel-release", "os.kernel-release"); + OSINFO_ADD_PARAM("kernel-version", "os.kernel-version"); + + ret = 0; + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 05621b521a..ee019455e5 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -122,3 +122,4 @@ int qemuAgentSetUserPassword(qemuAgentPtr mon, bool crypted); int qemuAgentGetUsers(qemuAgentPtr mon, virTypedParameterPtr *params, int *nparams, int *maxparams); +int qemuAgentGetOSInfo(qemuAgentPtr mon, virTypedParameterPtr *params, int *nparams, int *maxparams); diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 1477c0539c..cb5a2f65e8 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -1068,6 +1068,127 @@ testQemuAgentUsers(const void *data) return ret; } +static const char testQemuAgentOSInfoResponse[] = + "{\"return\": " + " {\"name\":\"CentOS Linux\", " + " \"kernel-release\":\"3.10.0-862.14.4.el7.x86_64\", " + " \"version\":\"7 (Core)\", " + " \"pretty-name\":\"CentOS Linux 7 (Core)\", " + " \"version-id\":\"7\", " + " \"kernel-version\":\"#1 SMP Wed Sep 26 15:12:11 UTC 2018\", " + " \"machine\":\"x86_64\", " + " \"id\":\"centos\"} " + "}"; + +static const char testQemuAgentOSInfoResponse2[] = + "{\"return\": " + " {\"name\":\"Microsoft Windows\", " + " \"kernel-release\":\"7601\", " + " \"version\":\"Microsoft Windows 77\", " + " \"variant\":\"client\", " + " \"pretty-name\":\"Windows 7 Professional\", " + " \"version-id\":\"\", " + " \"variant-id\":\"client\", " + " \"kernel-version\":\"6.1\", " + " \"machine\":\"x86_64\", " + " \"id\":\"mswindows\"} " + "}"; + +static int +testQemuAgentOSInfo(const void *data) +{ + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt); + virTypedParameterPtr params = NULL; + int nparams = 0; + int maxparams = 0; + int ret = -1; + + if (!test) + return -1; + + if (qemuMonitorTestAddAgentSyncResponse(test) < 0) + goto cleanup; + + if (qemuMonitorTestAddItem(test, "guest-get-osinfo", + testQemuAgentOSInfoResponse) < 0) + goto cleanup; + + /* get osinfo */ + if (qemuAgentGetOSInfo(qemuMonitorTestGetAgent(test), + ¶ms, &nparams, &maxparams) < 0) + goto cleanup; + + if (nparams != 8) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Expected 8 params, got %d", nparams); + goto cleanup; + } +# define VALIDATE_PARAM(param_name_, expected_) \ + do { \ + const char *value_ = NULL; \ + if (virTypedParamsGetString(params, nparams, param_name_, &value_) < 0 || \ + value_ == NULL) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, "missing param '%s'", param_name_); \ + goto cleanup; \ + } \ + if (STRNEQ(value_, expected_)) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "Expected name '%s', got '%s'", expected_, value_); \ + goto cleanup; \ + } \ + } while (0) + + VALIDATE_PARAM("os.id", "centos"); + VALIDATE_PARAM("os.name", "CentOS Linux"); + VALIDATE_PARAM("os.version", "7 (Core)"); + VALIDATE_PARAM("os.version-id", "7"); + VALIDATE_PARAM("os.pretty-name", "CentOS Linux 7 (Core)"); + VALIDATE_PARAM("os.kernel-release", "3.10.0-862.14.4.el7.x86_64"); + VALIDATE_PARAM("os.kernel-version", "#1 SMP Wed Sep 26 15:12:11 UTC 2018"); + VALIDATE_PARAM("os.machine", "x86_64"); + virTypedParamsFree(params, nparams); + params = NULL; + nparams = 0; + maxparams = 0; + + if (qemuMonitorTestAddAgentSyncResponse(test) < 0) + goto cleanup; + + if (qemuMonitorTestAddItem(test, "guest-get-osinfo", + testQemuAgentOSInfoResponse2) < 0) + goto cleanup; + + /* get users with domain */ + if (qemuAgentGetOSInfo(qemuMonitorTestGetAgent(test), + ¶ms, &nparams, &maxparams) < 0) + goto cleanup; + + if (nparams != 10) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Expected 10 params, got %d", nparams); + goto cleanup; + } + + VALIDATE_PARAM("os.id", "mswindows"); + VALIDATE_PARAM("os.name", "Microsoft Windows"); + VALIDATE_PARAM("os.pretty-name", "Windows 7 Professional"); + VALIDATE_PARAM("os.version", "Microsoft Windows 77"); + VALIDATE_PARAM("os.version-id", ""); + VALIDATE_PARAM("os.variant", "client"); + VALIDATE_PARAM("os.variant-id", "client"); + VALIDATE_PARAM("os.kernel-release", "7601"); + VALIDATE_PARAM("os.kernel-version", "6.1"); + VALIDATE_PARAM("os.machine", "x86_64"); + virTypedParamsFree(params, nparams); + + ret = 0; + + cleanup: + qemuMonitorTestFree(test); + return ret; +} + static int mymain(void) @@ -1099,6 +1220,7 @@ mymain(void) DO_TEST(ArbitraryCommand); DO_TEST(GetInterfaces); DO_TEST(Users); + DO_TEST(OSInfo); DO_TEST(Timeout); /* Timeout should always be called last */ -- 2.21.0
This function queries timezone information within the guest and adds the information to an array of typed parameters with field names intended to be returned to virDomainGetGuestInfo() Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- src/qemu/qemu_agent.c | 46 ++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 1 + tests/qemuagenttest.c | 76 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 7418749459..1b2e38d8b3 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2384,3 +2384,49 @@ qemuAgentGetOSInfo(qemuAgentPtr mon, virJSONValueFree(reply); return ret; } + +int +qemuAgentGetTimezone(qemuAgentPtr mon, + virTypedParameterPtr *params, + int *nparams, + int *maxparams) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + virJSONValuePtr data = NULL; + const char *name; + int offset; + + if (!(cmd = qemuAgentMakeCommand("guest-get-timezone", NULL))) + return -1; + + if (qemuAgentCommand(mon, cmd, &reply, true, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + goto cleanup; + + if (!(data = virJSONValueObjectGetObject(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest-get-timezone reply was missing return data")); + goto cleanup; + } + + if ((name = virJSONValueObjectGetString(data, "zone")) == NULL) + goto cleanup; + if (virTypedParamsAddString(params, nparams, maxparams, + "timezone.name", name) < 0) + goto cleanup; + + if ((virJSONValueObjectGetNumberInt(data, "offset", &offset)) < 0) + goto cleanup; + if (virTypedParamsAddInt(params, nparams, maxparams, + "timezone.offset", offset) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index ee019455e5..69b0176855 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -123,3 +123,4 @@ int qemuAgentSetUserPassword(qemuAgentPtr mon, int qemuAgentGetUsers(qemuAgentPtr mon, virTypedParameterPtr *params, int *nparams, int *maxparams); int qemuAgentGetOSInfo(qemuAgentPtr mon, virTypedParameterPtr *params, int *nparams, int *maxparams); +int qemuAgentGetTimezone(qemuAgentPtr mon, virTypedParameterPtr *params, int *nparams, int *maxparams); diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index cb5a2f65e8..03d6e7e1b8 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -1189,7 +1189,82 @@ testQemuAgentOSInfo(const void *data) return ret; } +static const char testQemuAgentTimezoneResponse1[] = +"{\"return\":{\"zone\":\"IST\",\"offset\":19800}}"; +static const char testQemuAgentTimezoneResponse2[] = +"{\"return\":{\"zone\":\"CEST\",\"offset\":7200}}"; +static const char testQemuAgentTimezoneResponse3[] = +"{\"return\":{\"zone\":\"NDT\",\"offset\":-9000}}"; +static const char testQemuAgentTimezoneResponse4[] = +"{\"return\":{\"zone\":\"PDT\",\"offset\":-25200}}"; +static int +testQemuAgentTimezone(const void *data) +{ + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; + qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt); + int ret = -1; + + if (!test) + return -1; + +#define VALIDATE_TIMEZONE(response_, expected_name_, expected_offset_) \ + do { \ + virTypedParameterPtr params_ = NULL; \ + int nparams_ = 0; \ + int maxparams_ = 0; \ + const char *name_ = NULL; \ + int offset_; \ + if (qemuMonitorTestAddAgentSyncResponse(test) < 0) \ + goto cleanup; \ + if (qemuMonitorTestAddItem(test, "guest-get-timezone", \ + response_) < 0) \ + goto cleanup; \ + if (qemuAgentGetTimezone(qemuMonitorTestGetAgent(test), \ + ¶ms_, &nparams_, &maxparams_) < 0) \ + goto cleanup; \ + if (nparams_ != 2) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "Expected 2 params, got %d", nparams_); \ + goto cleanup; \ + } \ + if (virTypedParamsGetString(params_, nparams_, \ + "timezone.name", &name_) < 0) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, "missing param '%s'", \ + "tiemzone.name"); \ + goto cleanup; \ + } \ + if (STRNEQ(name_, expected_name_)) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "Expected name '%s', got '%s'", expected_name_, name_); \ + goto cleanup; \ + } \ + if (virTypedParamsGetInt(params_, nparams_, \ + "timezone.offset", &offset_) < 0) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, "missing param '%s'", \ + "tiemzone.offset"); \ + goto cleanup; \ + } \ + if (offset_ != expected_offset_) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "Expected offset '%i', got '%i'", offset_, \ + expected_offset_); \ + goto cleanup; \ + } \ + virTypedParamsFree(params_, nparams_); \ + } while (0) + + VALIDATE_TIMEZONE(testQemuAgentTimezoneResponse1, "IST", 19800); + VALIDATE_TIMEZONE(testQemuAgentTimezoneResponse2, "CEST", 7200); + VALIDATE_TIMEZONE(testQemuAgentTimezoneResponse3, "NDT", -9000); + VALIDATE_TIMEZONE(testQemuAgentTimezoneResponse4, "PDT", -25200); + + ret = 0; + + cleanup: + qemuMonitorTestFree(test); + return ret; +} static int mymain(void) { @@ -1221,6 +1296,7 @@ mymain(void) DO_TEST(GetInterfaces); DO_TEST(Users); DO_TEST(OSInfo); + DO_TEST(Timezone); DO_TEST(Timeout); /* Timeout should always be called last */ -- 2.21.0
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 | 77 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4ca3eb7bde..042e91cfed 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -23061,6 +23061,82 @@ 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; + +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; + + virCheckFlags(0, ret); + qemuDomainGetGuestInfoCheckSupport(&types); + + 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); + if ((types & VIR_DOMAIN_GUEST_INFO_USERS) && + qemuAgentGetUsers(agent, params, nparams, &maxparams) < 0) + goto exitagent; + if ((types & VIR_DOMAIN_GUEST_INFO_OS) && + qemuAgentGetOSInfo(agent, params, nparams, &maxparams) < 0) + goto exitagent; + if ((types & VIR_DOMAIN_GUEST_INFO_TIMEZONE) && + qemuAgentGetTimezone(agent, params, nparams, &maxparams) < 0) + goto exitagent; + if ((types & VIR_DOMAIN_GUEST_INFO_HOSTNAME) && + qemuAgentGetHostname(agent, &hostname) < 0) + goto exitagent; + if (virTypedParamsAddString(params, nparams, &maxparams, "hostname", + hostname) < 0) + goto exitagent; + + rv = 0; + +exitagent: + qemuDomainObjExitAgent(vm, agent); + + endjob: + qemuDomainObjEndAgentJob(vm); + + cleanup: + virDomainObjEndAPI(&vm); + VIR_FREE(hostname); + return rv; +} + static virHypervisorDriver qemuHypervisorDriver = { .name = QEMU_DRIVER_NAME, .connectURIProbe = qemuConnectURIProbe, @@ -23296,6 +23372,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
The 'guestinfo' command uses the new virDomainGetGuestInfo() API to query information about the specified domain and print it out for the user. The output is modeled roughly on the 'domstats' command. Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> --- tools/virsh-domain.c | 53 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index e79dc75342..83249768ad 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -14038,6 +14038,53 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd) return ret; } +/* + * "guestinfo" command + */ +static const vshCmdInfo info_guestinfo[] = { + {.name = "help", + .data = N_("query information about the guest (via agent)") + }, + {.name = "desc", + .data = N_("Use the guest agent to query various information from guest's " + "point of view") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_guestinfo[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), + {.name = NULL} +}; + +static bool +cmdGuestInfo(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + bool ret = false; + virTypedParameterPtr params = NULL; + int nparams = 0; + int i; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (virDomainGetGuestInfo(dom, 0, ¶ms, &nparams, 0) < 0) + goto cleanup; + + for (i = 0; i < nparams; i++) { + char *str = vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "%-20s: %s\n", params[i].field, str); + VIR_FREE(str); + } + + ret = true; + +cleanup: + virshDomainFree(dom); + return ret; +} + const vshCmdDef domManagementCmds[] = { {.name = "attach-device", .handler = cmdAttachDevice, @@ -14653,5 +14700,11 @@ const vshCmdDef domManagementCmds[] = { .info = info_domblkthreshold, .flags = 0 }, + {.name = "guestinfo", + .handler = cmdGuestInfo, + .opts = opts_guestinfo, + .info = info_guestinfo, + .flags = 0 + }, {.name = NULL} }; -- 2.21.0
On Thu, Aug 01, 2019 at 08:37:03AM -0500, Jonathon Jongsma wrote:
This series adds several bits of guest information provided by a new API function virDomainGetGuestInfo(). There is an implementation for qemu using the guest agent. In particular, it adds information about logged-in users, guest OS, and timezone.
I had previously submitted a patch series with a dedicated API function for querying guest users that returned an array of structures describing the users. It was suggested that I convert his to a more futureproof design using typed parameters and also combine it with other bits of guest information similar to virDomainListGetStats(). In fact, there were several bugs that requested this information be added to the 'bulk stats' API, but Peter Krempa suggested adding a new API for all data queried from the guest agent (see https://bugzilla.redhat.com/show_bug.cgi?id=1705514). This is that API proposal.
The reason we suggested 'bulk stats' approach is so that we can retrieve information for all VMs in single call. This is not just about laziness on side of management app, it is much easier for libvirt. We can either fetch the info for all VMs one-by-one (which can take too long), or resort to massive threading. On the other hand it seems that libvirt with its async jobs can handle this in single thread. I am not libvirt expert so please correct me if I am making wrong assumptions here. Also, libvirt has pretty good knowledge whether the guest agent is running or not. From application side we cannot get this information reliably and we need to resort to trial-error approach.
It follows the stats API quite closely, and tries to return data in similar ways (for example, the "users.N.*" field name scheme was inspired by various stats fields).
I plan to follow this series up with a patch that adds fsinfo to the returned information, but wanted to get comments on this approach now.
Apart from the above I have two other concerns. With how the API call is designed you cannot pick which commands to run (you always get them all). With the number of included commands growing in the future the time to complete the call will grow as well and could just take too long. Also, if you run the call periodically you always don't want to run all the commands. For example, you don't need to check the os-info as often as list of logged in users. You cannot set the timeout on the guest agent commands. Instead you block on them. As described in bug [1], rogue guest can potentially block your call indefinitely. If you plan to address this problem in a more general manner later that's probably fine too. Tomas [1] https://bugzilla.redhat.com/show_bug.cgi?id=1705426 -- Tomáš Golembiovský <tgolembi@redhat.com>
On Wed, 2019-08-07 at 12:39 +0200, Tomáš Golembiovský wrote:
On Thu, Aug 01, 2019 at 08:37:03AM -0500, Jonathon Jongsma wrote:
This series adds several bits of guest information provided by a new API function virDomainGetGuestInfo(). There is an implementation for qemu using the guest agent. In particular, it adds information about logged-in users, guest OS, and timezone.
I had previously submitted a patch series with a dedicated API function for querying guest users that returned an array of structures describing the users. It was suggested that I convert his to a more futureproof design using typed parameters and also combine it with other bits of guest information similar to virDomainListGetStats(). In fact, there were several bugs that requested this information be added to the 'bulk stats' API, but Peter Krempa suggested adding a new API for all data queried from the guest agent (see https://bugzilla.redhat.com/show_bug.cgi?id=1705514). This is that API proposal.
The reason we suggested 'bulk stats' approach is so that we can retrieve information for all VMs in single call. This is not just about laziness on side of management app, it is much easier for libvirt. We can either fetch the info for all VMs one-by-one (which can take too long), or resort to massive threading. On the other hand it seems that libvirt with its async jobs can handle this in single thread. I am not libvirt expert so please correct me if I am making wrong assumptions here. Also, libvirt has pretty good knowledge whether the guest agent is running or not. From application side we cannot get this information reliably and we need to resort to trial-error approach.
It's not entirely clear to me what you are suggesting here. Are you saying that this API is generally OK, but that you wish it would query all domains at once? Or are you suggesting some additional changes?
It follows the stats API quite closely, and tries to return data in similar ways (for example, the "users.N.*" field name scheme was inspired by various stats fields).
I plan to follow this series up with a patch that adds fsinfo to the returned information, but wanted to get comments on this approach now.
Apart from the above I have two other concerns.
With how the API call is designed you cannot pick which commands to run (you always get them all). With the number of included commands growing in the future the time to complete the call will grow as well and could just take too long. Also, if you run the call periodically you always don't want to run all the commands. For example, you don't need to check the os-info as often as list of logged in users.
If I understand what you're saying, I think it's incorrect. The API that I proposed takes a 'types' parameter (similar to the 'stats' parameter in the bulk stats API) which lets you specify which guest information types you would like to query. So if you want, you can query only a subset of the guest info categories.
You cannot set the timeout on the guest agent commands. Instead you block on them. As described in bug [1], rogue guest can potentially block your call indefinitely. If you plan to address this problem in a more general manner later that's probably fine too.
Yes, I think this issue is probably better addressed separately.
Tomas
On Wed, Aug 07, 2019 at 10:41:10AM -0500, Jonathon Jongsma wrote:
On Wed, 2019-08-07 at 12:39 +0200, Tomáš Golembiovský wrote:
On Thu, Aug 01, 2019 at 08:37:03AM -0500, Jonathon Jongsma wrote:
This series adds several bits of guest information provided by a new API function virDomainGetGuestInfo(). There is an implementation for qemu using the guest agent. In particular, it adds information about logged-in users, guest OS, and timezone.
I had previously submitted a patch series with a dedicated API function for querying guest users that returned an array of structures describing the users. It was suggested that I convert his to a more futureproof design using typed parameters and also combine it with other bits of guest information similar to virDomainListGetStats(). In fact, there were several bugs that requested this information be added to the 'bulk stats' API, but Peter Krempa suggested adding a new API for all data queried from the guest agent (see https://bugzilla.redhat.com/show_bug.cgi?id=1705514). This is that API proposal.
The reason we suggested 'bulk stats' approach is so that we can retrieve information for all VMs in single call. This is not just about laziness on side of management app, it is much easier for libvirt. We can either fetch the info for all VMs one-by-one (which can take too long), or resort to massive threading. On the other hand it seems that libvirt with its async jobs can handle this in single thread. I am not libvirt expert so please correct me if I am making wrong assumptions here. Also, libvirt has pretty good knowledge whether the guest agent is running or not. From application side we cannot get this information reliably and we need to resort to trial-error approach.
It's not entirely clear to me what you are suggesting here. Are you saying that this API is generally OK, but that you wish it would query all domains at once? Or are you suggesting some additional changes?
What I am saying is that being able to query single VM is nice, but it's just a start because we need to be able to query all the (running) VMs at once. And having libvirt API for that would be much more benefitial (for reasons specified) than querying VMs one-by-one in management apps. Of course it's pretty fine to have separate (external) API calls for single VM/list of VMs/all VMs, but how it behaves "under the hood" is what needs to be considered (is single VM special case of all VMs or is it the other way around?).
It follows the stats API quite closely, and tries to return data in similar ways (for example, the "users.N.*" field name scheme was inspired by various stats fields).
I plan to follow this series up with a patch that adds fsinfo to the returned information, but wanted to get comments on this approach now.
Apart from the above I have two other concerns.
With how the API call is designed you cannot pick which commands to run (you always get them all). With the number of included commands growing in the future the time to complete the call will grow as well and could just take too long. Also, if you run the call periodically you always don't want to run all the commands. For example, you don't need to check the os-info as often as list of logged in users.
If I understand what you're saying, I think it's incorrect. The API that I proposed takes a 'types' parameter (similar to the 'stats' parameter in the bulk stats API) which lets you specify which guest information types you would like to query. So if you want, you can query only a subset of the guest info categories.
You are right, please ignore this comment.
You cannot set the timeout on the guest agent commands. Instead you block on them. As described in bug [1], rogue guest can potentially block your call indefinitely. If you plan to address this problem in a more general manner later that's probably fine too.
Yes, I think this issue is probably better addressed separately.
OK. Tomas
Tomas
-- Tomáš Golembiovský <tgolembi@redhat.com>
On Wed, Aug 07, 2019 at 12:39:09 +0200, Tomáš Golembiovský wrote:
On Thu, Aug 01, 2019 at 08:37:03AM -0500, Jonathon Jongsma wrote:
This series adds several bits of guest information provided by a new API function virDomainGetGuestInfo(). There is an implementation for qemu using the guest agent. In particular, it adds information about logged-in users, guest OS, and timezone.
I had previously submitted a patch series with a dedicated API function for querying guest users that returned an array of structures describing the users. It was suggested that I convert his to a more futureproof design using typed parameters and also combine it with other bits of guest information similar to virDomainListGetStats(). In fact, there were several bugs that requested this information be added to the 'bulk stats' API, but Peter Krempa suggested adding a new API for all data queried from the guest agent (see https://bugzilla.redhat.com/show_bug.cgi?id=1705514). This is that API proposal.
The reason we suggested 'bulk stats' approach is so that we can retrieve information for all VMs in single call. This is not just about laziness on side of management app, it is much easier for libvirt. We can either
I don't want to crush your dreams, but for the VM bulk stats API theres nothing fancy that libvirt does here. We just fetch a list of all VMs and then sequentially in a loop fill in all the stats.
fetch the info for all VMs one-by-one (which can take too long), or resort to massive threading. On the other hand it seems that libvirt
Actually in some cases this might bring better results in cases when one VM is not responding.
with its async jobs can handle this in single thread. I am not libvirt
Yes it's a single thread and a loop.
expert so please correct me if I am making wrong assumptions here. Also, libvirt has pretty good knowledge whether the guest agent is running or not. From application side we cannot get this information reliably and we need to resort to trial-error approach.
Well, we know whether the guest agent channel socket is connected from guest's side, but we also expose this information in the XML file and there's also an event which is fired always when the state of the guest agent changes. This is possible via notifications from qemu via the virtio channel.
It follows the stats API quite closely, and tries to return data in similar ways (for example, the "users.N.*" field name scheme was inspired by various stats fields).
I plan to follow this series up with a patch that adds fsinfo to the returned information, but wanted to get comments on this approach now.
Apart from the above I have two other concerns.
With how the API call is designed you cannot pick which commands to run (you always get them all). With the number of included commands growing in the future the time to complete the call will grow as well and could just take too long. Also, if you run the call periodically you always don't want to run all the commands. For example, you don't need to check the os-info as often as list of logged in users.
This was clarified in a different reply.
You cannot set the timeout on the guest agent commands. Instead you block on them. As described in bug [1], rogue guest can potentially block your call indefinitely. If you plan to address this problem in a more general manner later that's probably fine too.
Libvirt's guest-agent APIs actually have some built-in timeout so that a rouge agent can't block forever, but this timeout is not really configurable. A good point would be probably to have an argument for the API to be able to use an arbitrary timeout for the calls. In general I'm not really sure that there's benefit in having the API return the stats for all guest OSes rather than just one. In my design for the VM stats API I think I went a bit too far in this case and the benefits of returning stats for all VMs compared to returning all data for a single VM are worth that. What was worth in that implementation was to be able to query multiple types of stats at once. I'm willing to discuss this further though.
participants (3)
-
Jonathon Jongsma -
Peter Krempa -
Tomáš Golembiovský