[libvirt] [PATCH v3 7/9] qemu: add helper for getting full FSInfo

Jonathon Jongsma posted 9 patches 5 years, 3 months ago
[libvirt] [PATCH v3 7/9] qemu: add helper for getting full FSInfo
Posted by Jonathon Jongsma 5 years, 3 months ago
This function adds the complete filesystem information returned by the
qemu agent to an array of typed parameters with field names intended to
to be returned by virDomainGetGuestInfo()

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
 src/qemu/qemu_agent.c |  90 ++++++++++++++++++
 src/qemu/qemu_agent.h |   5 +
 tests/qemuagenttest.c | 210 +++++++++++++++++++++++++++++++++++++++---
 3 files changed, 292 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 8d54979f89..169ad74f6f 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1953,6 +1953,96 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,
     return ret;
 }
 
+int
+qemuAgentGetFSInfoParams(qemuAgentPtr mon,
+                         virTypedParameterPtr *params,
+                         int *nparams, int *maxparams,
+                         virDomainDefPtr vmdef)
+{
+    int ret = -1;
+    qemuAgentFSInfoPtr *fsinfo = NULL;
+    size_t i, j;
+    int nfs;
+
+    if ((nfs = qemuAgentGetFSInfoInternal(mon, &fsinfo, vmdef)) < 0)
+        return -1;
+
+    if (virTypedParamsAddUInt(params, nparams, maxparams,
+                              "fs.count", nfs) < 0)
+        goto cleanup;
+
+    for (i = 0; i < nfs; i++) {
+        char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
+        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+                 "fs.%zu.name", i);
+        if (virTypedParamsAddString(params, nparams, maxparams,
+                                    param_name, fsinfo[i]->name) < 0)
+            goto cleanup;
+        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+                 "fs.%zu.mountpoint", i);
+        if (virTypedParamsAddString(params, nparams, maxparams,
+                                    param_name, fsinfo[i]->mountpoint) < 0)
+            goto cleanup;
+        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+                 "fs.%zu.fstype", i);
+        if (virTypedParamsAddString(params, nparams, maxparams,
+                                    param_name, fsinfo[i]->fstype) < 0)
+            goto cleanup;
+
+        /* disk usage values are not returned by older guest agents, so
+         * only add the params if the value is set */
+        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+                 "fs.%zu.total-bytes", i);
+        if (fsinfo[i]->total_bytes != -1 &&
+            virTypedParamsAddULLong(params, nparams, maxparams,
+                                    param_name, fsinfo[i]->total_bytes) < 0)
+            goto cleanup;
+
+        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+                 "fs.%zu.used-bytes", i);
+        if (fsinfo[i]->used_bytes != -1 &&
+            virTypedParamsAddULLong(params, nparams, maxparams,
+                                    param_name, fsinfo[i]->used_bytes) < 0)
+            goto cleanup;
+
+        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+                 "fs.%zu.disk.count", i);
+        if (virTypedParamsAddUInt(params, nparams, maxparams,
+                                  param_name, fsinfo[i]->ndisks) < 0)
+            goto cleanup;
+        for (j = 0; j < fsinfo[i]->ndisks; j++) {
+            qemuAgentDiskInfoPtr d = fsinfo[i]->disks[j];
+            snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+                     "fs.%zu.disk.%zu.alias", i, j);
+            if (d->alias &&
+                virTypedParamsAddString(params, nparams, maxparams,
+                                        param_name, d->alias) < 0)
+                goto cleanup;
+
+            snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+                     "fs.%zu.disk.%zu.serial", i, j);
+            if (d->serial &&
+                virTypedParamsAddString(params, nparams, maxparams,
+                                        param_name, d->serial) < 0)
+                goto cleanup;
+
+            snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+                     "fs.%zu.disk.%zu.device", i, j);
+            if (d->devnode &&
+                virTypedParamsAddString(params, nparams, maxparams,
+                                        param_name, d->devnode) < 0)
+                goto cleanup;
+        }
+    }
+    ret = nfs;
+
+ cleanup:
+    for (i = 0; i < nfs; i++)
+        qemuAgentFSInfoFree(fsinfo[i]);
+    VIR_FREE(fsinfo);
+
+    return ret;
+}
 
 static int
 qemuAgentGetFSInfoInternal(qemuAgentPtr mon, qemuAgentFSInfoPtr **info,
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 69b0176855..f6d74a2603 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -74,6 +74,11 @@ int qemuAgentFSThaw(qemuAgentPtr mon);
 int qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,
                        virDomainDefPtr vmdef);
 
