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

Jonathon Jongsma posted 9 patches 6 years, 5 months ago
[libvirt] [PATCH v2 7/9] qemu: add helper for getting full FSInfo
Posted by Jonathon Jongsma 6 years, 5 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()
---
 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),
+                                 &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 v2 7/9] qemu: add helper for getting full FSInfo
Posted by Daniel Henrique Barboza 6 years, 5 months ago
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),
> +                                 &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);

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 7/9] qemu: add helper for getting full FSInfo
Posted by Jonathon Jongsma 6 years, 5 months ago
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),
> > +                                 &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);
>  

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

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

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