From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36526) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RimS9-0008Sy-8A for qemu-devel@nongnu.org; Thu, 05 Jan 2012 07:29:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RimS1-00062t-7v for qemu-devel@nongnu.org; Thu, 05 Jan 2012 07:29:53 -0500 Received: from mx1.redhat.com ([209.132.183.28]:25857) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RimS0-00062o-Sx for qemu-devel@nongnu.org; Thu, 05 Jan 2012 07:29:45 -0500 Date: Thu, 5 Jan 2012 10:29:40 -0200 From: Luiz Capitulino Message-ID: <20120105102940.23debcce@doriath> In-Reply-To: <4F04B08E.9060604@redhat.com> References: <1325706313-21936-1-git-send-email-lcapitulino@redhat.com> <1325706313-21936-3-git-send-email-lcapitulino@redhat.com> <4F04B08E.9060604@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] qemu-ga: Add the guest-suspend command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: amit.shah@redhat.com, jcody@redhat.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com On Wed, 04 Jan 2012 13:03:26 -0700 Eric Blake 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. >