From: Eric Blake <eblake@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Florian Larysch <fl@n621.de>, qemu-devel@nongnu.org, sw@weilnetz.de
Subject: Re: [Qemu-devel] [PATCH for-2.12] os: truncate pidfile on creation
Date: Tue, 20 Mar 2018 11:07:06 -0500 [thread overview]
Message-ID: <8982638a-c7c0-3da8-4d69-96e5b45fbdfa@redhat.com> (raw)
In-Reply-To: <20180320160243.GZ4530@redhat.com>
On 03/20/2018 11:02 AM, Daniel P. Berrangé wrote:
> On Tue, Mar 20, 2018 at 09:50:19AM -0500, Eric Blake wrote:
>> On 03/20/2018 04:31 AM, Florian Larysch wrote:
>>> qemu_create_pidfile does not truncate the pidfile when it creates it,
>>> but rather overwrites its contents with the new pid. This works fine as
>>> long as the length of the pid doesn't decrease, but this might happen in
>>> case of wraparounds, causing pidfiles to contain trailing garbage which
>>> breaks operations such as 'kill $(cat pidfile)'.
>>
>> Ouch. Good analysis.
>>
>> 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?
>>
>> This part is right, if we don't care about the race with someone reading an
>> empty file.
>
> No, it is unsafe - we rely on lockf() to get the mutual exclusion.
Oh, right, because we aren't using O_EXCL.
> If a QEMU is running with pidfile locked, and its pid written into
> it, then a 2nd QEMU comes along it will truncate the pidfile owned
> by the original QEMU because the truncation happens before it has
> tried to acquire the lock. The 2nd QEMU will still exit, but the
> original QEMU's pid has now been lost.
>
> We must call ftruncate() after lockf(), but before writing the new
> pid into the file. That ensures there is no window in which it is
> possible to see the new & old pids mixed together.
So the window of time where the file can be empty still exists, but it
must be AFTER the lockf() (and the window where reading the file content
can see the wrong pid is longer - from the open until the ftruncate -
but at least the wrong data is limited to a previous pid or the empty
string, rather than a completely random wrong value from squashing
together two unrelated pids).
We'll need a v2.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
next prev parent reply other threads:[~2018-03-20 16:07 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-20 9:31 [Qemu-devel] [PATCH] os: truncate pidfile on creation Florian Larysch
2018-03-20 14:50 ` [Qemu-devel] [PATCH for-2.12] " Eric Blake
2018-03-20 15:49 ` Florian Larysch
2018-03-20 16:02 ` Daniel P. Berrangé
2018-03-20 16:07 ` Eric Blake [this message]
2018-03-20 16:21 ` Florian Larysch
2018-03-20 16:29 ` Daniel P. Berrangé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8982638a-c7c0-3da8-4d69-96e5b45fbdfa@redhat.com \
--to=eblake@redhat.com \
--cc=berrange@redhat.com \
--cc=fl@n621.de \
--cc=qemu-devel@nongnu.org \
--cc=sw@weilnetz.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).