[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Is block_save_iterate() dead code? (was: migration: Fix
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] Is block_save_iterate() dead code? (was: migration: Fix return code of ram_save_iterate() ) |
Date: |
Fri, 16 Dec 2016 17:03:57 +0000 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
* Thomas Huth (address@hidden) wrote:
> On 18.11.2016 09:13, Thomas Huth wrote:
> > On 17.11.2016 04:45, David Gibson wrote:
> >> On Mon, Nov 14, 2016 at 07:34:59PM +0100, Juan Quintela wrote:
> >>> Thomas Huth <address@hidden> wrote:
> >>>> qemu_savevm_state_iterate() expects the iterators to return 1
> >>>> when they are done, and 0 if there is still something left to do.
> >>>> However, ram_save_iterate() does not obey this rule and returns
> >>>> the number of saved pages instead. This causes a fatal hang with
> >>>> ppc64 guests when you run QEMU like this (also works with TCG):
> >>>>
> >>>> qemu-img create -f qcow2 /tmp/test.qcow2 1M
> >>>> qemu-system-ppc64 -nographic -nodefaults -m 256 \
> >>>> -hda /tmp/test.qcow2 -serial mon:stdio
> >>>>
> >>>> ... then switch to the monitor by pressing CTRL-a c and try to
> >>>> save a snapshot with "savevm test1" for example.
> >>>>
> >>>> After the first iteration, ram_save_iterate() always returns 0 here,
> >>>> so that qemu_savevm_state_iterate() hangs in an endless loop and you
> >>>> can only "kill -9" the QEMU process.
> >>>> Fix it by using proper return values in ram_save_iterate().
> >>>>
> >>>> Signed-off-by: Thomas Huth <address@hidden>
> >>>
> >>> Reviewed-by: Juan Quintela <address@hidden>
> >>>
> >>> Applied.
> >>>
> >>> I don't know how we broked this so much.
> >>
> >> Note that block save iterate has the same bug...
> >
> > I think you're right. Care to send a patch?
>
> Looking at this issue again ... could it be that block_save_iterate() is
> currently just dead code?
> As far as I can see, the ->save_live_iterate() handlers are only called
> from qemu_savevm_state_iterate(), right? And qemu_savevm_state_iterate()
> only calls the handlers if se->ops->is_active(se->opaque) returns true.
> But block_is_active() seems to only return 0 during savevm, most likely
> because qemu_savevm_state() explicitly sets the "blk" and "shared"
> MigrationParams to zero.
> So to me, it looks like we could also just remove block_save_iterate()
> completely ... or did I miss something here?
Doesn't it get called by migrate -b ?
Dave
> Thomas
>
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK