Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo

[Fam Zheng <famz@redhat.com>]

On Wed, 01/25 12:00, Dr. David Alan Gilbert wrote:
> * Fam Zheng (famz@redhat.com) wrote:
> > On Tue, 01/24 18:47, Dr. David Alan Gilbert (git) wrote:
> > > diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> > > index c313166..da8e4df 100644
> > > --- a/hw/intc/s390_flic_kvm.c
> > > +++ b/hw/intc/s390_flic_kvm.c
> > > @@ -286,7 +286,8 @@ static void kvm_s390_release_adapter_routes(S390FLICState *fs,
> > >   * increase until buffer is sufficient or maxium size is
> > >   * reached
> > >   */
> > > -static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > > +static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> > > +                         VMStateField *field, QJSON *vmdesc)
> > >  {
> > >      KVMS390FLICState *flic = opaque;
> > >      int len = FLIC_SAVE_INITIAL_SIZE;
> > > @@ -319,6 +320,8 @@ static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > >                          count * sizeof(struct kvm_s390_irq));
> > >      }
> > >      g_free(buf);
> > > +
> > > +    return 0;
> > >  }
> > 
> > This hunk left one 'return' behind in the function, which should have been
> > changed to 'return 0' as well, and now the compiler is not happy:
> > 
> > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c: In function ‘kvm_flic_save’:
> > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:306:9: error: ‘return’ with no value, in function returning non-void [-Werror]
> >          return;
> >          ^~~~~~
> > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:289:12: note: declared here
> >  static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> >             ^~~~~~~~~~~~~
> > cc1: all warnings being treated as errors
> 
> OK, so it looks like that's a failure path, adding a return -ENOMEM would seem to make
> sense there.

OK! I was wrong.

> 
> Do you have a way of build testing that on x86, or can it only be build
> tested on s390?
> (My build test included an s390x-softmmu build on x86-64).

Unfortunately I don't know. This is reported by the newly connected s390x
patchew tester, which is in staging so not replying publicly yet. Hopefully we
can get s390x native build covered in the future.

Fam

> 
> Dave
> > Fam
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK