On Thu, Mar 01, 2018 at 04:42:36PM -0700, Jim Fehlig wrote:
> Locks held by virtlockd are dropped on re-exec.
> 
> virtlockd       94306 POSIX 5.4G WRITE 0     0   0 /tmp/test.qcow2
> virtlockd       94306 POSIX   5B WRITE 0     0   0 /run/virtlockd.pid
> virtlockd       94306 POSIX   5B WRITE 0     0   0 /run/virtlockd.pid
> 
> Acquire locks in PostExecRestart code path.

This is really strange and should *not* be happening.  POSIX locks
are supposed to be preserved across execve() if the FD has CLOEXEC
unset, and you don't fork() before the exec.

eg see this demo program:

#include <stdio.h>
#include <fcntl.h>
#include <stdlib.h>
#include <pthread.h>
#include <unistd.h>


int main(int argc, char **argv) {

  if (argc == 2) {
    int fd = atoi(argv[1]);

    struct flock fl = {
      .l_type = F_WRLCK,
      .l_whence = SEEK_SET,
      .l_start = 0,
      .l_len = 42,
    };

    if (fcntl(fd, F_GETLK, &fl) < 0)
      abort();

    int flags;
    if ((flags = fcntl(fd, F_GETFD)) < 0)
      abort();
    flags |= FD_CLOEXEC;

    if (fcntl(fd, F_SETFD, flags) < 0)
      abort();

    fprintf(stderr, "Owned %d\n", fl.l_pid);
    fprintf(stderr, "Execd\n");
    sleep(50);

  } else {
    int fd = open("lock.txt", O_WRONLY|O_CREAT|O_TRUNC, 0755);
    if (fd < 0)
      abort();

    struct flock fl = {
      .l_type = F_WRLCK,
      .l_whence = SEEK_SET,
      .l_start = 0,
      .l_len = 42,
    };

    if (fcntl(fd, F_SETLK, &fl) < 0)
      abort();

    int flags;

    if ((flags = fcntl(fd, F_GETFD)) < 0)
      abort();
    flags &= ~FD_CLOEXEC;

    if (fcntl(fd, F_SETFD, flags) < 0)
      abort();

    char fdstr[100];
    snprintf(fdstr, sizeof(fdstr), "%d", fd);

    char *newargv[] = { argv[0], fdstr, NULL };

    fprintf(stderr, "Waiting\n");
    sleep(10);
    execve(argv[0], newargv, NULL);
  }
}



If you run this, you'll see the lock still exists after execveI().

So I wonder what we've screwed up to cause the locks to get
released - reaquiring them definitely isn't desirable as we
should not loose them in the first place !

> 
> Signed-off-by: Jim Fehlig <jfeh...@suse.com>
> ---
> 
> The CLOEXEC flag is set in virLockSpaceNewPostExecRestart(), so I assume
> it is fine to call virFileLock() here as well.
> 
>  src/util/virlockspace.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
> index 41af0cdb6..420878b0a 100644
> --- a/src/util/virlockspace.c
> +++ b/src/util/virlockspace.c
> @@ -337,6 +337,7 @@ virLockSpacePtr 
> virLockSpaceNewPostExecRestart(virJSONValuePtr object)
>          virJSONValuePtr owners;
>          size_t j;
>          ssize_t m;
> +        bool shared = false;
>  
>          if (VIR_ALLOC(res) < 0)
>              goto error;
> @@ -389,6 +390,21 @@ virLockSpacePtr 
> virLockSpaceNewPostExecRestart(virJSONValuePtr object)
>              goto error;
>          }
>  
> +        shared = !!(res->flags & VIR_LOCK_SPACE_ACQUIRE_SHARED);
> +        if (virFileLock(res->fd, shared, 0, 1, false) < 0) {
> +            if (errno == EACCES || errno == EAGAIN) {
> +                virReportError(VIR_ERR_RESOURCE_BUSY,
> +                               _("Lockspace resource '%s' is locked"),
> +                               res->name);
> +            } else {
> +                virReportSystemError(errno,
> +                                     _("Unable to acquire lock on '%s'"),
> +                                     res->path);
> +            }
> +            virLockSpaceResourceFree(res);
> +            goto error;
> +        }
> +
>          if (!(owners = virJSONValueObjectGet(child, "owners"))) {
>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                             _("Missing resource owners in JSON document"));
> -- 
> 2.16.2
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to