Re: [libvirt] [PATCH 15/16] storage_backend: Fix error reporting with regex helper

Some clients overwrite the error from the regex helper, or do half-baked error reporting with the exitstatus. Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; --- src/storage/storage_backend.c | 11 +++++------ src/storage/storage_backend.h | 3 +-- src/storage/storage_backend_fs.c | 5 ++--- src/storage/storage_backend_iscsi.c | 16 ++-------------- src/storage/storage_backend_logical.c | 30 +++++------------------------- 5 files changed, 15 insertions(+), 50 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 5f9ff8f..2da1a15 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1360,8 +1360,7 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, const char **regex, int *nvars, virStorageBackendListVolRegexFunc func, - void *data, - int *outexit) + void *data) { int fd = -1, err, ret = -1; FILE *list = NULL; @@ -1468,7 +1467,7 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, ret = 0; cleanup: - if (pid > 0 && virCommandWait(cmd, outexit) < 0) + if (pid > 0 && virCommandWait(cmd, NULL) < 0) ret = -1; if (groups) { @@ -1600,10 +1599,10 @@ virStorageBackendRunProgRegex(virConnectPtr conn, const char **regex ATTRIBUTE_UNUSED, int *nvars ATTRIBUTE_UNUSED, virStorageBackendListVolRegexFunc func ATTRIBUTE_UNUSED, - void *data ATTRIBUTE_UNUSED, - int *outexit ATTRIBUTE_UNUSED) + void *data ATTRIBUTE_UNUSED) { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, _("%s not implemented on Win32"), __FUNCTION__); + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + _("%s not implemented on Win32"), __FUNCTION__); return -1; } diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 65cbd7e..fcfbed0 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -135,8 +135,7 @@ int virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, const char **regex, int *nvars, virStorageBackendListVolRegexFunc func, - void *data, - int *exitstatus); + void *data); int virStorageBackendRunProgNul(virStoragePoolObjPtr pool, const char **prog, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index d940055..88f5b87 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -233,7 +233,6 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE }; const char *prog[] = { SHOWMOUNT, "--no-headers", "--exports", NULL, NULL }; virStoragePoolSourcePtr source = NULL; - int exitstatus; char *retval = NULL; unsigned int i; @@ -246,8 +245,8 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE prog[3] = source->host.name; if (virStorageBackendRunProgRegex(NULL, prog, 1, regexes, vars, - virStorageBackendFileSystemNetFindPoolSourcesFunc, - &state, &exitstatus) < 0) + virStorageBackendFileSystemNetFindPoolSourcesFunc, + &state) < 0) goto cleanup; retval = virStoragePoolSourceListFormat(&state.list); diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index b489394..f5da8d8 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -160,8 +160,7 @@ virStorageBackendISCSISession(virStoragePoolObjPtr pool, regexes, vars, virStorageBackendISCSIExtractSession, - &session, - NULL) < 0) + &session) < 0) return NULL; if (session == NULL && @@ -504,7 +503,6 @@ virStorageBackendISCSIScanTargets(const char *portal, }; struct virStorageBackendISCSITargetList list; int i; - int exitstatus; memset(&list, 0, sizeof(list)); @@ -514,17 +512,7 @@ virStorageBackendISCSIScanTargets(const char *portal, regexes, vars, virStorageBackendISCSIGetTargets, - &list, - &exitstatus) < 0) { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("iscsiadm command failed")); - return -1; - } - - if (exitstatus != 0) { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - _("iscsiadm sendtargets command failed with exitstatus %d"), - exitstatus); + &list) < 0) { return -1; } diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index c18cd57..fdee2bd 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -205,25 +205,13 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, pool->def->source.name, NULL }; - int exitstatus; - if (virStorageBackendRunProgRegex(pool, prog, 1, regexes, vars, virStorageBackendLogicalMakeVol, - vol, - &exitstatus) < 0) { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("lvs command failed")); - return -1; - } - - if (exitstatus != 0) { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - _("lvs command failed with exitstatus %d"), - exitstatus); + vol) < 0) { return -1; } @@ -321,7 +309,6 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, }; const char *const prog[] = { PVS, "--noheadings", "-o", "pv_name,vg_name", NULL }; const char *const scanprog[] = { VGSCAN, NULL }; - int exitstatus; char *retval = NULL; virStoragePoolSourceList sourceList; int i; @@ -331,7 +318,7 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, * that might be hanging around, so if this fails for some reason, the * worst that happens is that scanning doesn't pick everything up */ - if (virRun(scanprog, &exitstatus) < 0) { + if (virRun(scanprog, NULL) < 0) { VIR_WARN0("Failure when running vgscan to refresh physical volumes"); } @@ -339,8 +326,8 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, sourceList.type = VIR_STORAGE_POOL_LOGICAL; if (virStorageBackendRunProgRegex(NULL, prog, 1, regexes, vars, - virStorageBackendLogicalFindPoolSourcesFunc, - &sourceList, &exitstatus) < 0) + virStorageBackendLogicalFindPoolSourcesFunc, + &sourceList) < 0) return NULL; retval = virStoragePoolSourceListFormat(&sourceList); @@ -489,7 +476,6 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, "--nosuffix", "--options", "vg_size,vg_free", pool->def->source.name, NULL }; - int exitstatus; virFileWaitForDevices(); @@ -506,13 +492,7 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, regexes, vars, virStorageBackendLogicalRefreshPoolFunc, - NULL, - &exitstatus) < 0) { - virStoragePoolObjClearVols(pool); - return -1; - } - - if (exitstatus != 0) { + NULL) < 0) { virStoragePoolObjClearVols(pool); return -1; }