> -----Mesaj original-----
> De la: dev [mailto:dev-boun...@openvswitch.org] În numele Paul Boca
> Trimis: Friday, July 1, 2016 7:28 PM
> Către: dev@openvswitch.org
> Subiect: [ovs-dev] [PATCH V5] 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 <pb...@cloudbasesolutions.com>
> +static int
[Alin Gabriel Serdean: ] I don't really understand why 'int' when you are 
returning -1/0. The problem is: you need to deal with GetLastError and errno if 
a specific function failed. 
> +flock(FILE* fd, int operation)
> +{
> +    NTSTATUS status;
> +    HANDLE hFile;
> +    OVERLAPPED ov = {0};
> +
> +    hFile = (HANDLE)_get_osfhandle(fileno(filep_pidfile));
[Alin Gabriel Serdean: ] either use fd or drop the parameter fd.
> +    if (hFile == INVALID_HANDLE_VALUE) {
> +        VLOG_FATAL("Invalid handle value");
> +        return -1;
> +    }
> +
> +    if (operation & LOCK_UN) {
> +        if (UnlockFileEx(hFile, 0, 0, LOCK_MAX_SIZE, &ov) == FALSE) {
[Alin Gabriel Serdean: ] According to the docs UnlockFileEx can be 0 or NULL
https://msdn.microsoft.com/en-us/library/windows/desktop/aa365716(v=vs.85).aspx

> +            return -1;
> +        }
> +    } else {
> +        if (LockFileEx(hFile, operation, 0, 0, LOCK_MAX_SIZE, &ov) == FALSE) 
> {
> +            VLOG_FATAL("LockFileEx failed, status = 0x%08x\n", status);
> +            return -1;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  /* If a pidfile has been configured, creates it and stores the running
>   * process's pid in it.  Ensures that the pidfile will be deleted when the
>   * process exits. */
> @@ -444,6 +479,14 @@ make_pidfile(void)
>          VLOG_FATAL("Failed to write into the pidfile %s", pidfile);
>      }
> 
[Alin Gabriel Serdean: ] Unless I am missing something: there is another call 
to fflush just above, do we need to flush the buffers again?
> +    fflush(filep_pidfile);
> +    error = flock(filep_pidfile, LOCK_SH);
> +    if (error) {
> +        /* Looks like we failed to acquire the lock. */
> +        VLOG_FATAL("%s: fcntl(F_SETLK) failed (%s)", pidfile,
> +                   ovs_strerror(error));
> +    }
> +
>      /* 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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to