+int qemuAgentGetFSInfoParams(qemuAgentPtr mon,
+                             virTypedParameterPtr *params,
+                             int *nparams, int *maxparams,
+                             virDomainDefPtr vmdef);
+
 int qemuAgentSuspend(qemuAgentPtr mon,
                      unsigned int target);
 
diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index 0912a5f29a..105b32017a 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -168,38 +168,45 @@ testQemuAgentFSTrim(const void *data)
 
 
 static int
-testQemuAgentGetFSInfo(const void *data)
+testQemuAgentGetFSInfoCommon(virDomainXMLOptionPtr xmlopt,
+                             qemuMonitorTestPtr *test,
+                             virDomainDefPtr *def)
 {
-    virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
-    qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
+    int ret = -1;
     char *domain_filename = NULL;
-    virDomainDefPtr def = NULL;
-    virDomainFSInfoPtr *info = NULL;
-    int ret = -1, ninfo = 0, i;
+    qemuMonitorTestPtr ret_test = NULL;
+    virDomainDefPtr ret_def = NULL;
 
-    if (!test)
+    if (!test || !def)
+        return -1;
+
+    if (!(ret_test = qemuMonitorTestNewAgent(xmlopt)))
         return -1;
 
     if (virAsprintf(&domain_filename, "%s/qemuagentdata/fsinfo.xml",
                     abs_srcdir) < 0)
         goto cleanup;
 
-    if (!(def = virDomainDefParseFile(domain_filename, driver.caps, xmlopt,
-                                      NULL, VIR_DOMAIN_DEF_PARSE_INACTIVE)))
+    if (!(ret_def = virDomainDefParseFile(domain_filename, driver.caps, xmlopt,
+                                          NULL, VIR_DOMAIN_DEF_PARSE_INACTIVE)))
         goto cleanup;
 
-    if (qemuMonitorTestAddAgentSyncResponse(test) < 0)
+    if (qemuMonitorTestAddAgentSyncResponse(ret_test) < 0)
         goto cleanup;
 
-    if (qemuMonitorTestAddItem(test, "guest-get-fsinfo",
+    if (qemuMonitorTestAddItem(ret_test, "guest-get-fsinfo",
                                "{\"return\": ["
                                "  {\"name\": \"sda1\", \"mountpoint\": \"/\","
+                               "   \"total-bytes\":952840192,"
+                               "   \"used-bytes\":229019648,"
                                "   \"disk\": ["
-                               "     {\"bus-type\": \"ide\","
+                               "     {\"serial\": \"ARBITRARYSTRING\","
+                               "      \"bus-type\": \"ide\","
                                "      \"bus\": 1, \"unit\": 0,"
                                "      \"pci-controller\": {"
                                "        \"bus\": 0, \"slot\": 1,"
                                "        \"domain\": 0, \"function\": 1},"
+                               "      \"dev\": \"/dev/sda1\","
                                "      \"target\": 0}],"
                                "   \"type\": \"ext4\"},"
                                "  {\"name\": \"dm-1\","
@@ -221,6 +228,32 @@ testQemuAgentGetFSInfo(const void *data)
                                "  {\"name\": \"sdb1\","
                                "   \"mountpoint\": \"/mnt/disk\","
                                "   \"disk\": [], \"type\": \"xfs\"}]}") < 0)
+                               goto cleanup;
+    *test = ret_test;
+    ret_test = NULL;
+    *def = ret_def;
+    ret_def = NULL;
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(domain_filename);
+    if (ret_test)
+        qemuMonitorTestFree(ret_test);
+    virDomainDefFree(ret_def);
+
+    return ret;
+}
+
+static int
+testQemuAgentGetFSInfo(const void *data)
+{
+    virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+    qemuMonitorTestPtr test = NULL;
+    virDomainDefPtr def = NULL;
+    virDomainFSInfoPtr *info = NULL;
+    int ret = -1, ninfo = 0, i;
+
+    if (testQemuAgentGetFSInfoCommon(xmlopt, &test, &def) < 0)
         goto cleanup;
 
     if ((ninfo = qemuAgentGetFSInfo(qemuMonitorTestGetAgent(test),
@@ -295,7 +328,157 @@ testQemuAgentGetFSInfo(const void *data)
     for (i = 0; i < ninfo; i++)
         virDomainFSInfoFree(info[i]);
     VIR_FREE(info);
-    VIR_FREE(domain_filename);
+    virDomainDefFree(def);
+    qemuMonitorTestFree(test);
+    return ret;
+}
+
+static int
+testQemuAgentGetFSInfoParams(const void *data)
+{
+    virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+    qemuMonitorTestPtr test = NULL;
+    virDomainDefPtr def = NULL;
+    virTypedParameterPtr params = NULL;
+    int nparams = 0, maxparams = 0;
+    int ret = -1;
+    unsigned int count;
+
+    if (testQemuAgentGetFSInfoCommon(xmlopt, &test, &def) < 0)
+        goto cleanup;
+
+    if (qemuAgentGetFSInfoParams(qemuMonitorTestGetAgent(test),
+                                 &params, &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), &params,
+                                 &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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 7/9] qemu: add helper for getting full FSInfo
Posted by Daniel Henrique Barboza 5 years, 3 months ago

On 8/23/19 1:31 PM, Jonathon Jongsma wrote:
> This function adds the complete filesystem information returned by the
> qemu agent to an array of typed parameters with field names intended to
> to be returned by virDomainGetGuestInfo()
>
> Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
> ---

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


A few comments below:


>   src/qemu/qemu_agent.c |  90 ++++++++++++++++++
>   src/qemu/qemu_agent.h |   5 +
>   tests/qemuagenttest.c | 210 +++++++++++++++++++++++++++++++++++++++---
>   3 files changed, 292 insertions(+), 13 deletions(-)
>
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index 8d54979f89..169ad74f6f 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -1953,6 +1953,96 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,
>       return ret;
>   }
>   
> +int
> +qemuAgentGetFSInfoParams(qemuAgentPtr mon,
> +                         virTypedParameterPtr *params,
> +                         int *nparams, int *maxparams,
> +                         virDomainDefPtr vmdef)
> +{
> +    int ret = -1;
> +    qemuAgentFSInfoPtr *fsinfo = NULL;
> +    size_t i, j;
> +    int nfs;
> +
> +    if ((nfs = qemuAgentGetFSInfoInternal(mon, &fsinfo, vmdef)) < 0)
> +        return -1;
> +
> +    if (virTypedParamsAddUInt(params, nparams, maxparams,
> +                              "fs.count", nfs) < 0)
> +        goto cleanup;
> +
> +    for (i = 0; i < nfs; i++) {
> +        char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
> +        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> +                 "fs.%zu.name", i);
> +        if (virTypedParamsAddString(params, nparams, maxparams,
> +                                    param_name, fsinfo[i]->name) < 0)
> +            goto cleanup;
> +        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> +                 "fs.%zu.mountpoint", i);
> +        if (virTypedParamsAddString(params, nparams, maxparams,
> +                                    param_name, fsinfo[i]->mountpoint) < 0)
> +            goto cleanup;
> +        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> +                 "fs.%zu.fstype", i);
> +        if (virTypedParamsAddString(params, nparams, maxparams,
> +                                    param_name, fsinfo[i]->fstype) < 0)
> +            goto cleanup;
> +
> +        /* disk usage values are not returned by older guest agents, so
> +         * only add the params if the value is set */
> +        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> +                 "fs.%zu.total-bytes", i);
> +        if (fsinfo[i]->total_bytes != -1 &&
> +            virTypedParamsAddULLong(params, nparams, maxparams,
> +                                    param_name, fsinfo[i]->total_bytes) < 0)
> +            goto cleanup;
> +
> +        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> +                 "fs.%zu.used-bytes", i);
> +        if (fsinfo[i]->used_bytes != -1 &&
> +            virTypedParamsAddULLong(params, nparams, maxparams,
> +                                    param_name, fsinfo[i]->used_bytes) < 0)
> +            goto cleanup;
> +
> +        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> +                 "fs.%zu.disk.count", i);
> +        if (virTypedParamsAddUInt(params, nparams, maxparams,
> +                                  param_name, fsinfo[i]->ndisks) < 0)
> +            goto cleanup;
> +        for (j = 0; j < fsinfo[i]->ndisks; j++) {
> +            qemuAgentDiskInfoPtr d = fsinfo[i]->disks[j];
> +            snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> +                     "fs.%zu.disk.%zu.alias", i, j);
> +            if (d->alias &&
> +                virTypedParamsAddString(params, nparams, maxparams,
> +                                        param_name, d->alias) < 0)
> +                goto cleanup;
> +
> +            snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> +                     "fs.%zu.disk.%zu.serial", i, j);
> +            if (d->serial &&
> +                virTypedParamsAddString(params, nparams, maxparams,
> +                                        param_name, d->serial) < 0)
> +                goto cleanup;
> +
> +            snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> +                     "fs.%zu.disk.%zu.device", i, j);
> +            if (d->devnode &&
> +                virTypedParamsAddString(params, nparams, maxparams,
> +                                        param_name, d->devnode) < 0)
> +                goto cleanup;
> +        }
> +    }
> +    ret = nfs;
> +
> + cleanup:
> +    for (i = 0; i < nfs; i++)
> +        qemuAgentFSInfoFree(fsinfo[i]);
> +    VIR_FREE(fsinfo);
> +
> +    return ret;
> +}
>   
>   static int
>   qemuAgentGetFSInfoInternal(qemuAgentPtr mon, qemuAgentFSInfoPtr **info,
> diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
> index 69b0176855..f6d74a2603 100644
> --- a/src/qemu/qemu_agent.h
> +++ b/src/qemu/qemu_agent.h
> @@ -74,6 +74,11 @@ int qemuAgentFSThaw(qemuAgentPtr mon);
>   int qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,
>                          virDomainDefPtr vmdef);
>   
> +int qemuAgentGetFSInfoParams(qemuAgentPtr mon,
> +                             virTypedParameterPtr *params,
> +                             int *nparams, int *maxparams,
> +                             virDomainDefPtr vmdef);
> +
>   int qemuAgentSuspend(qemuAgentPtr mon,
>                        unsigned int target);
>   
> diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
> index 0912a5f29a..105b32017a 100644
> --- a/tests/qemuagenttest.c
> +++ b/tests/qemuagenttest.c
> @@ -168,38 +168,45 @@ testQemuAgentFSTrim(const void *data)
>   
>   
>   static int
> -testQemuAgentGetFSInfo(const void *data)
> +testQemuAgentGetFSInfoCommon(virDomainXMLOptionPtr xmlopt,
> +                             qemuMonitorTestPtr *test,
> +                             virDomainDefPtr *def)
>   {
> -    virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
> -    qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
> +    int ret = -1;
>       char *domain_filename = NULL;
> -    virDomainDefPtr def = NULL;
> -    virDomainFSInfoPtr *info = NULL;
> -    int ret = -1, ninfo = 0, i;
> +    qemuMonitorTestPtr ret_test = NULL;
> +    virDomainDefPtr ret_def = NULL;
>   
> -    if (!test)
> +    if (!test || !def)
> +        return -1;
> +
> +    if (!(ret_test = qemuMonitorTestNewAgent(xmlopt)))
>           return -1;
>   
>       if (virAsprintf(&domain_filename, "%s/qemuagentdata/fsinfo.xml",
>                       abs_srcdir) < 0)
>           goto cleanup;
>   
> -    if (!(def = virDomainDefParseFile(domain_filename, driver.caps, xmlopt,
> -                                      NULL, VIR_DOMAIN_DEF_PARSE_INACTIVE)))
> +    if (!(ret_def = virDomainDefParseFile(domain_filename, driver.caps, xmlopt,
> +                                          NULL, VIR_DOMAIN_DEF_PARSE_INACTIVE)))
>           goto cleanup;
>   
> -    if (qemuMonitorTestAddAgentSyncResponse(test) < 0)
> +    if (qemuMonitorTestAddAgentSyncResponse(ret_test) < 0)
>           goto cleanup;
>   
> -    if (qemuMonitorTestAddItem(test, "guest-get-fsinfo",
> +    if (qemuMonitorTestAddItem(ret_test, "guest-get-fsinfo",
>                                  "{\"return\": ["
>                                  "  {\"name\": \"sda1\", \"mountpoint\": \"/\","
> +                               "   \"total-bytes\":952840192,"
> +                               "   \"used-bytes\":229019648,"
>                                  "   \"disk\": ["
> -                               "     {\"bus-type\": \"ide\","
> +                               "     {\"serial\": \"ARBITRARYSTRING\","
> +                               "      \"bus-type\": \"ide\","
>                                  "      \"bus\": 1, \"unit\": 0,"
>                                  "      \"pci-controller\": {"
>                                  "        \"bus\": 0, \"slot\": 1,"
>                                  "        \"domain\": 0, \"function\": 1},"
> +                               "      \"dev\": \"/dev/sda1\","
>                                  "      \"target\": 0}],"
>                                  "   \"type\": \"ext4\"},"
>                                  "  {\"name\": \"dm-1\","
> @@ -221,6 +228,32 @@ testQemuAgentGetFSInfo(const void *data)
>                                  "  {\"name\": \"sdb1\","
>                                  "   \"mountpoint\": \"/mnt/disk\","
>                                  "   \"disk\": [], \"type\": \"xfs\"}]}") < 0)
> +                               goto cleanup;
> +    *test = ret_test;
> +    ret_test = NULL;
> +    *def = ret_def;
> +    ret_def = NULL;
> +    ret = 0;
> +
> + cleanup:
> +    VIR_FREE(domain_filename);
> +    if (ret_test)
> +        qemuMonitorTestFree(ret_test);
> +    virDomainDefFree(ret_def);
> +
> +    return ret;
> +}
> +
> +static int
> +testQemuAgentGetFSInfo(const void *data)
> +{
> +    virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
> +    qemuMonitorTestPtr test = NULL;
> +    virDomainDefPtr def = NULL;
> +    virDomainFSInfoPtr *info = NULL;
> +    int ret = -1, ninfo = 0, i;
> +
> +    if (testQemuAgentGetFSInfoCommon(xmlopt, &test, &def) < 0)
>           goto cleanup;
>   
>       if ((ninfo = qemuAgentGetFSInfo(qemuMonitorTestGetAgent(test),
> @@ -295,7 +328,157 @@ testQemuAgentGetFSInfo(const void *data)
>       for (i = 0; i < ninfo; i++)
>           virDomainFSInfoFree(info[i]);
>       VIR_FREE(info);
> -    VIR_FREE(domain_filename);
> +    virDomainDefFree(def);
> +    qemuMonitorTestFree(test);
> +    return ret;
> +}
> +
> +static int
> +testQemuAgentGetFSInfoParams(const void *data)
> +{
> +    virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
> +    qemuMonitorTestPtr test = NULL;
> +    virDomainDefPtr def = NULL;
> +    virTypedParameterPtr params = NULL;
> +    int nparams = 0, maxparams = 0;
> +    int ret = -1;
> +    unsigned int count;
> +
> +    if (testQemuAgentGetFSInfoCommon(xmlopt, &test, &def) < 0)
> +        goto cleanup;
> +
> +    if (qemuAgentGetFSInfoParams(qemuMonitorTestGetAgent(test),
> +                                 &params, &nparams, &maxparams, def) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       "Failed to execute qemuAgentGetFSInfoParams()");
> +        goto cleanup;
> +    }
> +
> +    if (virTypedParamsGetUInt(params, nparams, "fs.count", &count) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       "expected filesystem count");
> +        goto cleanup;
> +    }
> +
> +    if (count != 3) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "expected 3 filesystems information, got %d", count);
> +        ret = -1;

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

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

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

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

Same comment as above.

> +    if (virTypedParamsGetString(params, nparams, "fs.1.name", &name) < 0 ||
> +        virTypedParamsGetString(params, nparams, "fs.1.mountpoint", &mountpoint) < 0 ||
> +        virTypedParamsGetString(params, nparams, "fs.1.fstype", &fstype) < 0 ||
> +        virTypedParamsGetULLong(params, nparams, "fs.1.used-bytes", &bytesused) == 1 ||
> +        virTypedParamsGetULLong(params, nparams, "fs.1.total-bytes", &bytestotal) == 1 ||
> +        virTypedParamsGetUInt(params, nparams, "fs.1.disk.count", &diskcount) < 0 ||
> +        virTypedParamsGetString(params, nparams, "fs.1.disk.0.alias", &alias) < 0 ||
> +        virTypedParamsGetString(params, nparams, "fs.1.disk.1.alias", &alias2) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +            "Incorrect parameters for dm-1 (%s,%s)",
> +            name, alias);
> +        ret = -1;
> +        goto cleanup;
> +    }
> +    if (STRNEQ(name, "dm-1") ||
> +        STRNEQ(mountpoint, "/opt") ||
> +        STRNEQ(fstype, "vfat") ||
> +        diskcount != 2 ||
> +        STRNEQ(alias, "vda") ||
> +        STRNEQ(alias2, "vdb")) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +            "unexpected filesystems information returned for dm-1 (%s,%s)",
> +            name, alias);
> +        ret = -1;
> +        goto cleanup;
> +    }
> +
> +    alias = NULL;
> +    if (virTypedParamsGetString(params, nparams, "fs.0.name", &name) < 0 ||
> +        virTypedParamsGetString(params, nparams, "fs.0.mountpoint", &mountpoint) < 0 ||
> +        virTypedParamsGetString(params, nparams, "fs.0.fstype", &fstype) < 0 ||
> +        virTypedParamsGetULLong(params, nparams, "fs.0.used-bytes", &bytesused) == 1 ||
> +        virTypedParamsGetULLong(params, nparams, "fs.0.total-bytes", &bytestotal) == 1 ||
> +        virTypedParamsGetUInt(params, nparams, "fs.0.disk.count", &diskcount) < 0 ||
> +        virTypedParamsGetString(params, nparams, "fs.0.disk.0.alias", &alias) == 1) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +            "Incorrect parameters for sdb1 (%s,%s)",
> +            name, alias);
> +        ret = -1;
> +        goto cleanup;
> +    }
> +    if (STRNEQ(name, "sdb1") ||
> +        STRNEQ(mountpoint, "/mnt/disk") ||
> +        STRNEQ(fstype, "xfs") ||
> +        diskcount != 0 ||
> +        alias != NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +            "unexpected filesystems information returned for sdb1 (%s,%s)",
> +            name, alias);
> +        ret = -1;
> +        goto cleanup;
> +    }
> +
> +    if (qemuMonitorTestAddAgentSyncResponse(test) < 0)
> +        goto cleanup;
> +
> +    if (qemuMonitorTestAddItem(test, "guest-get-fsinfo",
> +                               "{\"error\":"
> +                               "    {\"class\":\"CommandDisabled\","
> +                               "     \"desc\":\"The command guest-get-fsinfo "
> +                                               "has been disabled for "
> +                                               "this instance\","
> +                               "     \"data\":{\"name\":\"guest-get-fsinfo\"}"
> +                               "    }"
> +                               "}") < 0)
> +        goto cleanup;
> +
> +    if (qemuAgentGetFSInfoParams(qemuMonitorTestGetAgent(test), &params,
> +                                 &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);

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list