> -----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