[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] qemu-ga: Add the guest-suspend command
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] qemu-ga: Add the guest-suspend command |
Date: |
Thu, 5 Jan 2012 10:29:40 -0200 |
On Wed, 04 Jan 2012 13:03:26 -0700
Eric Blake <address@hidden> wrote:
> On 01/04/2012 12:45 PM, Luiz Capitulino wrote:
> > + if (pid == 0) {
> > + /* child */
> > + int fd;
> > +
> > + setsid();
> > + fclose(stdin);
> > + fclose(stdout);
> > + fclose(stderr);
> > +
> > + execlp(pmutils_bin, pmutils_bin, NULL);
>
> It's generally a bad idea to exec a child process without fd 0, 1, and 2
> open on something, even if that something is /dev/null. POSIX says that
> the system may, but not must, reopen fds on your behalf, and that the
> child without open std descriptors is then executing in a non-conforming
> environment and may misbehave in unexpected manners.
You're right. I just copied what we do in qmp_guest_shutdown()... I think we
have to open /dev/null then, do you agree Michael?
I think I'm doing to use dup2(), like dup2(dev_null_fd, 0). Any better
recommendation?
>
> > +
> > + /*
> > + * The exec call should not return, if it does something went
> > wrong.
> > + * In this case we try to suspend manually if 'mode' is 'hibernate'
> > + */
> > + slog("could not execute %s: %s\n", pmutils_bin, strerror(errno));
> > + slog("trying to suspend using the manual method...\n");
> > +
> > + fd = open(LINUX_SYS_STATE_FILE, O_WRONLY);
>
> Worse, since you _just_ closed stdin above, fd here will most likely be
> 0, but a O_WRONLY stdin is asking for problems.
>
> > + if (fd < 0) {
> > + slog("can't open file %s: %s\n", LINUX_SYS_STATE_FILE,
> > + strerror(errno));
>
> Also, I have no idea where slog() writes to, but since you closed
> stderr, if slog() is trying to use stderr, your error messages would be
> invisible.
>
- [Qemu-devel] [PATCH v4 0/2]: qemu-ga: Add the guest-suspend command, Luiz Capitulino, 2012/01/04
- [Qemu-devel] [PATCH 1/2] qemu-ga: set O_NONBLOCK for serial channels, Luiz Capitulino, 2012/01/04
- [Qemu-devel] [PATCH 2/2] qemu-ga: Add the guest-suspend command, Luiz Capitulino, 2012/01/04
- Re: [Qemu-devel] [PATCH v4 0/2]: qemu-ga: Add the guest-suspend command, Daniel P. Berrange, 2012/01/05
- Re: [Qemu-devel] [PATCH v4 0/2]: qemu-ga: Add the guest-suspend command, Luiz Capitulino, 2012/01/05
- Re: [Qemu-devel] [PATCH v4 0/2]: qemu-ga: Add the guest-suspend command, Daniel P. Berrange, 2012/01/05
- Re: [Qemu-devel] [PATCH v4 0/2]: qemu-ga: Add the guest-suspend command, Luiz Capitulino, 2012/01/05
- Re: [Qemu-devel] [PATCH v4 0/2]: qemu-ga: Add the guest-suspend command, Michael Roth, 2012/01/05
- Re: [Qemu-devel] [PATCH v4 0/2]: qemu-ga: Add the guest-suspend command, Luiz Capitulino, 2012/01/05
- Re: [Qemu-devel] [PATCH v4 0/2]: qemu-ga: Add the guest-suspend command, Michael Roth, 2012/01/05
- Re: [Qemu-devel] [PATCH v4 0/2]: qemu-ga: Add the guest-suspend command, Luiz Capitulino, 2012/01/06