On Tue, Mar 20, 2018 at 09:50:19AM -0500, Eric Blake wrote: > However, I have to wonder if we have the opposite problem - when the > file exists (truncated) but has not yet been written, how often do > we have a race where someone can see the empty file? > > Should we be going even further and writing the pid into a temporary > file and then rename(2)ing that file onto the real destination, so > that if the pid file exists at all, it has stable contents that > can't cause confusion?
I thought about doing that, but the window of opportunity is sufficiently small that this probably not a problem. Furthermore, the second purpose of the pid file is to provide mutual exclusion between qemu instances using the same pid file by means of the lockf() call. Given that the lock is associated with the inode, doing a rename() would defeat this if we didn't reopen the file before calling lockf(). However, doing that would in turn allow a race between multiple qemu instances (possibly resulting in the incorrect pid remaining in the file). That could also be fixed, but makes the whole affair even more complicated; not sure if this is worth the benefits. > >diff --git a/os-win32.c b/os-win32.c > >index 586a7c7d49..85dbad7af8 100644 > >--- a/os-win32.c > >+++ b/os-win32.c > >@@ -108,7 +108,7 @@ int qemu_create_pidfile(const char *filename) > > memset(&overlap, 0, sizeof(overlap)); > > file = CreateFile(filename, GENERIC_WRITE, FILE_SHARE_READ, NULL, > >- OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); > >+ CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); > > I assume this is right (at least for looking comparable to the POSIX > case), although I'm trusting you here. I adapted this for consistency reasons, but I don't have a Windows platform available to test on. I based this on [1] and [2], which seems reasonable, but I'd be happy if someone who is familiar with the Windows API could ack this (get_maintainer.pl suggested Stefan Weil, who is CC'ed). [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa363858(v=vs.85).aspx [2] https://stackoverflow.com/questions/14469607/difference-between-open-always-and-create-always-in-createfile-of-windows-api Florian
