render / rpms / libvirt

Forked from rpms/libvirt 10 months ago
Clone
Blob Blame History Raw
From 314e0b35249ff662cb76d9b03f33aeb700c6a43a Mon Sep 17 00:00:00 2001
Message-Id: <314e0b35249ff662cb76d9b03f33aeb700c6a43a@dist-git>
From: Jonathon Jongsma <jjongsma@redhat.com>
Date: Thu, 20 Feb 2020 10:52:26 -0600
Subject: [PATCH] qemu: don't access vmdef within qemu_agent.c
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In order to avoid holding an agent job and a normal job at the same
time, we want to avoid accessing the domain's definition while holding
the agent job. To achieve this, qemuAgentGetFSInfo() only returns the
raw information from the agent query to the caller. The caller can then
release the agent job and then proceed to look up the disk alias from
the vm definition. This necessitates moving a few helper functions to
qemu_driver.c and exposing the agent data structure (qemuAgentFSInfo) in
the header.

In addition, because the agent function no longer returns the looked-up
disk alias, we can't test the alias within qemuagenttest.  Instead we
simply test that we parse and return the raw agent data correctly.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 599ae372d8cf0923757c5a3792acb07dcf3e8802)
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1759566
Message-Id: <20200220165227.11491-5-jjongsma@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 src/qemu/qemu_agent.c  | 219 +----------------------------------
 src/qemu/qemu_agent.h  |  33 ++++--
 src/qemu/qemu_driver.c | 255 ++++++++++++++++++++++++++++++++++++++---
 tests/qemuagenttest.c  | 196 +++++--------------------------
 4 files changed, 299 insertions(+), 404 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 4739faeed8..ef2d2c500b 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1844,30 +1844,6 @@ qemuAgentSetTime(qemuAgentPtr mon,
     return ret;
 }
 
-typedef struct _qemuAgentDiskInfo qemuAgentDiskInfo;
-typedef qemuAgentDiskInfo *qemuAgentDiskInfoPtr;
-struct _qemuAgentDiskInfo {
-    char *serial;
-    virPCIDeviceAddress pci_controller;
-    char *bus_type;
-    unsigned int bus;
-    unsigned int target;
-    unsigned int unit;
-    char *devnode;
-};
-
-typedef struct _qemuAgentFSInfo qemuAgentFSInfo;
-typedef qemuAgentFSInfo *qemuAgentFSInfoPtr;
-struct _qemuAgentFSInfo {
-    char *mountpoint; /* path to mount point */
-    char *name;       /* device name in the guest (e.g. "sda1") */
-    char *fstype;     /* filesystem type */
-    long long total_bytes;
-    long long used_bytes;
-    size_t ndisks;
-    qemuAgentDiskInfoPtr *disks;
-};
-
 static void
 qemuAgentDiskInfoFree(qemuAgentDiskInfoPtr info)
 {
@@ -1880,7 +1856,7 @@ qemuAgentDiskInfoFree(qemuAgentDiskInfoPtr info)
     VIR_FREE(info);
 }
 
-static void
+void
 qemuAgentFSInfoFree(qemuAgentFSInfoPtr info)
 {
     size_t i;
@@ -1899,47 +1875,6 @@ qemuAgentFSInfoFree(qemuAgentFSInfoPtr info)
     VIR_FREE(info);
 }
 
-static virDomainFSInfoPtr
-qemuAgentFSInfoToPublic(qemuAgentFSInfoPtr agent,
-                        virDomainDefPtr vmdef)
-{
-    virDomainFSInfoPtr ret = NULL;
-    size_t i;
-
-    if (VIR_ALLOC(ret) < 0)
-        goto error;
-
-    ret->mountpoint = g_strdup(agent->mountpoint);
-    ret->name = g_strdup(agent->name);
-    ret->fstype = g_strdup(agent->fstype);
-
-    if (agent->disks &&
-        VIR_ALLOC_N(ret->devAlias, agent->ndisks) < 0)
-        goto error;
-
-    ret->ndevAlias = agent->ndisks;
-
-    for (i = 0; i < ret->ndevAlias; i++) {
-        qemuAgentDiskInfoPtr agentdisk = agent->disks[i];
-        virDomainDiskDefPtr diskDef;
-
-        if (!(diskDef = virDomainDiskByAddress(vmdef,
-                                               &agentdisk->pci_controller,
-                                               agentdisk->bus,
-                                               agentdisk->target,
-                                               agentdisk->unit)))
-            continue;
-
-        ret->devAlias[i] = g_strdup(diskDef->dst);
-    }
-
-    return ret;
-
- error:
-    virDomainFSInfoFree(ret);
-    return NULL;
-}
-
 static int
 qemuAgentGetFSInfoFillDisks(virJSONValuePtr jsondisks,
                             qemuAgentFSInfoPtr fsinfo)
