[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo
From: |
Cornelia Huck |
Subject: |
Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo |
Date: |
Thu, 26 Jan 2017 13:14:02 +0100 |
On Wed, 25 Jan 2017 14:44:20 +0000
"Dr. David Alan Gilbert" <address@hidden> wrote:
> * 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:
> > > > > 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.
I've coded something up and tried to test it with error injection to
trigger the failed case, but I can't really see an improvement :(
Before: source logs error, target fails to load the flic with 'invalid
argument'
After: source logs error, target fails to load the flic with 'could not
allocate memory'
The migration code does not seem to do anything with the return code of
put methods for now, so that's not too surprising. Is anything in the
works?
For now, I'd prefer to keep the old behaviour as 'invalid argument'
seems like a more obvious error on the target.
diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index e86a84e49a..3c62ef8258 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -293,27 +293,21 @@ static int kvm_flic_save(QEMUFile *f, void *opaque,
size_t size,
int len = FLIC_SAVE_INITIAL_SIZE;
void *buf;
int count;
+ int r = 0;
flic_disable_wait_pfault((struct KVMS390FLICState *) opaque);
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 0;
+ return -ENOMEM;
}
count = __get_all_irqs(flic, &buf, len);
if (count < 0) {
error_report("flic: couldn't retrieve irqs from kernel, rc %d",
count);
- /* Storing FLIC_FAILED into the count field here will cause the
- * target system to fail when attempting to load irqs from the
- * migration state */
- qemu_put_be64(f, FLIC_FAILED);
+ r = count;
} else {
qemu_put_be64(f, count);
qemu_put_buffer(f, (uint8_t *) buf,
@@ -321,7 +315,7 @@ static int kvm_flic_save(QEMUFile *f, void *opaque, size_t
size,
}
g_free(buf);
- return 0;
+ return r;
}
/**
- [Qemu-devel] [PULL 09/15] migration: disallow migrate_add_blocker during migration, (continued)
- [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, 2017/01/25
- Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo,
Cornelia Huck <=
- 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