On Fri, Aug 31, 2018 at 04:53:12PM +0200, Marc-André Lureau wrote: > There are variants of qemu_create_pidfile() in qemu-pr-helper and > qemu-ga. Let's have a common implementation in libqemuutil. > > The code is initially based from pr-helper write_pidfile(), with > various improvements and suggestions from Daniel Berrangé: > > QEMU will leave the pidfile existing on disk when it exits which > initially made me think it avoids the deletion race. The app > managing QEMU, however, may well delete the pidfile after it has > seen QEMU exit, and even if the app locks the pidfile before > deleting it, there is still a race. > > eg consider the following sequence > > QEMU 1 libvirtd QEMU 2 > > 1. lock(pidfile) > > 2. exit() > > 3. open(pidfile) > > 4. lock(pidfile) > > 5. open(pidfile) > > 6. unlink(pidfile) > > 7. close(pidfile) > > 8. lock(pidfile) > > IOW, at step 8 the new QEMU has successfully acquired the lock, but > the pidfile no longer exists on disk because it was deleted after > the original QEMU exited. > > While we could just say no external app should ever delete the > pidfile, I don't think that is satisfactory as people don't read > docs, and admins don't like stale pidfiles being left around on > disk. > > To make this robust, I think we might want to copy libvirt's > approach to pidfile acquisition which runs in a loop and checks that > the file on disk /after/ acquiring the lock matches the file that > was locked. Then we could in fact safely let QEMU delete its own > pidfiles on clean exit.. > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > --- > include/qemu/osdep.h | 3 +- > os-posix.c | 24 --------------- > os-win32.c | 25 ---------------- > qga/main.c | 54 +++++++--------------------------- > scsi/qemu-pr-helper.c | 40 ++++--------------------- > util/oslib-posix.c | 68 +++++++++++++++++++++++++++++++++++++++++++ > util/oslib-win32.c | 27 +++++++++++++++++ > vl.c | 4 +-- > 8 files changed, 114 insertions(+), 131 deletions(-)
Reviewed-by: Daniel P. Berrangé <berra...@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|