virStorageBackendGetMaps() returns 1 on failure instead of the conventional -1, and does not call virReportError() in any of its error paths.
On top of that, both callers silently discard the return value: virStorageBackendGetMaps() ignores the result of virStorageBackendCreateVols(), and virStorageBackendMpathRefreshPool() ignores the result of virStorageBackendGetMaps(). Fix all three issues: normalize error returns to -1, add virReportError() calls for each failure path in virStorageBackendGetMaps(), and check return values in both callers so that errors are properly propagated. Signed-off-by: Lucas Amaral <[email protected]> --- This is similar in spirit to commits e9b931d3e4 ("virpci: Report an error if virPCIGetVirtualFunctionIndex() fails") and 6c2c9e21ac ("virstorageobj: Make virStoragePoolObjAddVol() report an error on failure"), applying the same pattern to the mpath storage backend. Found while auditing storage backends for functions that return error codes without calling virReportError(). src/storage/storage_backend_mpath.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 9fc3983c52..a30153a93b 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -197,19 +197,25 @@ virStorageBackendGetMaps(virStoragePoolObj *pool) struct dm_names *names = NULL; if (!(dmt = dm_task_create(DM_DEVICE_LIST))) { - retval = 1; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to create devmapper task")); + retval = -1; goto out; } dm_task_no_open_count(dmt); if (!dm_task_run(dmt)) { - retval = 1; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to run devmapper task")); + retval = -1; goto out; } if (!(names = dm_task_get_names(dmt))) { - retval = 1; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to get devmapper device names")); + retval = -1; goto out; } @@ -218,7 +224,10 @@ virStorageBackendGetMaps(virStoragePoolObj *pool) goto out; } - virStorageBackendCreateVols(pool, names); + if (virStorageBackendCreateVols(pool, names) < 0) { + retval = -1; + goto out; + } out: if (dmt != NULL) @@ -248,7 +257,8 @@ virStorageBackendMpathRefreshPool(virStoragePoolObj *pool) virWaitForDevices(); - virStorageBackendGetMaps(pool); + if (virStorageBackendGetMaps(pool) < 0) + return -1; return 0; } -- 2.52.0
