[Libvir] [patch 6/9] Move iptablesSpawn()

[Daniel Veillard]

On Sat, Jan 05, 2008 at 12:11:04AM +0000, Daniel P. Berrange wrote:
> On Fri, Jan 04, 2008 at 03:57:31PM +0000, Mark McLoughlin wrote:
> > The next patch requires iptablesSpawn() higher up in
> > the file. This patch just moves the code around; there
> > is no functional change.
> 
> The util.c file has a virExec function, although its a little more
> cumbersome to use than iptablesSpawn. So for the storage APIs I've
> got a new virRun() function in util.c which is very similar to your
> iptablesSpawn code. How about we just make iptables use the virRun
> method in the attached patch ?
> 
> 
> diff -r a0ae4f87480b src/util.c
> --- a/src/util.c	Thu Dec 06 15:35:43 2007 -0500
> +++ b/src/util.c	Thu Dec 06 15:39:34 2007 -0500
> @@ -34,6 +34,7 @@
>  #include <errno.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
> +#include <sys/wait.h>
>  #include <string.h>
>  
>  #include "libvirt/virterror.h"
> @@ -196,6 +197,43 @@ virExecNonBlock(virConnectPtr conn,
>            int *retpid, int infd, int *outfd, int *errfd) {
>  
>      return(_virExec(conn, argv, retpid, infd, outfd, errfd, 1));
> +}
> +
> +/**
> + * @conn connection to report errors against
> + * @argv NULL terminated argv to run
> + * @status optional variable to return exit status in
> + *
> + * Run a command without using the shell.
> + *
> + * If status is NULL, then return 0 if the command run and
> + * exited with 0 status; Otherwise return -1
> + *
> + * If status is not-NULL, then return 0 if the command ran.
> + * The status variable is filled with the command exit status
> + * and should be checked by caller for success. Return -1
> + * only if the command could not be run.
> + */
> +int
> +virRun(virConnectPtr conn,
> +       char **argv,
> +       int *status) {
> +    int childpid, exitstatus, ret;
> +
> +    if ((ret = virExec(conn, argv, &childpid, -1, NULL, NULL)) < 0)
> +        return ret;
> +
> +    while ((ret = waitpid(childpid, &exitstatus, 0) == -1) && errno == EINTR);
> +    if (ret == -1)
> +        return -1;
> +
> +    if (status == NULL) {
> +        errno = EINVAL;
> +        return (WIFEXITED(exitstatus) && WEXITSTATUS(exitstatus) == 0) ? 0 : -1;
> +    } else {
> +        *status = exitstatus;
> +        return 0;
> +    }
>  }
>  
>  /* Like read(), but restarts after EINTR */
> diff -r a0ae4f87480b src/util.h
> --- a/src/util.h	Thu Dec 06 15:35:43 2007 -0500
> +++ b/src/util.h	Thu Dec 06 15:39:34 2007 -0500
> @@ -28,6 +28,7 @@
>  
>  int virExec(virConnectPtr conn, char **argv, int *retpid, int infd, int *outfd, int *errfd);
>  int virExecNonBlock(virConnectPtr conn, char **argv, int *retpid, int infd, int *outfd, int *errfd);
> +int virRun(virConnectPtr conn, char **argv, int *status);
>  
>  int saferead(int fd, void *buf, size_t count);
>  ssize_t safewrite(int fd, const void *buf, size_t count);
> 

 Looks fine to me, if this can be used now, no need to wait for the other
parts of the storage patch, push it now. In general util stuff is best
pushed earlier than later precisely to try to share as much as possible.

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/