[PATCH v1 0/2] qemu_process: ensure the reboot process is performed completely

When the vm reboot in ACPI mode, the vm has a certain probability to be shutoff or paused status if the libvirtd is restarted for some reason, which is not expected. This patchset ensure the reboot process is performed completely. Bihong Yu (2): qemu_process: set fakereboot flags false after processing fakereboot over qemu_process: continue to process fakereboot after restarting libvirtd src/qemu/qemu_process.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) -- 2.27.0

During the vm rebooting, the vm could be shut down if the libvirtd is restarted for some reason, which is not expected. We move set fakereboot flags false after processing fakereboot over, so we can ensure that fakereboot process have been executed. Signed-off-by: Bihong Yu <yubihong@huawei.com> --- src/qemu/qemu_process.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6027b30405..832ce164fb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -523,6 +523,7 @@ qemuProcessFakeReboot(void *opaque) cleanup: priv->pausedShutdown = false; + qemuDomainSetFakeReboot(driver, vm, false); if (ret == -1) ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCE)); virDomainObjEndAPI(&vm); @@ -540,7 +541,6 @@ qemuProcessShutdownOrReboot(virQEMUDriver *driver, g_autofree char *name = g_strdup_printf("reboot-%s", vm->def->name); virThread th; - qemuDomainSetFakeReboot(driver, vm, false); virObjectRef(vm); if (virThreadCreateFull(&th, false, @@ -551,6 +551,7 @@ qemuProcessShutdownOrReboot(virQEMUDriver *driver, VIR_ERROR(_("Failed to create reboot thread, killing domain")); ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_NOWAIT)); priv->pausedShutdown = false; + qemuDomainSetFakeReboot(driver, vm, false); virObjectUnref(vm); } } else { -- 2.27.0

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
.
participants (2)
-
Bihong Yu
-
Michal Prívozník