On 2/23/26 02:44, Lucas Amaral wrote: > 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; > }
How about instead of overwriting retval in every error path, let's just init the retval to -1, and then just set it to 0 in the single success path that jumps onto the label (no devices found). Michal
