On 8/23/19 6:31 PM, Jonathon Jongsma wrote:

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 | 14 ++++ src/driver-hypervisor.h | 8 +++ src/libvirt-domain.c | 117 +++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 140 insertions(+)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 2fe9bb8e91..ad68db7549 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c

+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);

I think that @types should also be written in hex format since it's a bitwise-OR of integer values.

+ + 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;

Unfortunatelly, 5.6.0 is gone. This needs to be a new section of name LIBVIRT_5.7.0. Michal

On 8/23/19 6:31 PM, Jonathon Jongsma wrote:

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 1bd281dd6d..665d938a99 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -7650,3 +7650,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);

We prefer remoteGetHypervisorConn() ...

+ virDomainPtr dom = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));

.. which also reports an error. So no need to invent a new one.

+ 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;

1: here

+ + 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);

You need to free @params here.

+ + return rv; +} diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index daac506672..5ba144648a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8306,6 +8306,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; + }

No need to check for this here. It's checked for in virTypedParamsDeserialize(). That's why you need to pass _MAX argument. In fact, the check should go to [1].

+ + if (params) {

The @params can't be NULL here, can they? I mean, in the public API you check for: virCheckNonNullArgGoto(params, error); But I agree that our RPC infrastructure is complicated and it's not easy to understand it at the first glance. Basically, it works like this: at client side on virConnectOpen() the conn->driver is initialized to remote driver (@hypervisor_driver from remote_driver.c). So calling any public API ends up calling the callback from remote_driver.c (remoteDomainGetGuestInfo() in this case). That is the reason why these callbacks in remote driver need to look exactly like the public APIs (and hence you need to create typedef for every public API). In the end, these callbacks do mere arg serialization and reply deserialization. On server, things are a bit different. We have a code which deserializes incoming RPC packet and then we have this huge array of pairs PROC_NR + dispatch callback (look at the end of src/remote/remote_daemon_dispatch_stubs.h you'll find the array there). Yeah, yeah, it's not list of pairs, rather than tuples, but for our case we can safely ignore the other members of the tuple. So, after server has decoded the incoming RPC it learned what procedure the client wants to call and thus it calls the associated callback (remoteDispatchDomainGetGuestInfoHelper() in this case). And this callback will then call the public API again, but this time conn->driver points to actual driver (qemu driver from instance). It is only here where all the interesting work is done. It's more or less documented here: https://libvirt.org/api.html https://libvirt.org/internals/rpc.html https://libvirt.org/api_extension.html (looks like you've followed this one) Please let me know if you want me to expand on something.

+ 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. @@ -8733,6 +8785,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 */

Ooops, s/6/7/ :D Michal

On 8/23/19 1:31 PM, Jonathon Jongsma wrote:

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>

Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com> And, assuming that what I mentioned in my comment below is a matter of style rather than a syntax sin: Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

--- src/qemu/qemu_agent.c | 91 +++++++++++++++++++++++ src/qemu/qemu_agent.h | 2 + tests/qemuagenttest.c | 168 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 261 insertions(+)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 361db299a5..963a4b9359 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2240,3 +2240,94 @@ 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, + "user.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, "user.%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, + "user.%zu.domain", i); + if (virTypedParamsAddString(params, nparams, maxparams, + param_name, strvalue) < 0) + goto cleanup; + } + + double logintime;

I'd rather declare 'logintime' at the start of the 'for' loop, together with 'param_name' and 'entry'. Since this patch passed 'make syntax-check' I believe this is more a matter of "artistic freedom" than a rule, so I guess it's ok.

+ 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, + "user.%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..f2936a59f0 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, + "user.%zu.name", nth); + if (username && + virTypedParamsGetString(params, nparams, param_name, username) < 0) + return -1; + + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "user.%zu.domain", nth); + if (domain && + virTypedParamsGetString(params, nparams, param_name, domain) < 0) + return -1; + + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "user.%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, "user.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, "%s", "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, "user.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 */

