| Message ID | 570642E5.2020609@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
Quoting Yuriy Pudgorodskiy (2016-04-07 06:22:13) > On 4/7/2016 2:53 AM, Michael Roth wrote: > > Quoting Denis V. Lunev (2016-04-06 00:43:30) > >> From: Yuriy Pudgorodskiy <yur@virtuozzo.com> > >> > >> Signed-off-by: Yuriy Pudgorodskiy <yur@virtuozzo.com> > >> Signed-off-by: Denis V. Lunev <den@openvz.org> > >> CC: Michael Roth <mdroth@linux.vnet.ibm.com> > >> --- > >> qga/commands.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/qga/commands.c b/qga/commands.c > >> index e091ee1..9ad2f7d 100644 > >> --- a/qga/commands.c > >> +++ b/qga/commands.c > >> @@ -446,6 +446,7 @@ GuestExec *qmp_guest_exec(const char *path, > >> g_io_channel_set_encoding(in_ch, NULL, NULL); > >> g_io_channel_set_buffered(in_ch, false); > >> g_io_channel_set_flags(in_ch, G_IO_FLAG_NONBLOCK, NULL); > >> + g_io_channel_set_close_on_unref(in_ch, true); > >> g_io_add_watch(in_ch, G_IO_OUT, guest_exec_input_watch, &gei->in); > >> } > >> > >> @@ -461,6 +462,8 @@ GuestExec *qmp_guest_exec(const char *path, > >> g_io_channel_set_encoding(err_ch, NULL, NULL); > >> g_io_channel_set_buffered(out_ch, false); > >> g_io_channel_set_buffered(err_ch, false); > >> + g_io_channel_set_close_on_unref(out_ch, true); > >> + g_io_channel_set_close_on_unref(err_ch, true); > > I don't seem any harm in adding these for safety, but don't the handles > > get closed via the g_io_channel_shutdown(ch, ...) calls we make prior to > > unref in guest_exec_{output,input}_watch()? Or is there another unref > > path I'm missing? > > > > Well, it may be fixed with explicit g_io_channel_shutdown(), and may be > it is a > better fix style, but it needs to be fixed somehow. We missed in our > code a call > to shutdown in guest_exec_output_watch(): > > > diff --git a/qga/commands.c b/qga/commands.c > index 45688c8..9acb2f6 100644 > --- a/qga/commands.c > +++ b/qga/commands.c > @@ -373,6 +373,7 @@ static gboolean guest_exec_output_watch(GIOChannel *ch, > return true; > > close: > + g_io_channel_shutdown(ch, true, NULL); > g_io_channel_unref(ch); > g_atomic_int_set(&p->closed, 1); > return false; When I scanned the code I could've sworn I saw it for both input/output, but you're right, it's missing for output. I might squash in the above hunk for symmetry and to avoid confusion about whether it needs to be added later, but for now I've applied it as-is. > > > > >> g_io_add_watch(out_ch, G_IO_IN | G_IO_HUP, > >> guest_exec_output_watch, &gei->out); > >> g_io_add_watch(err_ch, G_IO_IN | G_IO_HUP, > >> -- > >> 2.1.4 > >> > > . > > >
diff --git a/qga/commands.c b/qga/commands.c index 45688c8..9acb2f6 100644 --- a/qga/commands.c +++ b/qga/commands.c @@ -373,6 +373,7 @@ static gboolean guest_exec_output_watch(GIOChannel *ch, return true; close: + g_io_channel_shutdown(ch, true, NULL); g_io_channel_unref(ch); g_atomic_int_set(&p->closed, 1); return false;