| Message ID | 1738788841-211843-1-git-send-email-steven.sistare@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | migration: use parameters.mode in cpr_state_save | expand |
On Wed, Feb 05, 2025 at 12:54:01PM -0800, Steve Sistare wrote: > qmp_migrate guarantees that cpr_channel is not null for > MIG_MODE_CPR_TRANSFER when cpr_state_save is called: > > qmp_migrate() > if (s->parameters.mode == MIG_MODE_CPR_TRANSFER && !cpr_channel) { > return; > } > cpr_state_save(cpr_channel) > > but cpr_state_save checks for mode differently before using channel, > and Coverity cannot infer that they are equivalent in outgoing QEMU, > and warns that channel may be NULL: > > cpr_state_save(channel) > MigMode mode = migrate_mode(); > if (mode == MIG_MODE_CPR_TRANSFER) { > f = cpr_transfer_output(channel, errp); > > To make Coverity happy, use parameters.mode in cpr_state_save. > > Resolves: Coverity CID 1590980 > Reported-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > --- > migration/cpr.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/migration/cpr.c b/migration/cpr.c > index 584b0b9..7f20bd5 100644 > --- a/migration/cpr.c > +++ b/migration/cpr.c > @@ -8,6 +8,7 @@ > #include "qemu/osdep.h" > #include "qapi/error.h" > #include "migration/cpr.h" > +#include "migration/migration.h" > #include "migration/misc.h" > #include "migration/options.h" > #include "migration/qemu-file.h" > @@ -132,7 +133,7 @@ int cpr_state_save(MigrationChannel *channel, Error **errp) > { > int ret; > QEMUFile *f; > - MigMode mode = migrate_mode(); > + MigMode mode = migrate_get_current()->parameters.mode; Are we sure this can make coverity happy? Another more straightforward change is caching migrate mode in qmp_migrate() and also check that before invoking cpr_state_save(). Thanks, > > trace_cpr_state_save(MigMode_str(mode)); > > -- > 1.8.3.1 >
On 2/5/2025 4:28 PM, Peter Xu wrote: > On Wed, Feb 05, 2025 at 12:54:01PM -0800, Steve Sistare wrote: >> qmp_migrate guarantees that cpr_channel is not null for >> MIG_MODE_CPR_TRANSFER when cpr_state_save is called: >> >> qmp_migrate() >> if (s->parameters.mode == MIG_MODE_CPR_TRANSFER && !cpr_channel) { >> return; >> } >> cpr_state_save(cpr_channel) >> >> but cpr_state_save checks for mode differently before using channel, >> and Coverity cannot infer that they are equivalent in outgoing QEMU, >> and warns that channel may be NULL: >> >> cpr_state_save(channel) >> MigMode mode = migrate_mode(); >> if (mode == MIG_MODE_CPR_TRANSFER) { >> f = cpr_transfer_output(channel, errp); >> >> To make Coverity happy, use parameters.mode in cpr_state_save. >> >> Resolves: Coverity CID 1590980 >> Reported-by: Peter Maydell <peter.maydell@linaro.org> >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> >> --- >> migration/cpr.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/migration/cpr.c b/migration/cpr.c >> index 584b0b9..7f20bd5 100644 >> --- a/migration/cpr.c >> +++ b/migration/cpr.c >> @@ -8,6 +8,7 @@ >> #include "qemu/osdep.h" >> #include "qapi/error.h" >> #include "migration/cpr.h" >> +#include "migration/migration.h" >> #include "migration/misc.h" >> #include "migration/options.h" >> #include "migration/qemu-file.h" >> @@ -132,7 +133,7 @@ int cpr_state_save(MigrationChannel *channel, Error **errp) >> { >> int ret; >> QEMUFile *f; >> - MigMode mode = migrate_mode(); >> + MigMode mode = migrate_get_current()->parameters.mode; > > Are we sure this can make coverity happy? It should, based on Peter Maydell's analysis, but I would appreciate if he could apply and test the fix. > Another more straightforward change is caching migrate mode in > qmp_migrate() and also check that before invoking cpr_state_save(). Surely anyone would consider my one-line change to be straight forward. - Steve >> trace_cpr_state_save(MigMode_str(mode)); >> >> -- >> 1.8.3.1 >> >
diff --git a/migration/cpr.c b/migration/cpr.c index 584b0b9..7f20bd5 100644 --- a/migration/cpr.c +++ b/migration/cpr.c @@ -8,6 +8,7 @@ #include "qemu/osdep.h" #include "qapi/error.h" #include "migration/cpr.h" +#include "migration/migration.h" #include "migration/misc.h" #include "migration/options.h" #include "migration/qemu-file.h" @@ -132,7 +133,7 @@ int cpr_state_save(MigrationChannel *channel, Error **errp) { int ret; QEMUFile *f; - MigMode mode = migrate_mode(); + MigMode mode = migrate_get_current()->parameters.mode; trace_cpr_state_save(MigMode_str(mode));
qmp_migrate guarantees that cpr_channel is not null for MIG_MODE_CPR_TRANSFER when cpr_state_save is called: qmp_migrate() if (s->parameters.mode == MIG_MODE_CPR_TRANSFER && !cpr_channel) { return; } cpr_state_save(cpr_channel) but cpr_state_save checks for mode differently before using channel, and Coverity cannot infer that they are equivalent in outgoing QEMU, and warns that channel may be NULL: cpr_state_save(channel) MigMode mode = migrate_mode(); if (mode == MIG_MODE_CPR_TRANSFER) { f = cpr_transfer_output(channel, errp); To make Coverity happy, use parameters.mode in cpr_state_save. Resolves: Coverity CID 1590980 Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> --- migration/cpr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)