On 8/23/19 1:31 PM, Jonathon Jongsma wrote:

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> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>

src/qemu/qemu_agent.c | 53 ++++++++++++++++++ src/qemu/qemu_agent.h | 1 + tests/qemuagenttest.c | 122 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 176 insertions(+)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 963a4b9359..0fcc3fdabd 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2331,3 +2331,56 @@ qemuAgentGetUsers(qemuAgentPtr mon, virJSONValueFree(reply); 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 f2936a59f0..489b77d403 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 */

On 8/23/19 6:31 PM, Jonathon Jongsma wrote:

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 0fcc3fdabd..e986204162 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;

According to qemu-ga qapi schema (qga/qapi-schema.json) the "zone" is optional (denoted by asterisk before the attribute name).

+ + 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);

Again, looong line.

diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 489b77d403..0912a5f29a 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; \

These gotos look a bit misindented :-) Michal

On 8/23/19 1:31 PM, Jonathon Jongsma wrote:

Since version 3.0, qemu has returned disk usage statistics in guest-get-fsinfo. And since 3.1, it has returned information about the disk serial number and device node of disks that are targeted by the filesystem.

Unfortunately, the public API virDomainGetFSInfo() returns the filesystem info using a virDomainFSInfo struct, and due to API/ABI guaranteeds it cannot be extended. So this new information cannot

s/guaranteeds/guarantees

easily be added to the public API. However, it is possible to add this new filesystem information to a new virDomainGetGuestInfo() API which will be based on typed parameters and is thus more extensible.

In order to support these two use cases, I added an internal struct which the agent code uses to return all of the new data fields. This internal struct can be converted to the public struct at a cost of some extra memory allocation.

In a following commit, this additional information will be used within virDomainGetGuestInfo().

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> ---

Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com> Got a few style questions below. Assuming it's ok or the commiter can fix those before pushing, Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

src/qemu/qemu_agent.c | 203 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 182 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index e986204162..8d54979f89 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1827,19 +1827,144 @@ qemuAgentSetTime(qemuAgentPtr mon, return ret; }

+typedef struct _qemuAgentDiskInfo qemuAgentDiskInfo; +typedef qemuAgentDiskInfo *qemuAgentDiskInfoPtr; +struct _qemuAgentDiskInfo { + char *alias; + char *serial; + char *devnode; +}; + +typedef struct _qemuAgentFSInfo qemuAgentFSInfo; +typedef qemuAgentFSInfo *qemuAgentFSInfoPtr; +struct _qemuAgentFSInfo { + char *mountpoint; /* path to mount point */ + char *name; /* device name in the guest (e.g. "sda1") */ + char *fstype; /* filesystem type */ + long long total_bytes; + long long used_bytes; + size_t ndisks; + qemuAgentDiskInfoPtr *disks; +}; + +static void +qemuAgentDiskInfoFree(qemuAgentDiskInfoPtr info) +{ + if (!info) + return; + + VIR_FREE(info->serial); + VIR_FREE(info->alias); + VIR_FREE(info->devnode); + VIR_FREE(info); +} + +static void +qemuAgentFSInfoFree(qemuAgentFSInfoPtr info) +{ + size_t i; + + if (!info) + return; + + VIR_FREE(info->mountpoint); + VIR_FREE(info->name); + VIR_FREE(info->fstype); + + for (i = 0; i < info->ndisks; i++) + qemuAgentDiskInfoFree(info->disks[i]); + VIR_FREE(info->disks); + + VIR_FREE(info); +} + +static virDomainFSInfoPtr +qemuAgentFSInfoToPublic(qemuAgentFSInfoPtr agent) +{ + virDomainFSInfoPtr ret = NULL; + size_t n; + + if (VIR_ALLOC(ret) < 0) + return NULL;

+ if (VIR_STRDUP(ret->name, agent->name) < 0) + goto error; + if (VIR_STRDUP(ret->mountpoint, agent->mountpoint) < 0) + goto error; + if (VIR_STRDUP(ret->fstype, agent->fstype) < 0) + goto error; + + ret->ndevAlias = agent->ndisks; + + if (ret->ndevAlias == 0) + return ret; + + if (VIR_ALLOC_N(ret->devAlias, ret->ndevAlias) < 0) + goto error; + + for (n = 0; n < ret->ndevAlias; n++) { + if (VIR_STRDUP(ret->devAlias[n], agent->disks[n]->alias) < 0) + goto error; + } + + return ret; + + error: + virDomainFSInfoFree(ret); + return NULL; +} + +static int +qemuAgentGetFSInfoInternal(qemuAgentPtr mon, qemuAgentFSInfoPtr **info, + virDomainDefPtr vmdef); int qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info, virDomainDefPtr vmdef) +{ + int ret = -1; + qemuAgentFSInfoPtr *agentinfo = NULL; + virDomainFSInfoPtr *info_ret = NULL; + size_t i; + int nfs; + + nfs = qemuAgentGetFSInfoInternal(mon, &agentinfo, vmdef); + if (nfs < 0) + return ret; + if (VIR_ALLOC_N(info_ret, nfs) < 0) + goto cleanup; + + for (i = 0; i < nfs; i++) { + if (!(info_ret[i] = qemuAgentFSInfoToPublic(agentinfo[i]))) + goto cleanup; + } + + *info = info_ret; + info_ret = NULL; + ret = nfs; + + cleanup: + for (i = 0; i < nfs; i++) { + qemuAgentFSInfoFree(agentinfo[i]); + /* if there was an error, free any memory we've allocated for the + * return value */ + if (info_ret) + virDomainFSInfoFree(info_ret[i]); + } + return ret; +} + + +static int +qemuAgentGetFSInfoInternal(qemuAgentPtr mon, qemuAgentFSInfoPtr **info, + virDomainDefPtr vmdef) { size_t i, j, k; int ret = -1; - size_t ndata = 0, ndisk; - char **alias; + size_t ndata = 0; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr data; - virDomainFSInfoPtr *info_ret = NULL; + qemuAgentFSInfoPtr *info_ret = NULL; virPCIDeviceAddress pci_address; const char *result = NULL;

@@ -1915,6 +2040,33 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info, if (VIR_STRDUP(info_ret[i]->fstype, result) < 0) goto cleanup;

+ + /* 'used-bytes' and 'total-bytes' were added in qemu-ga 3.0 */ + unsigned long long bytes_val;

Two nits here: - extra blank line here results in two consecutive blank lines in the middle of the function - there's the same 'artistic freedom' question I made in a previous patch with declaring 'bytes_val' here, instead of the start of the loop together with 'entry'.

+ if (virJSONValueObjectHasKey(entry, "used-bytes")) { + if (virJSONValueObjectGetNumberUlong(entry, "used-bytes", + &bytes_val) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Error getting 'used-bytes' in reply of guest-get-fsinfo")); + goto cleanup; + } + info_ret[i]->used_bytes = bytes_val; + } else { + info_ret[i]->used_bytes = -1; + } + + if (virJSONValueObjectHasKey(entry, "total-bytes")) { + if (virJSONValueObjectGetNumberUlong(entry, "total-bytes", + &bytes_val) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Error getting 'total-bytes' in reply of guest-get-fsinfo")); + goto cleanup; + } + info_ret[i]->total_bytes = bytes_val; + } else { + info_ret[i]->total_bytes = -1; + } + if (!(entry = virJSONValueObjectGet(entry, "disk"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'disk' missing in reply of guest-get-fsinfo")); @@ -1927,31 +2079,45 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info, goto cleanup; }

- ndisk = virJSONValueArraySize(entry); - if (ndisk == 0) + info_ret[i]->ndisks = virJSONValueArraySize(entry); + if (info_ret[i]->ndisks == 0) continue; - if (VIR_ALLOC_N(info_ret[i]->devAlias, ndisk) < 0) + if (VIR_ALLOC_N(info_ret[i]->disks, info_ret[i]->ndisks) < 0) goto cleanup;

- alias = info_ret[i]->devAlias; - info_ret[i]->ndevAlias = 0; - for (j = 0; j < ndisk; j++) { - virJSONValuePtr disk = virJSONValueArrayGet(entry, j); + for (j = 0; j < info_ret[i]->ndisks; j++) { + virJSONValuePtr jsondisk = virJSONValueArrayGet(entry, j); virJSONValuePtr pci; int diskaddr[3], pciaddr[4]; const char *diskaddr_comp[] = {"bus", "target", "unit"}; const char *pciaddr_comp[] = {"domain", "bus", "slot", "function"}; + const char *val; virDomainDiskDefPtr diskDef;

- if (!disk) { + if (VIR_ALLOC(info_ret[i]->disks[j]) < 0) + goto cleanup; + + qemuAgentDiskInfoPtr disk = info_ret[i]->disks[j];

'artistic freedom' question here too.

+ + if (!jsondisk) { virReportError(VIR_ERR_INTERNAL_ERROR, _("array element '%zd' of '%zd' missing in " "guest-get-fsinfo 'disk' data"), - j, ndisk); + j, info_ret[i]->ndisks); goto cleanup; }

- if (!(pci = virJSONValueObjectGet(disk, "pci-controller"))) { + if ((val = virJSONValueObjectGetString(jsondisk, "serial"))) { + if (VIR_STRDUP(disk->serial, val) < 0) + goto cleanup; + } + + if ((val = virJSONValueObjectGetString(jsondisk, "dev"))) { + if (VIR_STRDUP(disk->devnode, val) < 0) + goto cleanup; + } + + if (!(pci = virJSONValueObjectGet(jsondisk, "pci-controller"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'pci-controller' missing in guest-get-fsinfo " "'disk' data")); @@ -1960,7 +2126,7 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,

for (k = 0; k < 3; k++) { if (virJSONValueObjectGetNumberInt( - disk, diskaddr_comp[k], &diskaddr[k]) < 0) { + jsondisk, diskaddr_comp[k], &diskaddr[k]) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("'%s' missing in guest-get-fsinfo " "'disk' data"), diskaddr_comp[k]); @@ -1986,13 +2152,8 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info, diskaddr[0], diskaddr[1], diskaddr[2]))) continue;

- if (VIR_STRDUP(*alias, diskDef->dst) < 0) + if (VIR_STRDUP(disk->alias, diskDef->dst) < 0) goto cleanup; - - if (*alias) { - alias++; - info_ret[i]->ndevAlias++; - } } }

@@ -2003,7 +2164,7 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info, cleanup: if (info_ret) { for (i = 0; i < ndata; i++) - virDomainFSInfoFree(info_ret[i]); + qemuAgentFSInfoFree(info_ret[i]); VIR_FREE(info_ret); } virJSONValueFree(cmd);

On 8/23/19 1:31 PM, Jonathon Jongsma wrote:

This function adds the complete filesystem information returned by the qemu agent to an array of typed parameters with field names intended to to be returned by virDomainGetGuestInfo()

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com> ---

Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com> A few comments below:

src/qemu/qemu_agent.c | 90 ++++++++++++++++++ src/qemu/qemu_agent.h | 5 + tests/qemuagenttest.c | 210 +++++++++++++++++++++++++++++++++++++++--- 3 files changed, 292 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 8d54979f89..169ad74f6f 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1953,6 +1953,96 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info, return ret; }

+int +qemuAgentGetFSInfoParams(qemuAgentPtr mon, + virTypedParameterPtr *params, + int *nparams, int *maxparams, + virDomainDefPtr vmdef) +{ + int ret = -1; + qemuAgentFSInfoPtr *fsinfo = NULL; + size_t i, j; + int nfs; + + if ((nfs = qemuAgentGetFSInfoInternal(mon, &fsinfo, vmdef)) < 0) + return -1; + + if (virTypedParamsAddUInt(params, nparams, maxparams, + "fs.count", nfs) < 0) + goto cleanup; + + for (i = 0; i < nfs; i++) { + char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "fs.%zu.name", i); + if (virTypedParamsAddString(params, nparams, maxparams, + param_name, fsinfo[i]->name) < 0) + goto cleanup; + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "fs.%zu.mountpoint", i); + if (virTypedParamsAddString(params, nparams, maxparams, + param_name, fsinfo[i]->mountpoint) < 0) + goto cleanup; + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "fs.%zu.fstype", i); + if (virTypedParamsAddString(params, nparams, maxparams, + param_name, fsinfo[i]->fstype) < 0) + goto cleanup; + + /* disk usage values are not returned by older guest agents, so + * only add the params if the value is set */ + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "fs.%zu.total-bytes", i); + if (fsinfo[i]->total_bytes != -1 && + virTypedParamsAddULLong(params, nparams, maxparams, + param_name, fsinfo[i]->total_bytes) < 0) + goto cleanup; + + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "fs.%zu.used-bytes", i); + if (fsinfo[i]->used_bytes != -1 && + virTypedParamsAddULLong(params, nparams, maxparams, + param_name, fsinfo[i]->used_bytes) < 0) + goto cleanup; + + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "fs.%zu.disk.count", i); + if (virTypedParamsAddUInt(params, nparams, maxparams, + param_name, fsinfo[i]->ndisks) < 0) + goto cleanup; + for (j = 0; j < fsinfo[i]->ndisks; j++) { + qemuAgentDiskInfoPtr d = fsinfo[i]->disks[j]; + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "fs.%zu.disk.%zu.alias", i, j); + if (d->alias && + virTypedParamsAddString(params, nparams, maxparams, + param_name, d->alias) < 0) + goto cleanup; + + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "fs.%zu.disk.%zu.serial", i, j); + if (d->serial && + virTypedParamsAddString(params, nparams, maxparams, + param_name, d->serial) < 0) + goto cleanup; + + snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, + "fs.%zu.disk.%zu.device", i, j); + if (d->devnode && + virTypedParamsAddString(params, nparams, maxparams, + param_name, d->devnode) < 0) + goto cleanup; + } + } + ret = nfs; + + cleanup: + for (i = 0; i < nfs; i++) + qemuAgentFSInfoFree(fsinfo[i]); + VIR_FREE(fsinfo); + + return ret; +}

static int qemuAgentGetFSInfoInternal(qemuAgentPtr mon, qemuAgentFSInfoPtr **info, diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 69b0176855..f6d74a2603 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -74,6 +74,11 @@ int qemuAgentFSThaw(qemuAgentPtr mon); int qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info, virDomainDefPtr vmdef);

+int qemuAgentGetFSInfoParams(qemuAgentPtr mon, + virTypedParameterPtr *params, + int *nparams, int *maxparams, + virDomainDefPtr vmdef); + int qemuAgentSuspend(qemuAgentPtr mon, unsigned int target);

diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 0912a5f29a..105b32017a 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -168,38 +168,45 @@ testQemuAgentFSTrim(const void *data)

static int -testQemuAgentGetFSInfo(const void *data) +testQemuAgentGetFSInfoCommon(virDomainXMLOptionPtr xmlopt, + qemuMonitorTestPtr *test, + virDomainDefPtr *def) { - virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; - qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt); + int ret = -1; char *domain_filename = NULL; - virDomainDefPtr def = NULL; - virDomainFSInfoPtr *info = NULL; - int ret = -1, ninfo = 0, i; + qemuMonitorTestPtr ret_test = NULL; + virDomainDefPtr ret_def = NULL;

- if (!test) + if (!test || !def) + return -1; + + if (!(ret_test = qemuMonitorTestNewAgent(xmlopt))) return -1;

if (virAsprintf(&domain_filename, "%s/qemuagentdata/fsinfo.xml", abs_srcdir) < 0) goto cleanup;

- if (!(def = virDomainDefParseFile(domain_filename, driver.caps, xmlopt, - NULL, VIR_DOMAIN_DEF_PARSE_INACTIVE))) + if (!(ret_def = virDomainDefParseFile(domain_filename, driver.caps, xmlopt, + NULL, VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto cleanup;

- if (qemuMonitorTestAddAgentSyncResponse(test) < 0) + if (qemuMonitorTestAddAgentSyncResponse(ret_test) < 0) goto cleanup;

- if (qemuMonitorTestAddItem(test, "guest-get-fsinfo", + if (qemuMonitorTestAddItem(ret_test, "guest-get-fsinfo", "{\"return\": [" " {\"name\": \"sda1\", \"mountpoint\": \"/\"," + " \"total-bytes\":952840192," + " \"used-bytes\":229019648," " \"disk\": [" - " {\"bus-type\": \"ide\"," + " {\"serial\": \"ARBITRARYSTRING\"," + " \"bus-type\": \"ide\"," " \"bus\": 1, \"unit\": 0," " \"pci-controller\": {" " \"bus\": 0, \"slot\": 1," " \"domain\": 0, \"function\": 1}," + " \"dev\": \"/dev/sda1\"," " \"target\": 0}]," " \"type\": \"ext4\"}," " {\"name\": \"dm-1\"," @@ -221,6 +228,32 @@ testQemuAgentGetFSInfo(const void *data) " {\"name\": \"sdb1\"," " \"mountpoint\": \"/mnt/disk\"," " \"disk\": [], \"type\": \"xfs\"}]}") < 0) + goto cleanup; + *test = ret_test; + ret_test = NULL; + *def = ret_def; + ret_def = NULL; + ret = 0; + + cleanup: + VIR_FREE(domain_filename); + if (ret_test) + qemuMonitorTestFree(ret_test); + virDomainDefFree(ret_def); + + return ret; +} + +static int +testQemuAgentGetFSInfo(const void *data) +{ + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; + qemuMonitorTestPtr test = NULL; + virDomainDefPtr def = NULL; + virDomainFSInfoPtr *info = NULL; + int ret = -1, ninfo = 0, i; + + if (testQemuAgentGetFSInfoCommon(xmlopt, &test, &def) < 0) goto cleanup;

if ((ninfo = qemuAgentGetFSInfo(qemuMonitorTestGetAgent(test), @@ -295,7 +328,157 @@ testQemuAgentGetFSInfo(const void *data) for (i = 0; i < ninfo; i++) virDomainFSInfoFree(info[i]); VIR_FREE(info); - VIR_FREE(domain_filename); + virDomainDefFree(def); + qemuMonitorTestFree(test); + return ret; +} + +static int +testQemuAgentGetFSInfoParams(const void *data) +{ + virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; + qemuMonitorTestPtr test = NULL; + virDomainDefPtr def = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0, maxparams = 0; + int ret = -1; + unsigned int count; + + if (testQemuAgentGetFSInfoCommon(xmlopt, &test, &def) < 0) + goto cleanup; + + if (qemuAgentGetFSInfoParams(qemuMonitorTestGetAgent(test), + ¶ms, &nparams, &maxparams, def) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "Failed to execute qemuAgentGetFSInfoParams()"); + goto cleanup; + } + + if (virTypedParamsGetUInt(params, nparams, "fs.count", &count) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "expected filesystem count"); + goto cleanup; + } + + if (count != 3) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "expected 3 filesystems information, got %d", count); + ret = -1;

You already initialized 'ret' with '-1' up there. Over here (and in the other instances in this function) you don't need to set it again.

+ goto cleanup; + } + const char *name, *mountpoint, *fstype, *alias, *serial; + unsigned int diskcount; + unsigned long long bytesused, bytestotal;

Even considering that variable declaration in the middle of the function body is okay, a line break before and after the declarations can enhance code readability IMO.

+ if (virTypedParamsGetString(params, nparams, "fs.2.name", &name) < 0 || + virTypedParamsGetString(params, nparams, "fs.2.mountpoint", &mountpoint) < 0 || + virTypedParamsGetString(params, nparams, "fs.2.fstype", &fstype) < 0 || + virTypedParamsGetULLong(params, nparams, "fs.2.used-bytes", &bytesused) <= 0 || + virTypedParamsGetULLong(params, nparams, "fs.2.total-bytes", &bytestotal) <= 0 || + virTypedParamsGetUInt(params, nparams, "fs.2.disk.count", &diskcount) < 0 || + virTypedParamsGetString(params, nparams, "fs.2.disk.0.alias", &alias) < 0 || + virTypedParamsGetString(params, nparams, "fs.2.disk.0.serial", &serial) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Missing an expected parameter for sda1 (%s,%s)", + name, alias); + ret = -1; + goto cleanup; + } + if ( + STRNEQ(name, "sda1") || + STRNEQ(mountpoint, "/") || + STRNEQ(fstype, "ext4") || + bytesused != 229019648 || + bytestotal != 952840192 || + diskcount != 1 || + STRNEQ(alias, "hdc") || + STRNEQ(serial, "ARBITRARYSTRING")) + { + virReportError(VIR_ERR_INTERNAL_ERROR, + "unexpected filesystems information returned for sda1 (%s,%s)", + name, alias); + ret = -1; + goto cleanup; + } + + const char *alias2;

Same comment as above.

+ if (virTypedParamsGetString(params, nparams, "fs.1.name", &name) < 0 || + virTypedParamsGetString(params, nparams, "fs.1.mountpoint", &mountpoint) < 0 || + virTypedParamsGetString(params, nparams, "fs.1.fstype", &fstype) < 0 || + virTypedParamsGetULLong(params, nparams, "fs.1.used-bytes", &bytesused) == 1 || + virTypedParamsGetULLong(params, nparams, "fs.1.total-bytes", &bytestotal) == 1 || + virTypedParamsGetUInt(params, nparams, "fs.1.disk.count", &diskcount) < 0 || + virTypedParamsGetString(params, nparams, "fs.1.disk.0.alias", &alias) < 0 || + virTypedParamsGetString(params, nparams, "fs.1.disk.1.alias", &alias2) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Incorrect parameters for dm-1 (%s,%s)", + name, alias); + ret = -1; + goto cleanup; + } + if (STRNEQ(name, "dm-1") || + STRNEQ(mountpoint, "/opt") || + STRNEQ(fstype, "vfat") || + diskcount != 2 || + STRNEQ(alias, "vda") || + STRNEQ(alias2, "vdb")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "unexpected filesystems information returned for dm-1 (%s,%s)", + name, alias); + ret = -1; + goto cleanup; + } + + alias = NULL; + if (virTypedParamsGetString(params, nparams, "fs.0.name", &name) < 0 || + virTypedParamsGetString(params, nparams, "fs.0.mountpoint", &mountpoint) < 0 || + virTypedParamsGetString(params, nparams, "fs.0.fstype", &fstype) < 0 || + virTypedParamsGetULLong(params, nparams, "fs.0.used-bytes", &bytesused) == 1 || + virTypedParamsGetULLong(params, nparams, "fs.0.total-bytes", &bytestotal) == 1 || + virTypedParamsGetUInt(params, nparams, "fs.0.disk.count", &diskcount) < 0 || + virTypedParamsGetString(params, nparams, "fs.0.disk.0.alias", &alias) == 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Incorrect parameters for sdb1 (%s,%s)", + name, alias); + ret = -1; + goto cleanup; + } + if (STRNEQ(name, "sdb1") || + STRNEQ(mountpoint, "/mnt/disk") || + STRNEQ(fstype, "xfs") || + diskcount != 0 || + alias != NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "unexpected filesystems information returned for sdb1 (%s,%s)", + name, alias); + ret = -1; + goto cleanup; + } + + if (qemuMonitorTestAddAgentSyncResponse(test) < 0) + goto cleanup; + + if (qemuMonitorTestAddItem(test, "guest-get-fsinfo", + "{\"error\":" + " {\"class\":\"CommandDisabled\"," + " \"desc\":\"The command guest-get-fsinfo " + "has been disabled for " + "this instance\"," + " \"data\":{\"name\":\"guest-get-fsinfo\"}" + " }" + "}") < 0) + goto cleanup; + + if (qemuAgentGetFSInfoParams(qemuMonitorTestGetAgent(test), ¶ms, + &nparams, &maxparams, def) != -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "agent get-fsinfo command should have failed"); + goto cleanup; + } + + ret = 0; + + cleanup: + virTypedParamsFree(params, nparams); virDomainDefFree(def); qemuMonitorTestFree(test); return ret; @@ -1288,6 +1471,7 @@ mymain(void) DO_TEST(FSFreeze); DO_TEST(FSThaw); DO_TEST(FSTrim); + DO_TEST(GetFSInfoParams); DO_TEST(GetFSInfo); DO_TEST(Suspend); DO_TEST(Shutdown);

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 */ };

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

On 8/23/19 6:31 PM, Jonathon Jongsma wrote:

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 | 85 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+)

