[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Tue, May 10, 2011 at 04:07:54PM -0400, Cole Robinson wrote:
> Some clients overwrite the error from the regex helper, or do half-baked
> error reporting with the exitstatus.
> 
> Signed-off-by: Cole Robinson <crobinso redhat com>
> ---
>  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;
>      }

ACK


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]