@@ -2013,7 +1948,6 @@ qemuAgentGetFSInfoFillDisks(virJSONValuePtr jsondisks,
         GET_DISK_ADDR(pci, &disk->pci_controller.bus, "bus");
         GET_DISK_ADDR(pci, &disk->pci_controller.slot, "slot");
         GET_DISK_ADDR(pci, &disk->pci_controller.function, "function");
-
 #undef GET_DISK_ADDR
     }
 
@@ -2024,9 +1958,9 @@ qemuAgentGetFSInfoFillDisks(virJSONValuePtr jsondisks,
  *          -2 when agent command is not supported by the agent
  *          -1 otherwise
  */
-static int
-qemuAgentGetFSInfoInternal(qemuAgentPtr mon,
-                           qemuAgentFSInfoPtr **info)
+int
+qemuAgentGetFSInfo(qemuAgentPtr mon,
+                   qemuAgentFSInfoPtr **info)
 {
     size_t i;
     int ret = -1;
@@ -2158,151 +2092,6 @@ qemuAgentGetFSInfoInternal(qemuAgentPtr mon,
     return ret;
 }
 
-/* Returns: 0 on success
- *          -1 otherwise
- */
-int
-qemuAgentGetFSInfo(qemuAgentPtr mon,
-                   virDomainFSInfoPtr **info,
-                   virDomainDefPtr vmdef)
-{
-    int ret = -1;
-    qemuAgentFSInfoPtr *agentinfo = NULL;
-    virDomainFSInfoPtr *info_ret = NULL;
-    size_t i;
-    int nfs;
-
-    nfs = qemuAgentGetFSInfoInternal(mon, &agentinfo);
-    if (nfs < 0)
-        return ret;
-    if (VIR_ALLOC_N(info_ret, nfs) < 0)
-        goto cleanup;
-
-    for (i = 0; i < nfs; i++) {
-        if (!(info_ret[i] = qemuAgentFSInfoToPublic(agentinfo[i], vmdef)))
-            goto cleanup;
-    }
-
-    *info = g_steal_pointer(&info_ret);
-    ret = nfs;
-
- cleanup:
-    for (i = 0; i < nfs; i++) {
-        qemuAgentFSInfoFree(agentinfo[i]);
-        /* if there was an error, free any memory we've allocated for the
-         * return value */
-        if (info_ret)
-            virDomainFSInfoFree(info_ret[i]);
-    }
-    VIR_FREE(agentinfo);
-    VIR_FREE(info_ret);
-    return ret;
-}
-
-/* Returns: 0 on success
- *          -2 when agent command is not supported by the agent
- *          -1 otherwise
- */
-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)) < 0)
-        return nfs;
-
-    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];
-        g_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;
-        g_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;
-        g_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 */
-        g_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;
-
-        g_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;
-
-        g_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++) {
-            virDomainDiskDefPtr diskdef = NULL;
-            qemuAgentDiskInfoPtr d = fsinfo[i]->disks[j];
-            /* match the disk to the target in the vm definition */
-            diskdef = virDomainDiskByAddress(vmdef,
-                                             &d->pci_controller,
-                                             d->bus,
-                                             d->target,
-                                             d->unit);
-            if (diskdef) {
-                g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
-                           "fs.%zu.disk.%zu.alias", i, j);
-                if (diskdef->dst &&
-                    virTypedParamsAddString(params, nparams, maxparams,
-                                            param_name, diskdef->dst) < 0)
-                    goto cleanup;
-            }
-
-            g_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;
-
-            g_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;
-}
-
 /*
  * qemuAgentGetInterfaces:
  * @mon: Agent monitor
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 85e436cf68..5656fe60ff 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -65,19 +65,38 @@ typedef enum {
     QEMU_AGENT_SHUTDOWN_LAST,
 } qemuAgentShutdownMode;
 
+typedef struct _qemuAgentDiskInfo qemuAgentDiskInfo;
+typedef qemuAgentDiskInfo *qemuAgentDiskInfoPtr;
+struct _qemuAgentDiskInfo {
+    char *serial;
+    virPCIDeviceAddress pci_controller;
+    char *bus_type;
+    unsigned int bus;
+    unsigned int target;
+    unsigned int unit;
+    char *devnode;
+};
+
+typedef struct _qemuAgentFSInfo qemuAgentFSInfo;
+typedef qemuAgentFSInfo *qemuAgentFSInfoPtr;
+struct _qemuAgentFSInfo {
+    char *mountpoint; /* path to mount point */
+    char *name;       /* device name in the guest (e.g. "sda1") */
+    char *fstype;     /* filesystem type */
+    long long total_bytes;
+    long long used_bytes;
+    size_t ndisks;
+    qemuAgentDiskInfoPtr *disks;
+};
+void qemuAgentFSInfoFree(qemuAgentFSInfoPtr info);
+
 int qemuAgentShutdown(qemuAgentPtr mon,
                       qemuAgentShutdownMode mode);
 
 int qemuAgentFSFreeze(qemuAgentPtr mon,
                       const char **mountpoints, unsigned int nmountpoints);
 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 qemuAgentGetFSInfo(qemuAgentPtr mon, qemuAgentFSInfoPtr **info);
 
 int qemuAgentSuspend(qemuAgentPtr mon,
                      unsigned int target);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 69e4f7264b..ac3a7ad282 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -21839,6 +21839,111 @@ qemuNodeAllocPages(virConnectPtr conn,
                                 startCell, cellCount, add);
 }
 
