From patchwork Tue Oct 13 14:09:43 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Denis V. Lunev" X-Patchwork-Id: 529752 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4EF501402B2 for ; Wed, 14 Oct 2015 01:11:22 +1100 (AEDT) Received: from localhost ([::1]:36142 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zm0IS-00017N-AM for incoming@patchwork.ozlabs.org; Tue, 13 Oct 2015 10:11:20 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47673) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zm0H9-0000BJ-D3 for qemu-devel@nongnu.org; Tue, 13 Oct 2015 10:10:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zm0H6-0003an-6D for qemu-devel@nongnu.org; Tue, 13 Oct 2015 10:09:59 -0400 Received: from mx2.parallels.com ([199.115.105.18]:41367) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zm0H5-0003a6-VR for qemu-devel@nongnu.org; Tue, 13 Oct 2015 10:09:56 -0400 Received: from [199.115.105.250] (helo=mail.odin.com) by mx2.parallels.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-SHA384:256) (Exim 4.86) (envelope-from ) id 1Zm0H0-0005Ms-Ka; Tue, 13 Oct 2015 07:09:50 -0700 Received: from [10.30.2.139] (10.30.2.139) by US-EXCH.sw.swsoft.com (10.255.249.47) with Microsoft SMTP Server (TLS) id 15.0.1130.7; Tue, 13 Oct 2015 07:09:46 -0700 To: Michael Roth References: <1444213941-11128-1-git-send-email-den@openvz.org> <1444213941-11128-6-git-send-email-den@openvz.org> <20151013040508.10003.13913@loki> From: "Denis V. Lunev" Message-ID: <561D10A7.6050409@openvz.org> Date: Tue, 13 Oct 2015 17:09:43 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <20151013040508.10003.13913@loki> X-ClientProxiedBy: US-EXCH.sw.swsoft.com (10.255.249.47) To US-EXCH.sw.swsoft.com (10.255.249.47) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 199.115.105.18 Cc: Yuri Pudgorodskiy , qemu-devel@nongnu.org, v.tolstov@selfip.ru Subject: Re: [Qemu-devel] [PATCH 5/5] qga: guest-exec simple stdin/stdout/stderr redirection X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 10/13/2015 07:05 AM, Michael Roth wrote: > Quoting Denis V. Lunev (2015-10-07 05:32:21) >> From: Yuri Pudgorodskiy >> >> Implemented with base64-encoded strings in qga json protocol. >> Glib portable GIOChannel is used for data I/O. >> >> Optinal stdin parameter of guest-exec command is now used as >> stdin content for spawned subprocess. >> >> If capture-output bool flag is specified, guest-exec redirects out/err >> file descriptiors internally to pipes and collects subprocess >> output. >> >> Guest-exe-status is modified to return this collected data to requestor >> in base64 encoding. >> >> Signed-off-by: Yuri Pudgorodskiy >> Signed-off-by: Denis V. Lunev >> CC: Michael Roth > For capture-output mode, win32 guests appear to spin forever on EOF and > the exec'd process never gets reaped as a result. The below patch > seems to fix this issue. If this seems reasonable I can squash it > into the patch directly, but you might want to check I didn't break one > of your !capture-output use-cases (I assume this is still mainly focused > on redirecting to virtio-serial for stdio). > > Another issue I noticed is that glib relies on > gspawn-win{32,64}-helper[-console].exe for g_spawn*() interface, so guest > exec won't work for .msi distributables unless those are added. This can > probably be addressed during soft-freeze though. > > diff --git a/qga/commands.c b/qga/commands.c > index 27c06c5..fbf8abd 100644 > --- a/qga/commands.c > +++ b/qga/commands.c > @@ -318,7 +318,7 @@ static gboolean guest_exec_output_watch(GIOChannel *ch, > struct GuestExecIOData *p = (struct GuestExecIOData *)p_; > gsize bytes_read = 0; > > - if (cond == G_IO_HUP) { > + if (cond == G_IO_HUP || cond == G_IO_ERR) { > g_io_channel_unref(ch); > g_atomic_int_set(&p->closed, 1); > return FALSE; > @@ -330,10 +330,18 @@ static gboolean guest_exec_output_watch(GIOChannel *ch, > t = g_try_realloc(p->data, p->size + GUEST_EXEC_IO_SIZE); > } > if (t == NULL) { > + GIOStatus gstatus = 0; > p->truncated = true; > /* ignore truncated output */ > gchar buf[GUEST_EXEC_IO_SIZE]; > - g_io_channel_read_chars(ch, buf, sizeof(buf), &bytes_read, NULL); > + gstatus = g_io_channel_read_chars(ch, buf, sizeof(buf), > + &bytes_read, NULL); > + if (gstatus == G_IO_STATUS_EOF || gstatus == G_IO_STATUS_ERROR) { > + g_io_channel_unref(ch); > + g_atomic_int_set(&p->closed, 1); > + return false; > + } > + > return TRUE; > } > p->size += GUEST_EXEC_IO_SIZE; > @@ -342,8 +350,14 @@ static gboolean guest_exec_output_watch(GIOChannel *ch, > > /* Calling read API once. > * On next available data our callback will be called again */ > - g_io_channel_read_chars(ch, (gchar *)p->data + p->length, > + GIOStatus gstatus = 0; > + gstatus = g_io_channel_read_chars(ch, (gchar *)p->data + p->length, > p->size - p->length, &bytes_read, NULL); > + if (gstatus == G_IO_STATUS_EOF || gstatus == G_IO_STATUS_ERROR) { > + g_io_channel_unref(ch); > + g_atomic_int_set(&p->closed, 1); > + return false; > + } not completely. we have tested your code and found that minor change is required Signed-off-by: Yuri Pudgorodskiy --- qga/commands.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) I'll send a patch with this change and minor cosmetic improvements soon (to make code shorter), but you can take your version with this fix applied at your taste. Den diff --git a/qga/commands.c b/qga/commands.c index 1811ce6..deb041e 100644 --- a/qga/commands.c +++ b/qga/commands.c @@ -413,7 +413,7 @@ GuestExec *qmp_guest_exec(const char *path, if (has_inp_data) { gei->in.data = g_base64_decode(inp_data, &gei->in.size); #ifdef G_OS_WIN32 - in_ch = g_io_channel_win32_new_fd(in_ch); + in_ch = g_io_channel_win32_new_fd(in_fd); #else in_ch = g_io_channel_unix_new(in_fd); #endif