> -----Mesaj original-----
> De la: dev [mailto:[email protected]] În numele Paul Boca
> Trimis: Friday, July 1, 2016 7:28 PM
> Către: [email protected]
> 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 <[email protected]>
> +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
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
