Re: [Qemu-devel] [PATCH 1/5] migration: pre_save return int

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Cornelia Huck (cohuck@redhat.com) wrote:
> On Tue, 19 Sep 2017 19:00:34 +0100
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> 
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Modify the pre_save method on VMStateDescription to return an int
> > rather than void so that it potentially can fail.
> > 
> > Changed zillions of devices to make them return 0; the only
> > case I've made it return non-0 is hw/intc/s390_flic_kvm.c that already
> > had an error_report/return case.
> 
> Never thought that this device would be at the bleeding edge ;)

It's the one case that had bothered to do a proper error.

> > 
> > Note: If you add an error exit in your pre_save you must emit
> > an error_report to say why.
> 
> Would it make sense to relay an error object? For example,
> cpu_pre_save() in target/s390x/machine.c calls
> kvm_s390_vcpu_interrupt_pre_save() which already does an error report.
> If we relay that error instead, we would avoid saying "oops, this
> didn't work" several times with decreasing amount of information.
> 
> On the other hand, that change would be more invasive.

Right, and it's very very verbose.

> 
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> 
> >  hw/intc/s390_flic_kvm.c        |  6 ++++--
> 
> >  hw/s390x/css.c                 | 10 +++++++---
> >  hw/s390x/virtio-ccw.c          |  4 +++-
> 
> >  target/s390x/machine.c         |  4 +++-
> 
> That said, the changes in s390-related code look fine.

Thanks.

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