+static int
+qemuDomainGetFSInfoAgent(virQEMUDriverPtr driver,
+                         virDomainObjPtr vm,
+                         qemuAgentFSInfoPtr **info)
+{
+    int ret = -1;
+    qemuAgentPtr agent;
+
+    if (qemuDomainObjBeginAgentJob(driver, vm,
+                                   QEMU_AGENT_JOB_QUERY) < 0)
+        return ret;
+
+    if (virDomainObjCheckActive(vm) < 0)
+        goto endjob;
+
+    if (!qemuDomainAgentAvailable(vm, true))
+        goto endjob;
+
+    agent = qemuDomainObjEnterAgent(vm);
+    ret = qemuAgentGetFSInfo(agent, info);
+    qemuDomainObjExitAgent(vm, agent);
+
+ endjob:
+    qemuDomainObjEndAgentJob(vm);
+    return ret;
+}
+
+static virDomainFSInfoPtr
+qemuAgentFSInfoToPublic(qemuAgentFSInfoPtr agent,
+                        virDomainDefPtr vmdef)
+{
+    virDomainFSInfoPtr ret = NULL;
+    size_t i;
+
+    if (VIR_ALLOC(ret) < 0)
+        goto error;
+
+    ret->mountpoint = g_strdup(agent->mountpoint);
+    ret->name = g_strdup(agent->name);
+    ret->fstype = g_strdup(agent->fstype);
+
+    if (agent->disks &&
+        VIR_ALLOC_N(ret->devAlias, agent->ndisks) < 0)
+        goto error;
+
+    ret->ndevAlias = agent->ndisks;
+
+    for (i = 0; i < ret->ndevAlias; i++) {
+        qemuAgentDiskInfoPtr agentdisk = agent->disks[i];
+        virDomainDiskDefPtr diskDef;
+
+        if (!(diskDef = virDomainDiskByAddress(vmdef,
+                                               &agentdisk->pci_controller,
+                                               agentdisk->bus,
+                                               agentdisk->target,
+                                               agentdisk->unit)))
+            continue;
+
+        ret->devAlias[i] = g_strdup(diskDef->dst);
+    }
+
+    return ret;
+
+ error:
+    virDomainFSInfoFree(ret);
+    return NULL;
+}
+
+/* Returns: 0 on success
+ *          -1 otherwise
+ */
+static int
+virDomainFSInfoFormat(qemuAgentFSInfoPtr *agentinfo,
+                      int nagentinfo,
+                      virDomainDefPtr vmdef,
+                      virDomainFSInfoPtr **info)
+{
+    int ret = -1;
+    virDomainFSInfoPtr *info_ret = NULL;
+    size_t i;
+
+    if (nagentinfo < 0)
+        return ret;
+    if (VIR_ALLOC_N(info_ret, nagentinfo) < 0)
+        goto cleanup;
+
+    for (i = 0; i < nagentinfo; i++) {
+        if (!(info_ret[i] = qemuAgentFSInfoToPublic(agentinfo[i], vmdef)))
+            goto cleanup;
+    }
+
+    *info = g_steal_pointer(&info_ret);
+    ret = nagentinfo;
+
+ cleanup:
+    for (i = 0; i < nagentinfo; i++) {
+        qemuAgentFSInfoFree(agentinfo[i]);
+        /* if there was an error, free any memory we've allocated for the
+         * return value */
+        if (info_ret)
+            virDomainFSInfoFree(info_ret[i]);
+    }
+    VIR_FREE(info_ret);
+    return ret;
+}
 
 static int
 qemuDomainGetFSInfo(virDomainPtr dom,
@@ -21847,8 +21952,9 @@ qemuDomainGetFSInfo(virDomainPtr dom,
 {
     virQEMUDriverPtr driver = dom->conn->privateData;
     virDomainObjPtr vm;
-    qemuAgentPtr agent;
+    qemuAgentFSInfoPtr *agentinfo = NULL;
     int ret = -1;
+    int nfs;
 
     virCheckFlags(0, ret);
 
@@ -21858,25 +21964,22 @@ qemuDomainGetFSInfo(virDomainPtr dom,
     if (virDomainGetFSInfoEnsureACL(dom->conn, vm->def) < 0)
         goto cleanup;
 
-    if (qemuDomainObjBeginJobWithAgent(driver, vm,
-                                       QEMU_JOB_QUERY,
-                                       QEMU_AGENT_JOB_QUERY) < 0)
+    if ((nfs = qemuDomainGetFSInfoAgent(driver, vm, &agentinfo)) < 0)
+        goto cleanup;
+
+    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
         goto cleanup;
 
     if (virDomainObjCheckActive(vm) < 0)
         goto endjob;
 
-    if (!qemuDomainAgentAvailable(vm, true))
-        goto endjob;
-
-    agent = qemuDomainObjEnterAgent(vm);
-    ret = qemuAgentGetFSInfo(agent, info, vm->def);
-    qemuDomainObjExitAgent(vm, agent);
+    ret = virDomainFSInfoFormat(agentinfo, nfs, vm->def, info);
 
  endjob:
-    qemuDomainObjEndJobWithAgent(driver, vm);
+    qemuDomainObjEndJob(driver, vm);
 
  cleanup:
+    g_free(agentinfo);
     virDomainObjEndAPI(&vm);
     return ret;
 }
@@ -22882,6 +22985,103 @@ qemuDomainGetGuestInfoCheckSupport(unsigned int *types)
     *types = *types & supportedGuestInfoTypes;
 }
 
+/* Returns: 0 on success
+ *          -1 otherwise
+ */
+static int
+qemuAgentFSInfoFormatParams(qemuAgentFSInfoPtr *fsinfo,
+                            int nfs,
+                            virDomainDefPtr vmdef,
+                            virTypedParameterPtr *params,
+                            int *nparams, int *maxparams)
+{
+    int ret = -1;
+    size_t i, j;
+
+    /* FIXME: get disk target */
+
+    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];
+        g_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;
+        g_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;
+        g_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 */
+        g_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;
+
+        g_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;
+
+        g_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++) {
+            virDomainDiskDefPtr diskdef = NULL;
+            qemuAgentDiskInfoPtr d = fsinfo[i]->disks[j];
+            /* match the disk to the target in the vm definition */
+            diskdef = virDomainDiskByAddress(vmdef,
+                                             &d->pci_controller,
+                                             d->bus,
+                                             d->target,
+                                             d->unit);
+            if (diskdef) {
+                g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+                           "fs.%zu.disk.%zu.alias", i, j);
+                if (diskdef->dst &&
+                    virTypedParamsAddString(params, nparams, maxparams,
+                                            param_name, diskdef->dst) < 0)
+                    goto cleanup;
+            }
+
+            g_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;
+
+            g_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:
+    return ret;
+}
+
 static int
 qemuDomainGetGuestInfo(virDomainPtr dom,
                        unsigned int types,
@@ -22897,6 +23097,9 @@ qemuDomainGetGuestInfo(virDomainPtr dom,
     g_autofree char *hostname = NULL;
     unsigned int supportedTypes = types;
     int rc;
+    int nfs = 0;
+    qemuAgentFSInfoPtr *agentfsinfo = NULL;
+    size_t i;
 
     virCheckFlags(0, -1);
     qemuDomainGetGuestInfoCheckSupport(&supportedTypes);
@@ -22907,13 +23110,12 @@ qemuDomainGetGuestInfo(virDomainPtr dom,
     if (virDomainGetGuestInfoEnsureACL(dom->conn, vm->def) < 0)
         goto cleanup;
 
-    if (qemuDomainObjBeginJobWithAgent(driver, vm,
-                                       QEMU_JOB_QUERY,
-                                       QEMU_AGENT_JOB_QUERY) < 0)
+    if (qemuDomainObjBeginAgentJob(driver, vm,
+                                   QEMU_AGENT_JOB_QUERY) < 0)
         goto cleanup;
 
     if (!qemuDomainAgentAvailable(vm, true))
-        goto endjob;
+        goto endagentjob;
 
     agent = qemuDomainObjEnterAgent(vm);
 
@@ -22948,7 +23150,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom,
         }
     }
     if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) {
-        rc = qemuAgentGetFSInfoParams(agent, params, nparams, &maxparams, vm->def);
+        rc = nfs = qemuAgentGetFSInfo(agent, &agentfsinfo);
         if (rc < 0 && !(rc == -2 && types == 0))
             goto exitagent;
     }
@@ -22958,10 +23160,29 @@ qemuDomainGetGuestInfo(virDomainPtr dom,
  exitagent:
     qemuDomainObjExitAgent(vm, agent);
 
+ endagentjob:
+    qemuDomainObjEndAgentJob(vm);
+
+    if (nfs > 0) {
+        if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+            goto cleanup;
+
+        if (virDomainObjCheckActive(vm) < 0)
+            goto endjob;
+
+        /* we need to convert the agent fsinfo struct to parameters and match
+         * it to the vm disk target */
+        qemuAgentFSInfoFormatParams(agentfsinfo, nfs, vm->def, params, nparams, &maxparams);
+
  endjob:
-    qemuDomainObjEndJobWithAgent(driver, vm);
+        qemuDomainObjEndJob(driver, vm);
+    }
 
  cleanup:
+    for (i = 0; i < nfs; i++)
+        qemuAgentFSInfoFree(agentfsinfo[i]);
+    VIR_FREE(agentfsinfo);
+
     virDomainObjEndAPI(&vm);
     return ret;
 }
diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index 644dc9d08b..a45ce4f44a 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -247,14 +247,14 @@ testQemuAgentGetFSInfo(const void *data)
     virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
     qemuMonitorTestPtr test = NULL;
     virDomainDefPtr def = NULL;
