|
From: | Eric Blake |
Subject: | Re: [Qemu-devel] [PATCH v3] os: truncate pidfile on creation |
Date: | Tue, 20 Mar 2018 13:00:40 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
On 03/20/2018 12:33 PM, 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)'. Instead, always truncate the file before writing it. Note that the order is important here: We cannot simply use O_TRUNC in the open() call because another qemu process might truncate the pidfile of a process that is still running before reaching the lockf() barrier. The Windows version suffers from a similar problem, but as it does not provide effective mutual exclusion anyway (because the file handle is closed immediately after writing to it), adopting this behavior still seems to be an improvement, as it at least prevents garbled pidfiles. Signed-off-by: Florian Larysch <address@hidden> ---
Here after the --- is a nice place to summarize how v3 differs from v2, to save reviewers some time. In this particular case, it looks like all you did was correct a commit message typo in v2; at which point, since I gave Reviewed-by on v2, you could have pasted that into your amended v3 commit message to make it obvious that you didn't change the code. At any rate, my R-b still stands.
Also, if all that is wrong is a typo in the commit message, a maintainer is often willing to fix that up without you having to send a new revision, especially since the maintainer has to edit the commit message anyways to add their Signed-off-by. But you are also correct that a new revision CAN save a maintainer time, in part because adding S-o-b can be done by machine alone, so it is not a worthless exercise on your part to send a respin just for typos.
os-posix.c | 6 ++++++ os-win32.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-)
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Prev in Thread] | Current Thread | [Next in Thread] |