Re: [Qemu-devel] [PATCH 1/2] qga: fix fd leak with guest-exec i/o channels

[Yuriy Pudgorodskiy]

On 4/7/2016 2:53 AM, Michael Roth wrote:
> Quoting Denis V. Lunev (2016-04-06 00:43:30)
> > From: Yuriy Pudgorodskiy <address@hidden>
> >
> > Signed-off-by: Yuriy Pudgorodskiy <address@hidden>
> > Signed-off-by: Denis V. Lunev <address@hidden>
> > CC: Michael Roth <address@hidden>
> > ---
> >   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;



> >           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
> .