[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo |
Date: |
Wed, 25 Jan 2017 14:44:20 +0000 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
* Cornelia Huck (address@hidden) wrote:
> On Wed, 25 Jan 2017 13:22:55 +0000
> "Dr. David Alan Gilbert" <address@hidden> wrote:
>
> > * Cornelia Huck (address@hidden) wrote:
> > > On Wed, 25 Jan 2017 12:00:53 +0000
> > > "Dr. David Alan Gilbert" <address@hidden> wrote:
> > >
> > > > * Fam Zheng (address@hidden) 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.
> > >
> > > Just saw this. I don't think we want -ENOMEM, as that would change the
> > > actual state being saved, no?
> >
> > But isn't that the intention of this function?
> >
> > buf = g_try_malloc0(len);
> > if (!buf) {
> > /* Storing FLIC_FAILED into the count field here will cause the
> > * target system to fail when attempting to load irqs from the
> > * migration state */
> > error_report("flic: couldn't allocate memory");
> > qemu_put_be64(f, FLIC_FAILED);
> > return;
> > }
> >
> > What should happen on the destination - should the migration fail?
> > If we want the migration to fail then we should now return an error
> > status rather than 0, and then we see a failed migration on the source
> > as well.
>
> Yes. There's also another error case below where we should return an
> error instead of putting FLIC_FAILED, then.
>
> The problem is that this is rather hard to test: So I'd prefer to fix
> the compile for now and introduce error return codes in a separate
> patch.
OK, that's fair.
Dave
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- [Qemu-devel] [PULL 13/15] PCI/migration merge vmstate_pci_device and vmstate_pcie_device, (continued)
- [Qemu-devel] [PULL 13/15] PCI/migration merge vmstate_pci_device and vmstate_pcie_device, Dr. David Alan Gilbert (git), 2017/01/24
- [Qemu-devel] [PULL 09/15] migration: disallow migrate_add_blocker during migration, Dr. David Alan Gilbert (git), 2017/01/24
- [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo, Dr. David Alan Gilbert (git), 2017/01/24
- Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo, Fam Zheng, 2017/01/25
- Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo, Dr. David Alan Gilbert, 2017/01/25
- Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo, Cornelia Huck, 2017/01/25
- Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo, Fam Zheng, 2017/01/25
- Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo, Cornelia Huck, 2017/01/25
- Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo, Dr. David Alan Gilbert, 2017/01/25
- Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo, Cornelia Huck, 2017/01/25
- Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo,
Dr. David Alan Gilbert <=
- Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo, Cornelia Huck, 2017/01/26
- Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo, Dr. David Alan Gilbert, 2017/01/27
[Qemu-devel] [PULL 03/15] migration: migrate QTAILQ, Dr. David Alan Gilbert (git), 2017/01/24
[Qemu-devel] [PULL 08/15] migration: Allow "device add" options to only add migratable devices, Dr. David Alan Gilbert (git), 2017/01/24
[Qemu-devel] [PULL 11/15] migration: re-active images while migration been canceled after inactive them, Dr. David Alan Gilbert (git), 2017/01/24
[Qemu-devel] [PULL 14/15] migration: transform remaining DPRINTF into trace_, Dr. David Alan Gilbert (git), 2017/01/24
[Qemu-devel] [PULL 15/15] migration/tracing: Add tracing on save, Dr. David Alan Gilbert (git), 2017/01/24
Re: [Qemu-devel] [PULL 00/15] migration queue, Peter Maydell, 2017/01/25