During the vm rebooting, the vm could be paused if the libvirtd is
restarted for some reason, which is not expected. We need continue
fakereboot process if fakereboot flags is true and the vm is in
paused-user status.

Signed-off-by: Bihong Yu <yubihong@huawei.com>
---
 src/qemu/qemu_process.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 832ce164fb..a758b96fa6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8742,13 +8742,15 @@ qemuProcessReconnect(void *opaque)
         goto error;
     }
 
-    /* In case the domain shutdown while we were not running,
-     * we need to finish the shutdown process. And we need to do it after
-     * we have virQEMUCaps filled in.
+    /* In case the domain shutdown or fake reboot while we were not running,
+     * we need to finish the shutdown or fake reboot process. And we need to
+     * do it after we have virQEMUCaps filled in.
      */
     if (state == VIR_DOMAIN_SHUTDOWN ||
         (state == VIR_DOMAIN_PAUSED &&
-         reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN)) {
+         reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN) ||
+        (priv->fakeReboot && state == VIR_DOMAIN_PAUSED &&
+         reason == VIR_DOMAIN_PAUSED_USER)) {
         VIR_DEBUG("Finishing shutdown sequence for domain %s",
                   obj->def->name);
         qemuProcessShutdownOrReboot(driver, obj);
-- 
2.27.0

On 10/25/21 11:04 AM, Bihong Yu wrote:
> During the vm rebooting, the vm could be paused if the libvirtd is
> restarted for some reason, which is not expected. We need continue
> fakereboot process if fakereboot flags is true and the vm is in
> paused-user status.
> 
> Signed-off-by: Bihong Yu <yubihong@huawei.com>
> ---
>  src/qemu/qemu_process.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 832ce164fb..a758b96fa6 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -8742,13 +8742,15 @@ qemuProcessReconnect(void *opaque)
>          goto error;
>      }
>  
> -    /* In case the domain shutdown while we were not running,
> -     * we need to finish the shutdown process. And we need to do it after
> -     * we have virQEMUCaps filled in.
> +    /* In case the domain shutdown or fake reboot while we were not running,
> +     * we need to finish the shutdown or fake reboot process. And we need to
> +     * do it after we have virQEMUCaps filled in.
>       */
>      if (state == VIR_DOMAIN_SHUTDOWN ||
>          (state == VIR_DOMAIN_PAUSED &&
> -         reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN)) {
> +         reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN) ||
> +        (priv->fakeReboot && state == VIR_DOMAIN_PAUSED &&
> +         reason == VIR_DOMAIN_PAUSED_USER)) {

One thing that I don't quite understand is why this new condition checks
for state or reason. I could understand the reason a bit (because domain
is paused after SHUTDOWN event), but the reason? Can you elaborate please?

>          VIR_DEBUG("Finishing shutdown sequence for domain %s",
>                    obj->def->name);
>          qemuProcessShutdownOrReboot(driver, obj);
> 

Michal

On 2021/11/10 17:32, Michal Prívozník wrote:
> On 10/25/21 11:04 AM, Bihong Yu wrote:
>> During the vm rebooting, the vm could be paused if the libvirtd is
>> restarted for some reason, which is not expected. We need continue
>> fakereboot process if fakereboot flags is true and the vm is in
>> paused-user status.
>>
>> Signed-off-by: Bihong Yu <yubihong@huawei.com>
>> ---
>>  src/qemu/qemu_process.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index 832ce164fb..a758b96fa6 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -8742,13 +8742,15 @@ qemuProcessReconnect(void *opaque)
>>          goto error;
>>      }
>>  
>> -    /* In case the domain shutdown while we were not running,
>> -     * we need to finish the shutdown process. And we need to do it after
>> -     * we have virQEMUCaps filled in.
>> +    /* In case the domain shutdown or fake reboot while we were not running,
>> +     * we need to finish the shutdown or fake reboot process. And we need to
>> +     * do it after we have virQEMUCaps filled in.
>>       */
>>      if (state == VIR_DOMAIN_SHUTDOWN ||
>>          (state == VIR_DOMAIN_PAUSED &&
>> -         reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN)) {
>> +         reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN) ||
>> +        (priv->fakeReboot && state == VIR_DOMAIN_PAUSED &&
>> +         reason == VIR_DOMAIN_PAUSED_USER)) {
> 
> One thing that I don't quite understand is why this new condition checks
> for state or reason. I could understand the reason a bit (because domain
> is paused after SHUTDOWN event), but the reason? Can you elaborate please?

Hi, Michal. Thank you for your reply.
  The reason is that: while libvirt reboot vm with ACPI mode, the vm will
undergo the following state changes:
	running -> shutdown -> prelaunch -> running
  If libvirtd reboot after vm prelaunch status and before vm running status,
the qemuProcessUpdateState() will update the vm status to 'VIR_DOMAIN_PAUSED'
and reason to 'VIR_DOMAIN_PAUSED_USER' according to the qemuMonitorGetStatus()
returning result in qemuProcessReconnect().
  So, we need to recognize this scenario and continue to finish rebooting
process.

> 
>>          VIR_DEBUG("Finishing shutdown sequence for domain %s",
>>                    obj->def->name);
>>          qemuProcessShutdownOrReboot(driver, obj);
>>
> 
> Michal
> 
> .
> 

On 11/10/21 2:25 PM, Bihong Yu wrote:
> 
> 
> On 2021/11/10 17:32, Michal Prívozník wrote:
>> On 10/25/21 11:04 AM, Bihong Yu wrote:
>>> During the vm rebooting, the vm could be paused if the libvirtd is
>>> restarted for some reason, which is not expected. We need continue
>>> fakereboot process if fakereboot flags is true and the vm is in
>>> paused-user status.
>>>
>>> Signed-off-by: Bihong Yu <yubihong@huawei.com>
>>> ---
>>>  src/qemu/qemu_process.c | 10 ++++++----
>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>>> index 832ce164fb..a758b96fa6 100644
>>> --- a/src/qemu/qemu_process.c
>>> +++ b/src/qemu/qemu_process.c
>>> @@ -8742,13 +8742,15 @@ qemuProcessReconnect(void *opaque)
>>>          goto error;
>>>      }
>>>  
>>> -    /* In case the domain shutdown while we were not running,
>>> -     * we need to finish the shutdown process. And we need to do it after
>>> -     * we have virQEMUCaps filled in.
>>> +    /* In case the domain shutdown or fake reboot while we were not running,
>>> +     * we need to finish the shutdown or fake reboot process. And we need to
>>> +     * do it after we have virQEMUCaps filled in.
>>>       */
>>>      if (state == VIR_DOMAIN_SHUTDOWN ||
>>>          (state == VIR_DOMAIN_PAUSED &&
>>> -         reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN)) {
>>> +         reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN) ||
>>> +        (priv->fakeReboot && state == VIR_DOMAIN_PAUSED &&
>>> +         reason == VIR_DOMAIN_PAUSED_USER)) {
>>
>> One thing that I don't quite understand is why this new condition checks
>> for state or reason. I could understand the reason a bit (because domain
>> is paused after SHUTDOWN event), but the reason? Can you elaborate please?
> 
> Hi, Michal. Thank you for your reply.
>   The reason is that: while libvirt reboot vm with ACPI mode, the vm will
> undergo the following state changes:
> 	running -> shutdown -> prelaunch -> running
>   If libvirtd reboot after vm prelaunch status and before vm running status,
> the qemuProcessUpdateState() will update the vm status to 'VIR_DOMAIN_PAUSED'
> and reason to 'VIR_DOMAIN_PAUSED_USER' according to the qemuMonitorGetStatus()
> returning result in qemuProcessReconnect().
>   So, we need to recognize this scenario and continue to finish rebooting
> process.

Ah, you are correct. I've missed that.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

Hi, Michal.
  Can you take the time to help me revivew the '[PATCH v1 1/2] qemu_process: set fakereboot flags false after processing fakereboot over'?

Best wishes!
Bihong

On 2021/11/10 21:41, Michal Prívozník wrote:
> On 11/10/21 2:25 PM, Bihong Yu wrote:
>>
>>
>> On 2021/11/10 17:32, Michal Prívozník wrote:
>>> On 10/25/21 11:04 AM, Bihong Yu wrote:
>>>> During the vm rebooting, the vm could be paused if the libvirtd is
>>>> restarted for some reason, which is not expected. We need continue
>>>> fakereboot process if fakereboot flags is true and the vm is in
>>>> paused-user status.
>>>>
>>>> Signed-off-by: Bihong Yu <yubihong@huawei.com>
>>>> ---
>>>>  src/qemu/qemu_process.c | 10 ++++++----
>>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>>>> index 832ce164fb..a758b96fa6 100644
>>>> --- a/src/qemu/qemu_process.c
>>>> +++ b/src/qemu/qemu_process.c
>>>> @@ -8742,13 +8742,15 @@ qemuProcessReconnect(void *opaque)
>>>>          goto error;
>>>>      }
>>>>  
>>>> -    /* In case the domain shutdown while we were not running,
>>>> -     * we need to finish the shutdown process. And we need to do it after
>>>> -     * we have virQEMUCaps filled in.
>>>> +    /* In case the domain shutdown or fake reboot while we were not running,
>>>> +     * we need to finish the shutdown or fake reboot process. And we need to
>>>> +     * do it after we have virQEMUCaps filled in.
>>>>       */
>>>>      if (state == VIR_DOMAIN_SHUTDOWN ||
>>>>          (state == VIR_DOMAIN_PAUSED &&
>>>> -         reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN)) {
>>>> +         reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN) ||
>>>> +        (priv->fakeReboot && state == VIR_DOMAIN_PAUSED &&
>>>> +         reason == VIR_DOMAIN_PAUSED_USER)) {
>>>
>>> One thing that I don't quite understand is why this new condition checks
>>> for state or reason. I could understand the reason a bit (because domain
>>> is paused after SHUTDOWN event), but the reason? Can you elaborate please?
>>
>> Hi, Michal. Thank you for your reply.
>>   The reason is that: while libvirt reboot vm with ACPI mode, the vm will
>> undergo the following state changes:
>> 	running -> shutdown -> prelaunch -> running
>>   If libvirtd reboot after vm prelaunch status and before vm running status,
>> the qemuProcessUpdateState() will update the vm status to 'VIR_DOMAIN_PAUSED'
>> and reason to 'VIR_DOMAIN_PAUSED_USER' according to the qemuMonitorGetStatus()
>> returning result in qemuProcessReconnect().
>>   So, we need to recognize this scenario and continue to finish rebooting
>> process.
> 
> Ah, you are correct. I've missed that.
> 
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> 
> Michal
> 
> .
> 

Hi, Michal.
  I saw that these two patches have been merged. Thank you for your review.

Best wishes!
Bihong

On 2021/11/11 9:26, Bihong Yu wrote:
> Hi, Michal.
>   Can you take the time to help me revivew the '[PATCH v1 1/2] qemu_process: set fakereboot flags false after processing fakereboot over'?
> 
> Best wishes!
> Bihong
> 
> On 2021/11/10 21:41, Michal Prívozník wrote:
>> On 11/10/21 2:25 PM, Bihong Yu wrote:
>>>
>>>
>>> On 2021/11/10 17:32, Michal Prívozník wrote:
>>>> On 10/25/21 11:04 AM, Bihong Yu wrote:
>>>>> During the vm rebooting, the vm could be paused if the libvirtd is
>>>>> restarted for some reason, which is not expected. We need continue
>>>>> fakereboot process if fakereboot flags is true and the vm is in
>>>>> paused-user status.
>>>>>
>>>>> Signed-off-by: Bihong Yu <yubihong@huawei.com>
>>>>> ---
>>>>>  src/qemu/qemu_process.c | 10 ++++++----
>>>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>>>>> index 832ce164fb..a758b96fa6 100644
>>>>> --- a/src/qemu/qemu_process.c
>>>>> +++ b/src/qemu/qemu_process.c
>>>>> @@ -8742,13 +8742,15 @@ qemuProcessReconnect(void *opaque)
>>>>>          goto error;
>>>>>      }
>>>>>  
>>>>> -    /* In case the domain shutdown while we were not running,
>>>>> -     * we need to finish the shutdown process. And we need to do it after
>>>>> -     * we have virQEMUCaps filled in.
>>>>> +    /* In case the domain shutdown or fake reboot while we were not running,
>>>>> +     * we need to finish the shutdown or fake reboot process. And we need to
>>>>> +     * do it after we have virQEMUCaps filled in.
>>>>>       */
>>>>>      if (state == VIR_DOMAIN_SHUTDOWN ||
>>>>>          (state == VIR_DOMAIN_PAUSED &&
>>>>> -         reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN)) {
>>>>> +         reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN) ||
>>>>> +        (priv->fakeReboot && state == VIR_DOMAIN_PAUSED &&
>>>>> +         reason == VIR_DOMAIN_PAUSED_USER)) {
>>>>
>>>> One thing that I don't quite understand is why this new condition checks
>>>> for state or reason. I could understand the reason a bit (because domain
>>>> is paused after SHUTDOWN event), but the reason? Can you elaborate please?
>>>
>>> Hi, Michal. Thank you for your reply.
>>>   The reason is that: while libvirt reboot vm with ACPI mode, the vm will
>>> undergo the following state changes:
>>> 	running -> shutdown -> prelaunch -> running
>>>   If libvirtd reboot after vm prelaunch status and before vm running status,
>>> the qemuProcessUpdateState() will update the vm status to 'VIR_DOMAIN_PAUSED'
>>> and reason to 'VIR_DOMAIN_PAUSED_USER' according to the qemuMonitorGetStatus()
>>> returning result in qemuProcessReconnect().
>>>   So, we need to recognize this scenario and continue to finish rebooting
>>> process.
>>
>> Ah, you are correct. I've missed that.
>>
>> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
>>
>> Michal
>>
>> .
>>