[libvirt] [PATCH 1/2] Implementation deficiency in virInitctlSetRunLevel v3

Daniel P. Berrange berrange at redhat.com
Fri Dec 20 15:57:30 UTC 2013


On Fri, Dec 20, 2013 at 07:39:21PM +0400, Reco wrote:
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index 9fc3207..2e8535e 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -31,6 +31,7 @@
>  # include <sys/resource.h>
>  #endif
>  #include <sched.h>
> +#include <stdlib.h>
>  
>  #ifdef __FreeBSD__
>  # include <sys/param.h>
> @@ -847,3 +848,65 @@ int virProcessGetStartTime(pid_t pid,
>      return 0;
>  }
>  #endif
> +
> +#ifdef HAVE_SETNS
> +int virProcessRunInMountNamespace(pid_t pid,
> +                                  virProcessNamespaceCallback cb,
> +                                  void *opaque)
> +{
> +    char* path = NULL;
> +    int ret = -1;
> +    int cpid = -1;
> +    int status = -1;
> +    int fd = -1;
> +
> +    if (virAsprintf(&path, "/proc/%llu/ns/mnt",
> +                    (unsigned long long)pid) < 0) {
> +        goto cleanup;
> +    }
> +
> +    if ((fd = open(path, O_RDONLY)) < 0) {
> +        virReportSystemError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",

The virReportSystemError method wants to be given 'errno' value here.

> +                             _("Kernel does not provide mount namespace"));
> +        goto cleanup;
> +    }
> +
> +    switch (cpid = fork()) {
> +    case 0:
> +        if (setns(fd, 0) == -1) {
> +            exit(-1);

I think we need  _exit instead of exit

> +        }
> +
> +        ret = cb(pid, opaque);
> +        exit(ret);

And again here.

> +        break;
> +    case -1:
> +        virReportSystemError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                             _("Fork failed"));
> +        goto cleanup;
> +    default:
> +        if (waitpid(cpid, &status, 0) < 0 || status < 0) {
> +            virReportSystemError(errno,
> +                                 _("Callback failed with status %i"),
> +                                 status);
> +            ret = 1;
> +        } else {
> +            ret = 0;
> +        }

You'll want to use virProcessWait(cpiud, &status) here instead
since to avoid tedious waitpid calling conventions

> +    }
> +
> +cleanup:
> +    VIR_FREE(path);
> +    VIR_FORCE_CLOSE(fd);
> +    return ret;
> +}
> +#else
> +int virProcessRunInMountNamespace(pid_t pid ATTRIBUTE_UNUSED,
> +                                  virProcessNamespaceCallback cb ATTRIBUTE_UNUSED,
> +                                  void *opaque ATTRIBUTE_UNUSED)
> +{
> +    virReportSystemError(ENOSYS, "%s",
> +                         _("Mount namespaces are not available on this platform"));
> +    return -1;
> +}
> +#endif


Looks good aside from those small tweaks.

BTW, thanks very much for taking the time to write these patches for us,
as well as reporting the issue in the first place !

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




More information about the libvir-list mailing list