target/i386: kvm: just return after migrate_add_blocker failed

[Qemu-devel] [PATCH] target/i386: kvm: just return after migrate_add_blocker failed

Li Qiang posted 1 patch 7 years, 1 month ago

Download mbox

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20181006091816.7659-1-liq3ea@163.com

Test docker-clang@ubuntu failed

Test checkpatch passed

target/i386/kvm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Expand all Fold all

[Qemu-devel] [PATCH] target/i386: kvm: just return after migrate_add_blocker failed

Posted by Li Qiang 7 years, 1 month ago

When migrate_add_blocker failed, the invtsc_mig_blocker is not
appended so no need to remove. This can save several instructions.

Signed-off-by: Li Qiang <liq3ea@163.com>
---
 target/i386/kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 0b2a07d3a4..6ba84a39f3 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1153,7 +1153,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
             if (local_err) {
                 error_report_err(local_err);
                 error_free(invtsc_mig_blocker);
-                goto fail;
+                return r;
             }
             /* for savevm */
             vmstate_x86_cpu.unmigratable = 1;
-- 
2.17.1

Re: [Qemu-devel] [PATCH] target/i386: kvm: just return after migrate_add_blocker failed

Posted by Dr. David Alan Gilbert 7 years ago

* Li Qiang (liq3ea@163.com) wrote:
> When migrate_add_blocker failed, the invtsc_mig_blocker is not
> appended so no need to remove. This can save several instructions.
> 
> Signed-off-by: Li Qiang <liq3ea@163.com>
> ---
>  target/i386/kvm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 0b2a07d3a4..6ba84a39f3 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1153,7 +1153,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>              if (local_err) {
>                  error_report_err(local_err);
>                  error_free(invtsc_mig_blocker);
> -                goto fail;
> +                return r;

So I think that's OK, however, I wonder if fail: should be
doing some more cleanup?
For example, I notice we've added cpu_update_state to the change state
handler's before that point, should that be removed?

Dave

>              }
>              /* for savevm */
>              vmstate_x86_cpu.unmigratable = 1;
> -- 
> 2.17.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK