[libvirt] [PATCH v2 0/9] 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, timezone, hostname, and filesystem info. This is the second version of this patch series. Various things were improved, but the biggest difference is that this version includes filesystem information. Filesystem information is already accessible via the public API virDomainGetFSInfo(), but recent versions of the qemu guest agent added additional information, and the existing public API is not able to be extended without breaking API since it returns a C struct. This new API uses typed parameters so that it can be extended as necessary when new fields are added. The overall API follows the bulk 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). It should be noted that like version 1 of this patch series, the API still only operates on a single domain, not a list of domains. I'm willing to consider changing the API to operate on a list of domains if that is the concensus, but I lean toward keeping it simpler. Here's an example of the output of the virsh command added in this patch operating on a fedora 30 guest: virsh # guestinfo fedora30 user.count : 1 user.0.name : test user.0.login-time : 1566422940895 os.id : fedora os.name : Fedora os.pretty-name : Fedora 30 (Workstation Edition) os.version : 30 (Workstation Edition) os.version-id : 30 os.machine : x86_64 os.variant : Workstation Edition os.variant-id : workstation os.kernel-release : 5.2.7-200.fc30.x86_64 os.kernel-version : #1 SMP Thu Aug 8 05:35:29 UTC 2019 timezone.name : CDT timezone.offset : -18000 hostname : myhostname fs.count : 3 fs.0.name : dm-0 fs.0.mountpoint : / fs.0.fstype : ext4 fs.0.total-bytes : 25928306688 fs.0.used-bytes : 10762133504 fs.0.disk.count : 1 fs.0.disk.0.alias : vda fs.0.disk.0.serial : 947684ABZ8639Q fs.0.disk.0.device : /dev/vda2 fs.1.name : vda1 fs.1.mountpoint : /boot fs.1.fstype : ext4 fs.1.total-bytes : 952840192 fs.1.used-bytes : 229019648 fs.1.disk.count : 1 fs.1.disk.0.alias : vda fs.1.disk.0.serial : 947684ABZ8639Q fs.1.disk.0.device : /dev/vda1 fs.2.name : md127 fs.2.mountpoint : /run/media/test/971b1edc-da61-4015-b465-560a9b1b6e9b fs.2.fstype : ext4 fs.2.total-bytes : 1950134272 fs.2.used-bytes : 6291456 fs.2.disk.count : 2 fs.2.disk.0.alias : vdb fs.2.disk.0.device : /dev/vdb1 fs.2.disk.1.alias : vdc fs.2.disk.1.device : /dev/vdc1 In contrast, here is the output of the virsh command operating on a fedora24 guest: virsh # guestinfo fedora24 error: Reconnected to the hypervisor fs.count : 2 fs.0.name : dm-0 fs.0.mountpoint : / fs.0.fstype : ext4 fs.0.disk.count : 1 fs.0.disk.0.alias : vda fs.1.name : vda1 fs.1.mountpoint : /boot fs.1.fstype : ext4 fs.1.disk.count : 1 fs.1.disk.0.alias : vda However, if you specifically request an info category that is not supported by the guest agent: virsh # guestinfo --user fedora24 error: internal error: unable to execute QEMU agent command 'guest-get-users': The command guest-get-users has not been found Jonathon Jongsma (9): lib: add virDomainGetGuestInfo() remote: implement virDomainGetGuestInfo qemu: add helper for getting guest users qemu: add helper function for querying OS info qemu: add helper for querying timezone info qemu: add support for new fields in FSInfo qemu: add helper for getting full FSInfo qemu: Implement virDomainGetGuestInfo() virsh: add 'guestinfo' command include/libvirt/libvirt-domain.h | 14 + src/driver-hypervisor.h | 8 + src/libvirt-domain.c | 117 ++++++ src/libvirt_public.syms | 1 + src/qemu/qemu_agent.c | 482 ++++++++++++++++++++++- src/qemu/qemu_agent.h | 9 + src/qemu/qemu_driver.c | 110 ++++++ 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 | 576 +++++++++++++++++++++++++++- tools/virsh-domain.c | 85 ++++ 13 files changed, 1494 insertions(+), 35 deletions(-) -- 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 | 14 ++++ src/driver-hypervisor.h | 8 +++ src/libvirt-domain.c | 117 +++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 140 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index f160ee88b5..22277b0a84 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -4902,4 +4902,18 @@ 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 */ + VIR_DOMAIN_GUEST_INFO_FILESYSTEM = (1 << 4), /* return filesystem 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..ad68db7549 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12212,6 +12212,123 @@ 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: + * + * "user.count" - the number of active users on this domain as an + * unsigned int + * "user.<num>.name - username of the user as a string + * "user.<num>.domain - domain of the user as a string (may only be + * present on certain guest types) + * "user.<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: + * Returns inforamtion about the filesystems within the domain. The typed + * parameter keys are in this format: + * "fs.count" - the number of filesystems defined on this domain + * as an unsigned int + * "fs.<num>.mountpoint" - the path to the mount point for the filesystem + * "fs.<num>.name" - device name in the guest (e.g. "sda1") + * "fs.<num>.fstype" - the type of filesystem + * "fs.<num>.total-bytes" - the total size of the filesystem + * "fs.<num>.used-bytes" - the number of bytes used in the filesystem + * "fs.<num>.disk.count" - the number of disks targeted by this filesystem + * "fs.<num>.disk.<num>.alias" - the device alias of the disk (e.g. sda) + * "fs.<num>.disk.<num>.serial" - the serial number of the disk + * "fs.<num>.disk.<num>.device" - the device node of the disk + * + * 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 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); + 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 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; + } + + 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. @@ -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 */ }; 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 | 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; + 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 */ -- 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 | 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..cd4105bfb8 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..9467c51d54 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

On 8/21/19 7:15 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>
Code looks good, but it fails make syntax-check here in my env: cppi: src/qemu/qemu_agent.c: line 2360: not properly indented cppi: tests/qemuagenttest.c: line 1127: not properly indented maint.mk: incorrect preprocessor indentation make: *** [cfg.mk:699: sc_preprocessor_indentation] Error 1 make: *** Waiting for unfinished jobs. Thanks, DHB
--- 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..cd4105bfb8 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..9467c51d54 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 */

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 cd4105bfb8..d7c00250ea 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 9467c51d54..aa1e993649 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

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 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> --- 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 d7c00250ea..d5519cb243 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; + 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]; + + 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); -- 2.21.0

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() --- src/qemu/qemu_agent.c | 89 ++++++++++++++++++ src/qemu/qemu_agent.h | 5 + tests/qemuagenttest.c | 210 +++++++++++++++++++++++++++++++++++++++--- 3 files changed, 291 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index d5519cb243..c101805b23 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1953,6 +1953,95 @@ 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; + + nfs = qemuAgentGetFSInfoInternal(mon, &fsinfo, vmdef); + + 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 aa1e993649..cf80711e95 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; + virDomainDefPtr ret_def; - 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; + goto cleanup; + } + const char *name, *mountpoint, *fstype, *alias, *serial; + unsigned int diskcount; + unsigned long long bytesused, bytestotal; + 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; + 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); -- 2.21.0

This patch fails to compile in my env throwing this error: Â CCÂ Â Â Â Â Â qemuagenttest.o qemuagenttest.c: In function 'testQemuAgentGetFSInfoCommon.constprop': qemuagenttest.c:242:5: error: 'ret_def' may be used uninitialized in this function [-Werror=maybe-uninitialized] Â 242 |Â Â Â Â virDomainDefFree(ret_def); Â Â Â Â Â |Â Â Â Â ^~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors make[2]: *** [Makefile:5856: qemuagenttest.o] Error 1 I attempted to fix it by doing: ---- diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index be53f7e61a..00dcf26681 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -175,7 +175,7 @@ testQemuAgentGetFSInfoCommon(virDomainXMLOptionPtr xmlopt, Â Â Â Â int ret = -1; Â Â Â Â char *domain_filename = NULL; Â Â Â Â qemuMonitorTestPtr ret_test; -Â Â Â virDomainDefPtr ret_def; +Â Â Â virDomainDefPtr ret_def = NULL; Â Â Â Â if (!test || !def) Â Â Â Â Â Â Â Â return -1; ------ I was able to compile it, but then 'make check' got rekt: ---- [...] PASS: qemudomainsnapshotxml2xmltest PASS: qemucommandutiltest PASS: virsh-cpuset ../build-aux/test-driver: line 107: 19017 Segmentation fault (core dumped) "$@" > $log_file 2>&1 FAIL: qemuagenttest PASS: qemumigparamstest [...] ============================================================================ Testsuite summary for libvirt 5.7.0 ============================================================================ # TOTAL: 129 # PASS:Â 127 # SKIP:Â 1 # XFAIL: 0 # FAIL:Â 1 # XPASS: 0 # ERROR: 0 ----- Not sure if there is something wrong with the patch, or with the fix I made (seems unlikely, but ...) or perhaps something else missing in my env (is there any external libs needed for this?). Thanks, DHB On 8/21/19 7:15 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() --- src/qemu/qemu_agent.c | 89 ++++++++++++++++++ src/qemu/qemu_agent.h | 5 + tests/qemuagenttest.c | 210 +++++++++++++++++++++++++++++++++++++++--- 3 files changed, 291 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index d5519cb243..c101805b23 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1953,6 +1953,95 @@ 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; + + nfs = qemuAgentGetFSInfoInternal(mon, &fsinfo, vmdef); + + 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 aa1e993649..cf80711e95 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; + virDomainDefPtr ret_def;
- 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; + goto cleanup; + } + const char *name, *mountpoint, *fstype, *alias, *serial; + unsigned int diskcount; + unsigned long long bytesused, bytestotal; + 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; + 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 Thu, 2019-08-22 at 19:02 -0300, Daniel Henrique Barboza wrote: > This patch fails to compile in my env throwing this error: > > > CC qemuagenttest.o > qemuagenttest.c: In function > 'testQemuAgentGetFSInfoCommon.constprop': > qemuagenttest.c:242:5: error: 'ret_def' may be used uninitialized in > this function [-Werror=maybe-uninitialized] > 242 | virDomainDefFree(ret_def); > | ^~~~~~~~~~~~~~~~~~~~~~~~~ > cc1: all warnings being treated as errors > make[2]: *** [Makefile:5856: qemuagenttest.o] Error 1 > > > I attempted to fix it by doing: > > ---- > > diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c > index be53f7e61a..00dcf26681 100644 > --- a/tests/qemuagenttest.c > +++ b/tests/qemuagenttest.c > @@ -175,7 +175,7 @@ > testQemuAgentGetFSInfoCommon(virDomainXMLOptionPtr xmlopt, > int ret = -1; > char *domain_filename = NULL; > qemuMonitorTestPtr ret_test; > - virDomainDefPtr ret_def; > + virDomainDefPtr ret_def = NULL; > > if (!test || !def) > return -1; > > ------ > > I was able to compile it, but then 'make check' got rekt: > > ---- > [...] > PASS: qemudomainsnapshotxml2xmltest > PASS: qemucommandutiltest > PASS: virsh-cpuset > ../build-aux/test-driver: line 107: 19017 Segmentation fault > (core dumped) "$@" > $log_file 2>&1 > FAIL: qemuagenttest > PASS: qemumigparamstest > [...] > ===================================================================== > ======= > Testsuite summary for libvirt 5.7.0 > ===================================================================== > ======= > # TOTAL: 129 > # PASS: 127 > # SKIP: 1 > # XFAIL: 0 > # FAIL: 1 > # XPASS: 0 > # ERROR: 0 > ----- > > > Not sure if there is something wrong with the patch, or with the fix > I made (seems unlikely, but ...) or perhaps something else missing in > my > env (is there any external libs needed for this?). > > > Thanks, > > > DHB Hmm, sorry for the sloppiness. I ran these tests multiple times, but apparently after a final rebase, I introduced something that I failed to catch before submitting. Thanks for testing it out. Jonathon > > > On 8/21/19 7:15 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() > > --- > > src/qemu/qemu_agent.c | 89 ++++++++++++++++++ > > src/qemu/qemu_agent.h | 5 + > > tests/qemuagenttest.c | 210 > > +++++++++++++++++++++++++++++++++++++++--- > > 3 files changed, 291 insertions(+), 13 deletions(-) > > > > diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c > > index d5519cb243..c101805b23 100644 > > --- a/src/qemu/qemu_agent.c > > +++ b/src/qemu/qemu_agent.c > > @@ -1953,6 +1953,95 @@ 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; > > + > > + nfs = qemuAgentGetFSInfoInternal(mon, &fsinfo, vmdef); > > + > > + 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 aa1e993649..cf80711e95 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; > > + virDomainDefPtr ret_def; > > > > - 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; > > + goto cleanup; > > + } > > + const char *name, *mountpoint, *fstype, *alias, *serial; > > + unsigned int diskcount; > > + unsigned long long bytesused, bytestotal; > > + 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; > > + 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 11:00 AM, Jonathon Jongsma wrote: > On Thu, 2019-08-22 at 19:02 -0300, Daniel Henrique Barboza wrote: > [...] > Hmm, sorry for the sloppiness. I ran these tests multiple times, but > apparently after a final rebase, I introduced something that I failed > to catch before submitting. Thanks for testing it out. No problem. I'll wait for your next version and resume testing - I am interested in seeing this feature in action in PowerPC guests. Thanks, DHB > > Jonathon > > >> >> On 8/21/19 7:15 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() >>> --- >>> src/qemu/qemu_agent.c | 89 ++++++++++++++++++ >>> src/qemu/qemu_agent.h | 5 + >>> tests/qemuagenttest.c | 210 >>> +++++++++++++++++++++++++++++++++++++++--- >>> 3 files changed, 291 insertions(+), 13 deletions(-) >>> >>> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c >>> index d5519cb243..c101805b23 100644 >>> --- a/src/qemu/qemu_agent.c >>> +++ b/src/qemu/qemu_agent.c >>> @@ -1953,6 +1953,95 @@ 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; >>> + >>> + nfs = qemuAgentGetFSInfoInternal(mon, &fsinfo, vmdef); >>> + >>> + 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 aa1e993649..cf80711e95 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; >>> + virDomainDefPtr ret_def; >>> >>> - 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; >>> + goto cleanup; >>> + } >>> + const char *name, *mountpoint, *fstype, *alias, *serial; >>> + unsigned int diskcount; >>> + unsigned long long bytesused, bytestotal; >>> + 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; >>> + 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); >>

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 11f97dbc65..bfbb38a2c8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -23204,6 +23204,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, @@ -23439,6 +23548,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 | 85 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) 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: + 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} }; -- 2.21.0
participants (2)
-
Daniel Henrique Barboza
-
Jonathon Jongsma