[ovs-dev,V9] windows: Added lockf function and lock PID file

If the PID file isn't locked then appctl.py detects it as stale and
bails out without doing anything. Because of this lots of Python tests fail.
Also this protects the PID file from being overwritten.

I used only shared lock, in order to be compatible with Python tests,
which try to acquire the lock exclusively. On Windows if the exclusive lock
is used, than the read access is denied too for other instances of this file.

Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com>
Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
---
V2: No changes
V3: No changes
V4: No changes
V5: Removed this patch from Python series.
    Removed unused constants and defined 0xFFFF0000 as constant.
    Updated commit text.
V6: flock returns now the error of GetLastError.
    Also it uses fd parameter instead of the global filep_pidfile.
    Removed unused local variable and unnecessary error message.
V7: Keep a shared lock on PID file after the fopen.
    This ensures us that at most one process can write the same PID file.
    fopen with 'w' on Windows creates a file with FILE_SHARED_WRITE
    flag, so anyone with write access on file can write it.
V8: Added LOCKFILE_FAIL_IMMEDIATELY flag when trying to get exclusive lock
    on PID file in order to avoid hangs of other daemons that try to acquire
    exclusive lock over the same PID file
V9: Small fatal error message update and on constants used.
    Lock only the first byte in PID file, this is enough in our case.
---
 lib/daemon-windows.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

On 25 July 2016 at 05:50, Paul Boca <pboca@cloudbasesolutions.com> wrote:

> If the PID file isn't locked then appctl.py detects it as stale and
> bails out without doing anything. Because of this lots of Python tests
> fail.
> Also this protects the PID file from being overwritten.
>
> I used only shared lock, in order to be compatible with Python tests,
> which try to acquire the lock exclusively. On Windows if the exclusive lock
> is used, than the read access is denied too for other instances of this
> file.
>
> Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com>
> Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
>

Thank you, applied!


> ---
> V2: No changes
> V3: No changes
> V4: No changes
> V5: Removed this patch from Python series.
>     Removed unused constants and defined 0xFFFF0000 as constant.
>     Updated commit text.
> V6: flock returns now the error of GetLastError.
>     Also it uses fd parameter instead of the global filep_pidfile.
>     Removed unused local variable and unnecessary error message.
> V7: Keep a shared lock on PID file after the fopen.
>     This ensures us that at most one process can write the same PID file.
>     fopen with 'w' on Windows creates a file with FILE_SHARED_WRITE
>     flag, so anyone with write access on file can write it.
> V8: Added LOCKFILE_FAIL_IMMEDIATELY flag when trying to get exclusive lock
>     on PID file in order to avoid hangs of other daemons that try to
> acquire
>     exclusive lock over the same PID file
> V9: Small fatal error message update and on constants used.
>     Lock only the first byte in PID file, this is enough in our case.
> ---
>  lib/daemon-windows.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/lib/daemon-windows.c b/lib/daemon-windows.c
> index 8cf0fea..d77724e 100644
> --- a/lib/daemon-windows.c
> +++ b/lib/daemon-windows.c
> @@ -18,6 +18,7 @@
>  #include "daemon.h"
>  #include "daemon-private.h"
>  #include <stdio.h>
> +#include <io.h>
>  #include <stdlib.h>
>  #include "dirs.h"
>  #include "ovs-thread.h"
> @@ -26,6 +27,10 @@
>
>  VLOG_DEFINE_THIS_MODULE(daemon_windows);
>
> +/* Constants for flock function */
> +#define        LOCK_SHARED     0x0                     /* Shared lock. */
> +#define        LOCK_UNLOCK     0x80000000              /* Unlock. Custom
> value. */
> +
>  static bool service_create;          /* Was --service specified? */
>  static bool service_started;         /* Have we dispatched service to
> start? */
>
> @@ -404,9 +409,39 @@ detach_process(int argc, char *argv[])
>  }
>
>  static void
> +flock(FILE* fd, int operation)
> +{
> +    HANDLE hFile;
> +    OVERLAPPED ov = {0};
> +
> +    hFile = (HANDLE)_get_osfhandle(fileno(fd));
> +    if (hFile == INVALID_HANDLE_VALUE) {
> +        VLOG_FATAL("Failed to get PID file handle (%s).",
> +                   ovs_lasterror_to_string());
> +    }
> +
> +    if (operation & LOCK_UNLOCK) {
> +        if (UnlockFileEx(hFile, 0, 1, 0, &ov) == 0) {
> +            VLOG_FATAL("Failed to unlock PID file (%s).",
> +                       ovs_lasterror_to_string());
> +        }
> +    } else {
> +       /* Use LOCKFILE_FAIL_IMMEDIATELY flag to avoid hang of another
> daemon that tries to
> +           acquire exclusive lock over the same PID file */
> +        if (LockFileEx(hFile, operation | LOCKFILE_FAIL_IMMEDIATELY,
> +                       0, 1, 0, &ov) == FALSE) {
> +            VLOG_FATAL("Failed to lock PID file (%s).",
> +                       ovs_lasterror_to_string());
> +        }
> +    }
> +}
> +
> +static void
>  unlink_pidfile(void)
>  {
>      if (filep_pidfile) {
> +        /* Remove the shared lock on file */
> +        flock(filep_pidfile, LOCK_UNLOCK);
>          fclose(filep_pidfile);
>      }
>      if (pidfile) {
> @@ -437,6 +472,8 @@ make_pidfile(void)
>          VLOG_FATAL("failed to open %s (%s)", pidfile,
> ovs_strerror(errno));
>      }
>
> +    flock(filep_pidfile, LOCKFILE_EXCLUSIVE_LOCK);
> +
>      fatal_signal_add_hook(unlink_pidfile, NULL, NULL, true);
>
>      fprintf(filep_pidfile, "%d\n", _getpid());
> @@ -444,6 +481,10 @@ make_pidfile(void)
>          VLOG_FATAL("Failed to write into the pidfile %s", pidfile);
>      }
>
> +    flock(filep_pidfile, LOCK_SHARED);
> +    /* This will remove the exclusive lock. The shared lock will remain */
> +    flock(filep_pidfile, LOCK_UNLOCK);
> +
>      /* Don't close the pidfile till the process exits. */
>  }
>
> --
> 2.7.2.windows.1
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>