Message ID | 20190718064257.29218-1-richardw.yang@linux.intel.com |
---|---|
State | New |
Headers | show |
Series | migration: equation is more proper than and to check LOADVM_QUIT | expand |
* Wei Yang (richardw.yang@linux.intel.com) wrote: > LOADVM_QUIT allows a command to quit all layers of nested loadvm loops, > while current return value check is not that proper even it works now. > > Current return value check "ret & LOADVM_QUIT" would return true if > bit[0] is 1. This would be true when ret is -1 which is used to indicate > an error of handling a command. > > Since there is only one place return LOADVM_QUIT and no other > combination of return value, use "ret == LOADVM_QUIT" would be more > proper. Yes, when I first wrote this it was more complex with a few flags, and they all got removed. > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > migration/savevm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/migration/savevm.c b/migration/savevm.c > index 1a9b1f411e..25fe7ea05a 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -2429,7 +2429,7 @@ retry: > case QEMU_VM_COMMAND: > ret = loadvm_process_command(f); > trace_qemu_loadvm_state_section_command(ret); > - if ((ret < 0) || (ret & LOADVM_QUIT)) { > + if ((ret < 0) || (ret == LOADVM_QUIT)) { > goto out; > } > break; > -- > 2.17.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Fri, Jul 19, 2019 at 07:41:28PM +0100, Dr. David Alan Gilbert wrote: >* Wei Yang (richardw.yang@linux.intel.com) wrote: >> LOADVM_QUIT allows a command to quit all layers of nested loadvm loops, >> while current return value check is not that proper even it works now. >> >> Current return value check "ret & LOADVM_QUIT" would return true if >> bit[0] is 1. This would be true when ret is -1 which is used to indicate >> an error of handling a command. >> >> Since there is only one place return LOADVM_QUIT and no other >> combination of return value, use "ret == LOADVM_QUIT" would be more >> proper. > >Yes, when I first wrote this it was more complex with a few flags, and >they all got removed. > Ah, life is much easier now :-) >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > >Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > >> --- >> migration/savevm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/migration/savevm.c b/migration/savevm.c >> index 1a9b1f411e..25fe7ea05a 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -2429,7 +2429,7 @@ retry: >> case QEMU_VM_COMMAND: >> ret = loadvm_process_command(f); >> trace_qemu_loadvm_state_section_command(ret); >> - if ((ret < 0) || (ret & LOADVM_QUIT)) { >> + if ((ret < 0) || (ret == LOADVM_QUIT)) { >> goto out; >> } >> break; >> -- >> 2.17.1 >> >-- >Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
* Wei Yang (richardw.yang@linux.intel.com) wrote: > LOADVM_QUIT allows a command to quit all layers of nested loadvm loops, > while current return value check is not that proper even it works now. > > Current return value check "ret & LOADVM_QUIT" would return true if > bit[0] is 1. This would be true when ret is -1 which is used to indicate > an error of handling a command. > > Since there is only one place return LOADVM_QUIT and no other > combination of return value, use "ret == LOADVM_QUIT" would be more > proper. > > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> Queued > --- > migration/savevm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/migration/savevm.c b/migration/savevm.c > index 1a9b1f411e..25fe7ea05a 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -2429,7 +2429,7 @@ retry: > case QEMU_VM_COMMAND: > ret = loadvm_process_command(f); > trace_qemu_loadvm_state_section_command(ret); > - if ((ret < 0) || (ret & LOADVM_QUIT)) { > + if ((ret < 0) || (ret == LOADVM_QUIT)) { > goto out; > } > break; > -- > 2.17.1 > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/migration/savevm.c b/migration/savevm.c index 1a9b1f411e..25fe7ea05a 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -2429,7 +2429,7 @@ retry: case QEMU_VM_COMMAND: ret = loadvm_process_command(f); trace_qemu_loadvm_state_section_command(ret); - if ((ret < 0) || (ret & LOADVM_QUIT)) { + if ((ret < 0) || (ret == LOADVM_QUIT)) { goto out; } break;
LOADVM_QUIT allows a command to quit all layers of nested loadvm loops, while current return value check is not that proper even it works now. Current return value check "ret & LOADVM_QUIT" would return true if bit[0] is 1. This would be true when ret is -1 which is used to indicate an error of handling a command. Since there is only one place return LOADVM_QUIT and no other combination of return value, use "ret == LOADVM_QUIT" would be more proper. Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> --- migration/savevm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)