| Message ID | 20240917185802.15619-2-farosas@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration/multifd: Fix rb->receivedmap cleanup race | expand |
On Tue, Sep 17, 2024 at 03:58:01PM -0300, Fabiano Rosas wrote: > There are two qemu_loadvm_state_cleanup() calls that were introduced > when qemu_loadvm_state_setup() was still called before loading the > configuration section, so there was state to be cleaned up if the > header checks failed. > > However, commit 9e14b84908 ("migration/savevm: load_header before > load_setup") has moved that configuration section part to > qemu_loadvm_state_header() which now happens before > qemu_loadvm_state_setup(). > > Remove the cleanup calls that are now misplaced. > > Fixes: 9e14b84908 ("migration/savevm: load_header before load_setup") Considering it's a cleanup, do you mind if I further remove this Fixes but just mention it in commit message (so as to make Michael's life easier when backport)? > Reviewed-by: Peter Xu <peterx@redhat.com> > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > migration/savevm.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/migration/savevm.c b/migration/savevm.c > index d500eae979..d0759694fd 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -2732,13 +2732,11 @@ static int qemu_loadvm_state_header(QEMUFile *f) > if (migrate_get_current()->send_configuration) { > if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { > error_report("Configuration section missing"); > - qemu_loadvm_state_cleanup(); > return -EINVAL; > } > ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0); > > if (ret) { > - qemu_loadvm_state_cleanup(); > return ret; > } > } > -- > 2.35.3 >
Peter Xu <peterx@redhat.com> writes: > On Tue, Sep 17, 2024 at 03:58:01PM -0300, Fabiano Rosas wrote: >> There are two qemu_loadvm_state_cleanup() calls that were introduced >> when qemu_loadvm_state_setup() was still called before loading the >> configuration section, so there was state to be cleaned up if the >> header checks failed. >> >> However, commit 9e14b84908 ("migration/savevm: load_header before >> load_setup") has moved that configuration section part to >> qemu_loadvm_state_header() which now happens before >> qemu_loadvm_state_setup(). >> >> Remove the cleanup calls that are now misplaced. >> >> Fixes: 9e14b84908 ("migration/savevm: load_header before load_setup") > > Considering it's a cleanup, do you mind if I further remove this Fixes but > just mention it in commit message (so as to make Michael's life easier when > backport)? Sure, go ahead. > >> Reviewed-by: Peter Xu <peterx@redhat.com> >> Signed-off-by: Fabiano Rosas <farosas@suse.de> >> --- >> migration/savevm.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/migration/savevm.c b/migration/savevm.c >> index d500eae979..d0759694fd 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -2732,13 +2732,11 @@ static int qemu_loadvm_state_header(QEMUFile *f) >> if (migrate_get_current()->send_configuration) { >> if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { >> error_report("Configuration section missing"); >> - qemu_loadvm_state_cleanup(); >> return -EINVAL; >> } >> ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0); >> >> if (ret) { >> - qemu_loadvm_state_cleanup(); >> return ret; >> } >> } >> -- >> 2.35.3 >>
diff --git a/migration/savevm.c b/migration/savevm.c index d500eae979..d0759694fd 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -2732,13 +2732,11 @@ static int qemu_loadvm_state_header(QEMUFile *f) if (migrate_get_current()->send_configuration) { if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { error_report("Configuration section missing"); - qemu_loadvm_state_cleanup(); return -EINVAL; } ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0); if (ret) { - qemu_loadvm_state_cleanup(); return ret; } }
There are two qemu_loadvm_state_cleanup() calls that were introduced when qemu_loadvm_state_setup() was still called before loading the configuration section, so there was state to be cleaned up if the header checks failed. However, commit 9e14b84908 ("migration/savevm: load_header before load_setup") has moved that configuration section part to qemu_loadvm_state_header() which now happens before qemu_loadvm_state_setup(). Remove the cleanup calls that are now misplaced. Fixes: 9e14b84908 ("migration/savevm: load_header before load_setup") Reviewed-by: Peter Xu <peterx@redhat.com> Signed-off-by: Fabiano Rosas <farosas@suse.de> --- migration/savevm.c | 2 -- 1 file changed, 2 deletions(-)