Hi,
> Subject: Re: [Qemu-devel] [PATCH 2/4] os-posix: report error message when
> lock file failed
>
> <arei.gong...@huawei.com> writes:
>
> > From: Gonglei <arei.gong...@huawei.com>
> >
> > It will cause that create vm failed When manager
> > tool is killed forcibly (kill -9 libvirtd_pid),
> > the file not was unlink, and unlock. It's better
> > that report the error message for users.
> >
> > Signed-off-by: Huangweidong <weidong.hu...@huawei.com>
> > Signed-off-by: Gonglei <arei.gong...@huawei.com>
> > ---
> > os-posix.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/os-posix.c b/os-posix.c
> > index 9d5ae70..89831dc 100644
> > --- a/os-posix.c
> > +++ b/os-posix.c
> > @@ -316,6 +316,7 @@ int qemu_create_pidfile(const char *filename)
> > return -1;
> > }
> > if (lockf(fd, F_TLOCK, 0) == -1) {
> > + error_report("lock file '%s' failed: %s", filename,
> > strerror(errno));
> > close(fd);
> > return -1;
> > }
>
> Only called from main():
>
Yes, indeed.
> if (pid_file && qemu_create_pidfile(pid_file) != 0) {
> os_pidfile_error();
> exit(1);
> }
>
> I suspect the error reporting is os_pidfile_error()'s job. And it
> actually does it (POSIX version shown, W32 is simpler):
>
> void os_pidfile_error(void)
> {
> if (daemonize) {
> uint8_t status = 1;
> if (write(fds[1], &status, 1) != 1) {
> perror("daemonize. Writing to pipe\n");
> }
> } else
> fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno));
> }
>
> Are you sure your patch makes sense?
Yes, I'm sure it make sense. And I had tested, the result is OK as expected.
When daemonize variable is true, the os_pidfile_error() usually don't report an
error,
because "if (write(fds[1], &status, 1) != 1)" is always false. On the other
hand, we should assure that we can get the original error message
when lock failed but not other information, such as "Could not acquire pid
file...".
Best regards,
-Gonglei