-    virDomainFSInfoPtr *info = NULL;
+    qemuAgentFSInfoPtr *info = NULL;
     int ret = -1, ninfo = 0, i;
 
     if (testQemuAgentGetFSInfoCommon(xmlopt, &test, &def) < 0)
         goto cleanup;
 
     if ((ninfo = qemuAgentGetFSInfo(qemuMonitorTestGetAgent(test),
-                                    &info, def)) < 0)
+                                    &info)) < 0)
         goto cleanup;
 
     if (ninfo != 3) {
@@ -266,35 +266,48 @@ testQemuAgentGetFSInfo(const void *data)
     if (STRNEQ(info[2]->name, "sda1") ||
         STRNEQ(info[2]->mountpoint, "/") ||
         STRNEQ(info[2]->fstype, "ext4") ||
-        info[2]->ndevAlias != 1 ||
-        !info[2]->devAlias || !info[2]->devAlias[0] ||
-        STRNEQ(info[2]->devAlias[0], "hdc")) {
+        info[2]->ndisks != 1 ||
+        !info[2]->disks || !info[2]->disks[0]) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-            "unexpected filesystems information returned for sda1 (%s,%s)",
-            info[2]->name, info[2]->devAlias ? info[2]->devAlias[0] : "null");
+            "unexpected filesystems information returned for sda1 (%s)",
+            info[2]->name);
         ret = -1;
         goto cleanup;
     }
     if (STRNEQ(info[1]->name, "dm-1") ||
         STRNEQ(info[1]->mountpoint, "/opt") ||
         STRNEQ(info[1]->fstype, "vfat") ||
-        info[1]->ndevAlias != 2 ||
-        !info[1]->devAlias || !info[1]->devAlias[0] || !info[1]->devAlias[1] ||
-        STRNEQ(info[1]->devAlias[0], "vda") ||
-        STRNEQ(info[1]->devAlias[1], "vdb")) {
+        info[1]->ndisks != 2 ||
+        !info[1]->disks || !info[1]->disks[0] || !info[1]->disks[1] ||
+        STRNEQ(info[1]->disks[0]->bus_type, "virtio") ||
+        info[1]->disks[0]->bus != 0 ||
+        info[1]->disks[0]->target != 0 ||
+        info[1]->disks[0]->unit != 0 ||
+        info[1]->disks[0]->pci_controller.domain != 0 ||
+        info[1]->disks[0]->pci_controller.bus != 0 ||
+        info[1]->disks[0]->pci_controller.slot != 6 ||
+        info[1]->disks[0]->pci_controller.function != 0 ||
+        STRNEQ(info[1]->disks[1]->bus_type, "virtio") ||
+        info[1]->disks[1]->bus != 0 ||
+        info[1]->disks[1]->target != 0 ||
+        info[1]->disks[1]->unit != 0 ||
+        info[1]->disks[1]->pci_controller.domain != 0 ||
+        info[1]->disks[1]->pci_controller.bus != 0 ||
+        info[1]->disks[1]->pci_controller.slot != 7 ||
+        info[1]->disks[1]->pci_controller.function != 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-            "unexpected filesystems information returned for dm-1 (%s,%s)",
-            info[0]->name, info[0]->devAlias ? info[0]->devAlias[0] : "null");
+            "unexpected filesystems information returned for dm-1 (%s)",
+            info[0]->name);
         ret = -1;
         goto cleanup;
     }
     if (STRNEQ(info[0]->name, "sdb1") ||
         STRNEQ(info[0]->mountpoint, "/mnt/disk") ||
         STRNEQ(info[0]->fstype, "xfs") ||
-        info[0]->ndevAlias != 0 || info[0]->devAlias) {
+        info[0]->ndisks != 0 || info[0]->disks) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-            "unexpected filesystems information returned for sdb1 (%s,%s)",
-            info[0]->name, info[0]->devAlias ? info[0]->devAlias[0] : "null");
+            "unexpected filesystems information returned for sdb1 (%s)",
+            info[0]->name);
         ret = -1;
         goto cleanup;
     }