Almost :-) This is missing documentation (virsh.pod change). And since I'm too lazy to write one I'll leave that up to you O:-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ccda71d7e0..977783951d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -14038,6 +14038,85 @@ 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 = "user", + .type = VSH_OT_BOOL, + .help = N_("report active users"), + }, + {.name = "os", + .type = VSH_OT_BOOL, + .help = N_("report operating system information"), + }, + {.name = "timezone", + .type = VSH_OT_BOOL, + .help = N_("report timezone information"), + }, + {.name = "hostname", + .type = VSH_OT_BOOL, + .help = N_("report hostname"), + }, + {.name = "filesystem", + .type = VSH_OT_BOOL, + .help = N_("report filesystem information"), + }, + {.name = NULL} +}; + +static bool +cmdGuestInfo(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + bool ret = false; + virTypedParameterPtr params = NULL; + int nparams = 0; + size_t i; + unsigned int types = 0; + + if (vshCommandOptBool(cmd, "user")) + types |= VIR_DOMAIN_GUEST_INFO_USERS; + if (vshCommandOptBool(cmd, "os")) + types |= VIR_DOMAIN_GUEST_INFO_OS; + if (vshCommandOptBool(cmd, "timezone")) + types |= VIR_DOMAIN_GUEST_INFO_TIMEZONE; + if (vshCommandOptBool(cmd, "hostname")) + types |= VIR_DOMAIN_GUEST_INFO_HOSTNAME; + if (vshCommandOptBool(cmd, "filesystem")) + types |= VIR_DOMAIN_GUEST_INFO_FILESYSTEM; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (virDomainGetGuestInfo(dom, types, ¶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:

Don't forget to free @params: virTypedParamsFree(params, nparams);

+ virshDomainFree(dom); + return ret; +} + const vshCmdDef domManagementCmds[] = { {.name = "attach-device", .handler = cmdAttachDevice, @@ -14653,5 +14732,11 @@ const vshCmdDef domManagementCmds[] = { .info = info_domblkthreshold, .flags = 0 }, + {.name = "guestinfo", + .handler = cmdGuestInfo, + .opts = opts_guestinfo, + .info = info_guestinfo, + .flags = 0 + }, {.name = NULL} };

Michal