On Thu, Jul 05, 2018 at 10:15:47AM +0100, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@redhat.com) wrote:
> > We were checking against -EIO, assuming that it will cover all IO
> > failures. But actually it is not. One example is that in
> > qemu_loadvm_section_start_full() we can have tons of places that will
> > return -EINVAL even if the error is caused by IO failures on the
> > network.
>
> I suspect we should fix those; but OK; I think the only cases
> in there that could hit that are the get_counted_string and the
> check_section_footer; they could both be fixed to return an errno
> like the other cases.
>
> > Let's loosen the recovery check logic here to cover all the error cases
> > happened by removing the explicit check against -EIO. After all we
> > won't lose anything here if any other failure happened.
> >
> > Signed-off-by: Peter Xu <pet...@redhat.com>
> > ---
> > migration/savevm.c | 16 ++++++----------
> > 1 file changed, 6 insertions(+), 10 deletions(-)
> >
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index 851d74e8b6..efcc795071 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -2276,18 +2276,14 @@ out:
> > qemu_file_set_error(f, ret);
> >
> > /*
> > - * Detect whether it is:
> > - *
> > - * 1. postcopy running (after receiving all device data, which
> > - * must be in POSTCOPY_INCOMING_RUNNING state. Note that
> > - * POSTCOPY_INCOMING_LISTENING is still not enough, it's
> > - * still receiving device states).
> > - * 2. network failure (-EIO)
> > - *
> > - * If so, we try to wait for a recovery.
> > + * If we are during an active postcopy, then we pause instead
> > + * of bail out to at least keep the VM's dirty data. Note
> > + * that POSTCOPY_INCOMING_LISTENING stage is still not enough,
> > + * during which we're still receiving device states and we
> > + * still haven't yet started the VM on destination.
> > */
> > if (postcopy_state_get() == POSTCOPY_INCOMING_RUNNING &&
> > - ret == -EIO && postcopy_pause_incoming(mis)) {
> > + postcopy_pause_incoming(mis)) {
>
> Hmm, OK; It does make me a bit nervous we might trigger this
> too often, but I see the other cases where we're missing stuff
> we should trigger it. We might find we need to go the other way
> and blacklist some errors.
AFAIU the difference will be only that we pause instead of quit QEMU.
IMHO the whitelist/blacklist will be more meaningful when there is a
third choice for us (besides "quit" and "go into pause state").
>
>
> Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
Thanks!
--
Peter Xu