@@ -313,7 +326,7 @@ testQemuAgentGetFSInfo(const void *data)
                                "}") < 0)
         goto cleanup;
 
-    if (qemuAgentGetFSInfo(qemuMonitorTestGetAgent(test), &info, def) != -1) {
+    if (qemuAgentGetFSInfo(qemuMonitorTestGetAgent(test), &info) >= 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        "agent get-fsinfo command should have failed");
         goto cleanup;
@@ -323,159 +336,13 @@ testQemuAgentGetFSInfo(const void *data)
 
  cleanup:
     for (i = 0; i < ninfo; i++)
-        virDomainFSInfoFree(info[i]);
+        qemuAgentFSInfoFree(info[i]);
     VIR_FREE(info);
     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;
-    const char *name, *mountpoint, *fstype, *alias, *serial;
-    unsigned int diskcount;
-    unsigned long long bytesused, bytestotal;
-    const char *alias2;
-
-    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);
-        goto cleanup;
-    }
-
-    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);
-        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);
-        goto cleanup;
-    }
-
-    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);
-        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);
-        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);
-        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);
-        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) != -2) {
-        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;
-}
-
-
 static int
 testQemuAgentSuspend(const void *data)
 {
@@ -1438,7 +1305,6 @@ mymain(void)
     DO_TEST(FSFreeze);
     DO_TEST(FSThaw);
     DO_TEST(FSTrim);
-    DO_TEST(GetFSInfoParams);
     DO_TEST(GetFSInfo);
     DO_TEST(Suspend);
     DO_TEST(Shutdown);
-- 
2.25.0