migration: equation is more proper than and to check LOADVM_QUIT

[Qemu-devel] [PATCH] migration: equation is more proper than and to check LOADVM_QUIT

Posted by Wei Yang 4 years, 9 months ago

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(-)

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

Re: [Qemu-devel] [PATCH] migration: equation is more proper than and to check LOADVM_QUIT

Posted by Dr. David Alan Gilbert 4 years, 9 months ago

* 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

Re: [Qemu-devel] [PATCH] migration: equation is more proper than and to check LOADVM_QUIT

Posted by Dr. David Alan Gilbert 4 years, 9 months ago

* 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

Re: [Qemu-devel] [PATCH] migration: equation is more proper than and to check LOADVM_QUIT

Posted by Wei Yang 4 years, 9 months ago

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
Help you, Help me