| Message ID | 1492104214-29994-2-git-send-email-kwolf@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 04/13/2017 12:23 PM, Kevin Wolf wrote: > Commit d35ff5e6 ('block: Ignore guest dev permissions during incoming > migration') added blk_resume_after_migration() to the precopy migration > path, but neglected to add it to the duplicated code that is used for > postcopy migration. This means that the guest device doesn't request the > necessary permissions, which ultimately led to failing assertions. > > Add the missing blk_resume_after_migration() to the postcopy path. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > migration/savevm.c | 8 ++++++++ > 1 file changed, 8 insertions(+) Reviewed-by: Eric Blake <eblake@redhat.com> Are we targetting this for 2.9-rc5, or is it 2.10 material?
Am 13.04.2017 um 19:39 hat Eric Blake geschrieben: > On 04/13/2017 12:23 PM, Kevin Wolf wrote: > > Commit d35ff5e6 ('block: Ignore guest dev permissions during incoming > > migration') added blk_resume_after_migration() to the precopy migration > > path, but neglected to add it to the duplicated code that is used for > > postcopy migration. This means that the guest device doesn't request the > > necessary permissions, which ultimately led to failing assertions. > > > > Add the missing blk_resume_after_migration() to the postcopy path. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > migration/savevm.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > Reviewed-by: Eric Blake <eblake@redhat.com> > > Are we targetting this for 2.9-rc5, or is it 2.10 material? At this point, I think it's clearly 2.10. Kevin
On 04/13/2017 12:54 PM, Kevin Wolf wrote: > Am 13.04.2017 um 19:39 hat Eric Blake geschrieben: >> On 04/13/2017 12:23 PM, Kevin Wolf wrote: >>> Commit d35ff5e6 ('block: Ignore guest dev permissions during incoming >>> migration') added blk_resume_after_migration() to the precopy migration >>> path, but neglected to add it to the duplicated code that is used for >>> postcopy migration. This means that the guest device doesn't request the >>> necessary permissions, which ultimately led to failing assertions. >>> >>> Add the missing blk_resume_after_migration() to the postcopy path. >>> >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >>> --- >>> migration/savevm.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> >> Are we targetting this for 2.9-rc5, or is it 2.10 material? > > At this point, I think it's clearly 2.10. Okay. Restating, to make sure I got your reasoning: the removed assertions of commit e3e0003 imply that 2.9 is not regressing in behavior, and at this point the worst the code can do without this patch applied is behave like it's done previously; therefore this patch is not fixing an observable 2.9 behavior and therefore not worth holding up the release. But for 2.10, it's absolutely essential, as we have another patch pending to revert e3e0003 at which point we have a behavior break without this patch. Works for me.
Am 13.04.2017 um 20:03 hat Eric Blake geschrieben: > On 04/13/2017 12:54 PM, Kevin Wolf wrote: > > Am 13.04.2017 um 19:39 hat Eric Blake geschrieben: > >> On 04/13/2017 12:23 PM, Kevin Wolf wrote: > >>> Commit d35ff5e6 ('block: Ignore guest dev permissions during incoming > >>> migration') added blk_resume_after_migration() to the precopy migration > >>> path, but neglected to add it to the duplicated code that is used for > >>> postcopy migration. This means that the guest device doesn't request the > >>> necessary permissions, which ultimately led to failing assertions. > >>> > >>> Add the missing blk_resume_after_migration() to the postcopy path. > >>> > >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> > >>> --- > >>> migration/savevm.c | 8 ++++++++ > >>> 1 file changed, 8 insertions(+) > >> > >> Reviewed-by: Eric Blake <eblake@redhat.com> > >> > >> Are we targetting this for 2.9-rc5, or is it 2.10 material? > > > > At this point, I think it's clearly 2.10. > > Okay. Restating, to make sure I got your reasoning: the removed > assertions of commit e3e0003 imply that 2.9 is not regressing in > behavior, and at this point the worst the code can do without this patch > applied is behave like it's done previously; therefore this patch is not > fixing an observable 2.9 behavior and therefore not worth holding up the > release. Right, basically the new op blockers become ineffective for guest devices after postcopy migration. > But for 2.10, it's absolutely essential, as we have another patch > pending to revert e3e0003 at which point we have a behavior break > without this patch. Correct. Kevin
diff --git a/migration/savevm.c b/migration/savevm.c index 3b19a4a..43fa9bf 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1623,6 +1623,14 @@ static void loadvm_postcopy_handle_run_bh(void *opaque) error_report_err(local_err); } + /* If we get an error here, just don't restart the VM yet. */ + blk_resume_after_migration(&local_err); + if (local_err) { + error_free(local_err); + local_err = NULL; + autostart = false; + } + trace_loadvm_postcopy_handle_run_cpu_sync(); cpu_synchronize_all_post_init();
Commit d35ff5e6 ('block: Ignore guest dev permissions during incoming migration') added blk_resume_after_migration() to the precopy migration path, but neglected to add it to the duplicated code that is used for postcopy migration. This means that the guest device doesn't request the necessary permissions, which ultimately led to failing assertions. Add the missing blk_resume_after_migration() to the postcopy path. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- migration/savevm.c | 8 ++++++++ 1 file changed, 8 insertions(+)