Re: [Qemu-devel] [PATCH for-2.5 3/3] vhost-user-test: fix crash with glib < 2.36

[Michael S. Tsirkin]

On Mon, Nov 30, 2015 at 02:12:54PM +0100, Marc-André Lureau wrote:
> On Mon, Nov 30, 2015 at 2:09 PM, Michael S. Tsirkin <address@hidden> wrote:
> > On Mon, Nov 30, 2015 at 06:00:29AM -0500, Marc-André Lureau wrote:
> >>
> >>
> >> ----- Original Message -----
> >> > On Fri, Nov 27, 2015 at 03:41:20PM +0100, address@hidden wrote:
> >> > > From: Marc-André Lureau <address@hidden>
> >> > >
> >> > > The prepare callback needs to be implemented with glib < 2.36.
> >> > >
> >> > > Signed-off-by: Marc-André Lureau <address@hidden>
> >> >
> >> > But nothing prevents us from implementing it for all versions.
> >>
> >> Sure, it's just a kind of dead code for me that should be removed in 
> >> future version. So I prefer to keep it under #ifdef.
> >>
> >> > > ---
> >> > >  tests/vhost-user-test.c | 13 ++++++++++++-
> >> > >  1 file changed, 12 insertions(+), 1 deletion(-)
> >> > >
> >> > > diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> >> > > index 29205ed..4ab48e4 100644
> >> > > --- a/tests/vhost-user-test.c
> >> > > +++ b/tests/vhost-user-test.c
> >> > > @@ -506,8 +506,19 @@ test_migrate_source_check(GSource *source)
> >> > >      return FALSE;
> >> > >  }
> >> > >
> >> > > +#if !GLIB_CHECK_VERSION(2,36,0)
> >> > > +static gboolean
> >> > > +test_migrate_source_prepare(GSource *source, gint *timeout)
> >> > > +{
> >> > > +    *timeout = -1;
> >> > > +    return FALSE;
> >> > > +}
> >> > > +#endif
> >> > > +
> >> > >  GSourceFuncs test_migrate_source_funcs = {
> >> > > -    NULL,
> >> > > +#if !GLIB_CHECK_VERSION(2,36,0)
> >> > > +    test_migrate_source_prepare,
> >> > > +#endif
> >> > >      test_migrate_source_check,
> >> >
> >> > So now for version 2.36 check will be used instead of prepare.
> >> > That's clearly wrong.
> >>
> >> instead?
> >> >
> >> > I'll switch this to named initializers to avoid the mess.
> >>
> >> I don't follow what is the mess here.
> >
> >
> > You sent v2 that fixes it, and my patch fixes it too, so we don't need to 
> > argue.
> 
> 
> I fixed it, you changed it because you don't agree with my will to
> keep the version check to remove the dumb callback in the future. I
> disagree, I think this is a right way to cleanup the code in the
> future. Btw, Author/ Reported-by is wrong: it is the other way around.
> You reported it to me, and I fixed it.

I don't remember reporting it, but my patch didn't use your fix:
check list archives. Since I did not like v1,
I posted a patch before you posted v2.
That's why I did not list you as the author.

> Please reconsider my patch.

If you feel so strongly about it, I'll apply your patch
if you add a comment quoting the glib documentation.

> -- 
> Marc-André Lureau