Patchwork Drop redundant resume_all_vcpus from main

login
register
mail settings
Submitter Jan Kiszka
Date Aug. 20, 2012, 6:11 p.m.
Message ID <50327DD8.8070205@siemens.com>
Download mbox | patch
Permalink /patch/178912/
State New
Headers show

Comments

Jan Kiszka - Aug. 20, 2012, 6:11 p.m.
VCPUs are either resumed directly via vm_start, after the incoming
migration is done, or when a continue command is issued. We don't need
the explicit resume before entering main_loop.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

I was adding nesting support to pause/resume_all_vcpus, and that
stumbled over the imbalance below.

 vl.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
Paolo Bonzini - Aug. 21, 2012, 7:01 a.m.
Il 20/08/2012 20:11, Jan Kiszka ha scritto:
> VCPUs are either resumed directly via vm_start, after the incoming
> migration is done, or when a continue command is issued. We don't need
> the explicit resume before entering main_loop.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> I was adding nesting support to pause/resume_all_vcpus, and that
> stumbled over the imbalance below.
> 
>  vl.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index ebee867..231d3ab 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3757,7 +3757,6 @@ int main(int argc, char **argv, char **envp)
>  
>      os_setup_post();
>  
> -    resume_all_vcpus();
>      main_loop();
>      bdrv_close_all();
>      pause_all_vcpus();
> 

Makes sense.  Do we need a "main loop and similar" tree, or can that
tree be just uq/master now that qemu-kvm.c is dying?

Paolo
Jan Kiszka - Aug. 21, 2012, 7:24 a.m.
On 2012-08-21 09:01, Paolo Bonzini wrote:
> Il 20/08/2012 20:11, Jan Kiszka ha scritto:
>> VCPUs are either resumed directly via vm_start, after the incoming
>> migration is done, or when a continue command is issued. We don't need
>> the explicit resume before entering main_loop.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> I was adding nesting support to pause/resume_all_vcpus, and that
>> stumbled over the imbalance below.
>>
>>  vl.c |    1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index ebee867..231d3ab 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3757,7 +3757,6 @@ int main(int argc, char **argv, char **envp)
>>  
>>      os_setup_post();
>>  
>> -    resume_all_vcpus();
>>      main_loop();
>>      bdrv_close_all();
>>      pause_all_vcpus();
>>
> 
> Makes sense.  Do we need a "main loop and similar" tree, or can that
> tree be just uq/master now that qemu-kvm.c is dying?

I'm not sure if this qualifies for uq/master. On the other hand, all the
efforts to refactor locking and make QEMU more scalable would like be
happy to have a home. Can be uq/master, but they will not only affect
KVM in the end.

Jan
Jan Kiszka - May 2, 2013, 11:20 a.m.
On 2012-08-21 09:01, Paolo Bonzini wrote:
> Il 20/08/2012 20:11, Jan Kiszka ha scritto:
>> VCPUs are either resumed directly via vm_start, after the incoming
>> migration is done, or when a continue command is issued. We don't need
>> the explicit resume before entering main_loop.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> I was adding nesting support to pause/resume_all_vcpus, and that
>> stumbled over the imbalance below.
>>
>>  vl.c |    1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index ebee867..231d3ab 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3757,7 +3757,6 @@ int main(int argc, char **argv, char **envp)
>>  
>>      os_setup_post();
>>  
>> -    resume_all_vcpus();
>>      main_loop();
>>      bdrv_close_all();
>>      pause_all_vcpus();
>>
> 
> Makes sense.  Do we need a "main loop and similar" tree, or can that
> tree be just uq/master now that qemu-kvm.c is dying?
> 
> Paolo
> 

Just noticed that this cleanup didn't make it into upstream back then.
Not truly trivial, but also not really risky.

Jan
Andreas Färber - May 2, 2013, 11:55 a.m.
Am 02.05.2013 13:20, schrieb Jan Kiszka:
> On 2012-08-21 09:01, Paolo Bonzini wrote:
>> Il 20/08/2012 20:11, Jan Kiszka ha scritto:
>>> VCPUs are either resumed directly via vm_start, after the incoming
>>> migration is done, or when a continue command is issued. We don't need
>>> the explicit resume before entering main_loop.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>
>>> I was adding nesting support to pause/resume_all_vcpus, and that
>>> stumbled over the imbalance below.
>>>
>>>  vl.c |    1 -
>>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/vl.c b/vl.c
>>> index ebee867..231d3ab 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -3757,7 +3757,6 @@ int main(int argc, char **argv, char **envp)
>>>  
>>>      os_setup_post();
>>>  
>>> -    resume_all_vcpus();
>>>      main_loop();
>>>      bdrv_close_all();
>>>      pause_all_vcpus();
>>>
>>
>> Makes sense.  Do we need a "main loop and similar" tree, or can that
>> tree be just uq/master now that qemu-kvm.c is dying?
> 
> Just noticed that this cleanup didn't make it into upstream back then.
> Not truly trivial, but also not really risky.

Since I happened to touch that CPU function just yesterday and Paolo and
me seem to agree the call is superfluous, applying it to qom-cpu:

https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Thanks,
Andreas
Jan Kiszka - May 2, 2013, 12:06 p.m.
On 2013-05-02 13:55, Andreas Färber wrote:
> Am 02.05.2013 13:20, schrieb Jan Kiszka:
>> On 2012-08-21 09:01, Paolo Bonzini wrote:
>>> Il 20/08/2012 20:11, Jan Kiszka ha scritto:
>>>> VCPUs are either resumed directly via vm_start, after the incoming
>>>> migration is done, or when a continue command is issued. We don't need
>>>> the explicit resume before entering main_loop.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>>
>>>> I was adding nesting support to pause/resume_all_vcpus, and that
>>>> stumbled over the imbalance below.
>>>>
>>>>  vl.c |    1 -
>>>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/vl.c b/vl.c
>>>> index ebee867..231d3ab 100644
>>>> --- a/vl.c
>>>> +++ b/vl.c
>>>> @@ -3757,7 +3757,6 @@ int main(int argc, char **argv, char **envp)
>>>>  
>>>>      os_setup_post();
>>>>  
>>>> -    resume_all_vcpus();
>>>>      main_loop();
>>>>      bdrv_close_all();
>>>>      pause_all_vcpus();
>>>>
>>>
>>> Makes sense.  Do we need a "main loop and similar" tree, or can that
>>> tree be just uq/master now that qemu-kvm.c is dying?
>>
>> Just noticed that this cleanup didn't make it into upstream back then.
>> Not truly trivial, but also not really risky.
> 
> Since I happened to touch that CPU function just yesterday and Paolo and
> me seem to agree the call is superfluous, applying it to qom-cpu:
> 
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Perfect!

Jan

Patch

diff --git a/vl.c b/vl.c
index ebee867..231d3ab 100644
--- a/vl.c
+++ b/vl.c
@@ -3757,7 +3757,6 @@  int main(int argc, char **argv, char **envp)
 
     os_setup_post();
 
-    resume_all_vcpus();
     main_loop();
     bdrv_close_all();
     pause_all_vcpus();