Re: [libvirt] [PATCH 08/16] storage: iscsi: Convert virExec to virCommand

["Daniel P. Berrange" <berrange redhat com>]

On Tue, May 10, 2011 at 04:07:47PM -0400, Cole Robinson wrote:
> 
> Signed-off-by: Cole Robinson <crobinso redhat com>
> ---
>  src/storage/storage_backend_iscsi.c |   24 ++++++++++++------------
>  1 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c
> index ec52dbc..b489394 100644
> --- a/src/storage/storage_backend_iscsi.c
> +++ b/src/storage/storage_backend_iscsi.c
> @@ -184,28 +184,24 @@ virStorageBackendIQNFound(const char *initiatoriqn,
>      int ret = IQN_MISSING, fd = -1;
>      char ebuf[64];
>      FILE *fp = NULL;
> -    pid_t child = 0;
>      char *line = NULL, *newline = NULL, *iqn = NULL, *token = NULL,
>          *saveptr = NULL;
> -    const char *const prog[] = {
> -        ISCSIADM, "--mode", "iface", NULL
> -    };
> +    virCommandPtr cmd = virCommandNewArgList(ISCSIADM,
> +                                             "--mode", "iface", NULL);
> +    pid_t pid = -1;
>  
>      if (VIR_ALLOC_N(line, LINE_SIZE) != 0) {
>          ret = IQN_ERROR;
>          virStorageReportError(VIR_ERR_INTERNAL_ERROR,
>                                _("Could not allocate memory for output of '%s'"),
> -                              prog[0]);
> +                              ISCSIADM);
>          goto out;
>      }
>  
>      memset(line, 0, LINE_SIZE);
>  
> -    if (virExec(prog, NULL, NULL, &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0) {
> -        virStorageReportError(VIR_ERR_INTERNAL_ERROR,
> -                              _("Failed to run '%s' when looking for existing interface with IQN '%s'"),
> -                              prog[0], initiatoriqn);
> -
> +    virCommandSetOutputFD(cmd, &fd);
> +    if (virCommandRunAsync(cmd, &pid) < 0) {
>          ret = IQN_ERROR;
>          goto out;
>      }
> @@ -214,7 +210,7 @@ virStorageBackendIQNFound(const char *initiatoriqn,
>          virStorageReportError(VIR_ERR_INTERNAL_ERROR,
>                                _("Failed to open stream for file descriptor "
>                                  "when reading output from '%s': '%s'"),
> -                              prog[0], virStrerror(errno, ebuf, sizeof ebuf));
> +                              ISCSIADM, virStrerror(errno, ebuf, sizeof ebuf));
>          ret = IQN_ERROR;
>          goto out;
>      }
> @@ -226,7 +222,7 @@ virStorageBackendIQNFound(const char *initiatoriqn,
>              virStorageReportError(VIR_ERR_INTERNAL_ERROR,
>                                    _("Unexpected line > %d characters "
>                                      "when parsing output of '%s'"),
> -                                  LINE_SIZE, prog[0]);
> +                                  LINE_SIZE, ISCSIADM);
>              goto out;
>          }
>          *newline = '\0';
> @@ -256,9 +252,13 @@ out:
>          VIR_DEBUG("Could not find interface with IQN '%s'", iqn);
>      }
>  
> +    if (pid > 0 && virCommandWait(cmd, NULL) < 0)
> +        ret = IQN_ERROR;
> +
>      VIR_FREE(line);
>      VIR_FORCE_FCLOSE(fp);
>      VIR_FORCE_CLOSE(fd);
> +    virCommandFree(cmd);
>  
>      return ret;
>  }

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 :|