| Message ID | 1327671245-5231-1-git-send-email-lersek@redhat.com |
|---|---|
| State | New |
| Headers | show |
Laszlo Ersek <lersek@redhat.com> writes: > The lockf() call in qemu_create_pidfile() aims at ensuring mutual > exclusion. We shouldn't close the pidfile on success (as introduced by > commit 1bbd1592), because that drops the lock as well [1]: > > "File locks shall be released on first close by the locking process > of any file descriptor for the file." > > Coverity may complain again about the leaked file descriptor; let's > worry about that later. > > v1->v2: > - add reference to 1bbd1592 > - explain the intentional fd leak in the source > > [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/lockf.html > > Signed-off-by: Laszlo Ersek <lersek@redhat.com> Thanks for cleaning up the mess I made! Reviewed-by: Markus Armbruster <armbru@redhat.com>
On 01/27/2012 07:34 AM, Laszlo Ersek wrote: > The lockf() call in qemu_create_pidfile() aims at ensuring mutual > exclusion. We shouldn't close the pidfile on success (as introduced by > commit 1bbd1592), because that drops the lock as well [1]: > > "File locks shall be released on first close by the locking process > of any file descriptor for the file." > > Coverity may complain again about the leaked file descriptor; let's > worry about that later. > > v1->v2: > - add reference to 1bbd1592 > - explain the intentional fd leak in the source > > [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/lockf.html Applied. Thanks. Regards, Anthony Liguori > > Signed-off-by: Laszlo Ersek<lersek@redhat.com> > --- > os-posix.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/os-posix.c b/os-posix.c > index 5c437ca..e3ed497 100644 > --- a/os-posix.c > +++ b/os-posix.c > @@ -348,6 +348,6 @@ int qemu_create_pidfile(const char *filename) > return -1; > } > > - close(fd); > + /* keep pidfile open& locked forever */ > return 0; > }
diff --git a/os-posix.c b/os-posix.c index 5c437ca..e3ed497 100644 --- a/os-posix.c +++ b/os-posix.c @@ -348,6 +348,6 @@ int qemu_create_pidfile(const char *filename) return -1; } - close(fd); + /* keep pidfile open & locked forever */ return 0; }
The lockf() call in qemu_create_pidfile() aims at ensuring mutual exclusion. We shouldn't close the pidfile on success (as introduced by commit 1bbd1592), because that drops the lock as well [1]: "File locks shall be released on first close by the locking process of any file descriptor for the file." Coverity may complain again about the leaked file descriptor; let's worry about that later. v1->v2: - add reference to 1bbd1592 - explain the intentional fd leak in the source [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/lockf.html Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- os-posix.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)