[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] util: add qemu_write_pidfile()
From: |
Daniel P . Berrangé |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] util: add qemu_write_pidfile() |
Date: |
Mon, 3 Sep 2018 10:55:28 +0100 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
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 <address@hidden>
> ---
> 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é <address@hidden>
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 :|
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH 1/3] util: add qemu_write_pidfile(),
Daniel P . Berrangé <=