[libvirt] [PATCH v2 1/1] qemu: hide details of fake reboot
If we use fake reboot then domain goes thru running->shutdown->running state changes with shutdown state only for short period of time. At least this is implementation details leaking into API. And also there is one real case when this is not convinient. I'm doing a backup with the help of temporary block snapshot (with the help of qemu's API which is used in the newly created libvirt's backup API). If guest is shutdowned I want to continue to backup so I don't kill the process and domain is in shutdown state. Later when backup is finished I want to destroy qemu process. So I check if it is in shutdowned state and destroy it if it is. Now if instead of shutdown domain got fake reboot then I can destroy process in the middle of fake reboot process. After shutdown event we also get stop event and now as domain state is running it will be transitioned to paused state and back to running later. Though this is not critical for the described case I guess it is better not to leak these details to user too. So let's leave domain in running state on stop event if fake reboot is in process. Reconnection code handles this patch without modification. It detects that qemu is not running due to shutdown and then calls qemuProcessShutdownOrReboot which reboots as fake reboot flag is set. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- Changes from v1[1]: - rebase on current master - add comments - use special flag to check if we should go paused or not* - add notes about reconnection to commit message * Using just fake reboot flag is not reliable. What if ACPI shutdown is ignored by guest? Reboot flag will remain set and now domain state will remain running on plain pause. [1] https://www.redhat.com/archives/libvir-list/2019-October/msg01827.html src/qemu/qemu_domain.h | 1 + src/qemu/qemu_process.c | 61 ++++++++++++++++++++++++----------------- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index a32852047c..a39b9546ae 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -319,6 +319,7 @@ struct _qemuDomainObjPrivate { char *lockState; bool fakeReboot; + bool pausedShutdown; virTristateBool allowReboot; int jobs_queued; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7e1db50e8f..3e5fe3b6de 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -505,6 +505,7 @@ qemuProcessFakeReboot(void *opaque) qemuDomainObjEndJob(driver, vm); cleanup: + priv->pausedShutdown = false; if (ret == -1) ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCE)); virDomainObjEndAPI(&vm); @@ -528,6 +529,7 @@ qemuProcessShutdownOrReboot(virQEMUDriverPtr driver, vm) < 0) { VIR_ERROR(_("Failed to create reboot thread, killing domain")); ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_NOWAIT)); + priv->pausedShutdown = false; virObjectUnref(vm); } } else { @@ -589,35 +591,41 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon G_GNUC_UNUSED, goto unlock; } - VIR_DEBUG("Transitioned guest %s to shutdown state", - vm->def->name); - virDomainObjSetState(vm, - VIR_DOMAIN_SHUTDOWN, - VIR_DOMAIN_SHUTDOWN_UNKNOWN); + /* In case of fake reboot qemu shutdown state is transient so don't + * change domain state nor send events. */ + if (!priv->fakeReboot) { + VIR_DEBUG("Transitioned guest %s to shutdown state", + vm->def->name); + virDomainObjSetState(vm, + VIR_DOMAIN_SHUTDOWN, + VIR_DOMAIN_SHUTDOWN_UNKNOWN); - switch (guest_initiated) { - case VIR_TRISTATE_BOOL_YES: - detail = VIR_DOMAIN_EVENT_SHUTDOWN_GUEST; - break; + switch (guest_initiated) { + case VIR_TRISTATE_BOOL_YES: + detail = VIR_DOMAIN_EVENT_SHUTDOWN_GUEST; + break; - case VIR_TRISTATE_BOOL_NO: - detail = VIR_DOMAIN_EVENT_SHUTDOWN_HOST; - break; + case VIR_TRISTATE_BOOL_NO: + detail = VIR_DOMAIN_EVENT_SHUTDOWN_HOST; + break; - case VIR_TRISTATE_BOOL_ABSENT: - case VIR_TRISTATE_BOOL_LAST: - default: - detail = VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED; - break; - } + case VIR_TRISTATE_BOOL_ABSENT: + case VIR_TRISTATE_BOOL_LAST: + default: + detail = VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED; + break; + } - event = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_SHUTDOWN, - detail); + event = virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_SHUTDOWN, + detail); - if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) { - VIR_WARN("Unable to save status on vm %s after state change", - vm->def->name); + if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) { + VIR_WARN("Unable to save status on vm %s after state change", + vm->def->name); + } + } else { + priv->pausedShutdown = true; } if (priv->agent) @@ -651,7 +659,10 @@ qemuProcessHandleStop(qemuMonitorPtr mon G_GNUC_UNUSED, reason = priv->pausedReason; priv->pausedReason = VIR_DOMAIN_PAUSED_UNKNOWN; - if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + /* In case of fake reboot qemu paused state is transient so don't + * reveal it in domain state nor sent events */ + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING && + !priv->pausedShutdown) { if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) { if (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY) reason = VIR_DOMAIN_PAUSED_POSTCOPY; -- 2.23.0
On 12/19/19 5:50 AM, Nikolay Shirokovskiy wrote:
If we use fake reboot then domain goes thru running->shutdown->running state changes with shutdown state only for short period of time. At least this is implementation details leaking into API. And also there is one real case when this is not convinient. I'm doing a backup with the help of temporary block snapshot (with the help of qemu's API which is used in the newly created libvirt's backup API). If guest is shutdowned I want to continue to backup so I don't kill the process and domain is in shutdown state. Later when backup is finished I want to destroy qemu process. So I check if it is in shutdowned state and destroy it if it is. Now if instead of shutdown domain got fake reboot then I can destroy process in the middle of fake reboot process.
After shutdown event we also get stop event and now as domain state is running it will be transitioned to paused state and back to running later. Though this is not critical for the described case I guess it is better not to leak these details to user too. So let's leave domain in running state on stop event if fake reboot is in process.
Reconnection code handles this patch without modification. It detects that qemu is not running due to shutdown and then calls qemuProcessShutdownOrReboot which reboots as fake reboot flag is set.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole
On Thu, 19 Dec 2019 13:50:19 +0300 Nikolay Shirokovskiy wrote:
If we use fake reboot then domain goes thru running->shutdown->running state changes with shutdown state only for short period of time. At least this is implementation details leaking into API. And also there is one real case when this is not convinient. I'm doing a backup with the help of temporary block snapshot (with the help of qemu's API which is used in the newly created libvirt's backup API). If guest is shutdowned I want to continue to backup so I don't kill the process and domain is in shutdown state. Later when backup is finished I want to destroy qemu process. So I check if it is in shutdowned state and destroy it if it is. Now if instead of shutdown domain got fake reboot then I can destroy process in the middle of fake reboot process.
After shutdown event we also get stop event and now as domain state is running it will be transitioned to paused state and back to running later. Though this is not critical for the described case I guess it is better not to leak these details to user too. So let's leave domain in running state on stop event if fake reboot is in process.
Reconnection code handles this patch without modification. It detects that qemu is not running due to shutdown and then calls qemuProcessShutdownOrReboot which reboots as fake reboot flag is set.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Question regarding this change: in the on-line documentation for virDomainReboot(), it states: Due to implementation limitations in some drivers (the qemu driver, for instance) it is not advised to migrate or save a guest that is rebooting as a result of this API. Migrating such a guest can lead to a plain shutdown on the destination. Is this still the case? In any event, how does one know when the reboot has finished (assuming the VM reacted to it)? TIA, -d
On 07.04.2020 20:31, Don Koch wrote:
On Thu, 19 Dec 2019 13:50:19 +0300 Nikolay Shirokovskiy wrote:
If we use fake reboot then domain goes thru running->shutdown->running state changes with shutdown state only for short period of time. At least this is implementation details leaking into API. And also there is one real case when this is not convinient. I'm doing a backup with the help of temporary block snapshot (with the help of qemu's API which is used in the newly created libvirt's backup API). If guest is shutdowned I want to continue to backup so I don't kill the process and domain is in shutdown state. Later when backup is finished I want to destroy qemu process. So I check if it is in shutdowned state and destroy it if it is. Now if instead of shutdown domain got fake reboot then I can destroy process in the middle of fake reboot process.
After shutdown event we also get stop event and now as domain state is running it will be transitioned to paused state and back to running later. Though this is not critical for the described case I guess it is better not to leak these details to user too. So let's leave domain in running state on stop event if fake reboot is in process.
Reconnection code handles this patch without modification. It detects that qemu is not running due to shutdown and then calls qemuProcessShutdownOrReboot which reboots as fake reboot flag is set.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Question regarding this change: in the on-line documentation for virDomainReboot(), it states:
Due to implementation limitations in some drivers (the qemu driver, for instance) it is not advised to migrate or save a guest that is rebooting as a result of this API. Migrating such a guest can lead to a plain shutdown on the destination.
Is this still the case?
Hi, Don. Yeah this is still the case.
In any event, how does one know when the reboot has finished (assuming the VM reacted to it)?
Unfortunately there is no event for reboot. Before the patch there was SHUTDOWN event but only when reboot is done thru ACPI. Now when fake reboot impl is more incapsulated there is no more SHUTDOWN event just as for reboot from guest. Nikolay
On Wed, 8 Apr 2020 10:34:08 +0300 nshirokovskiy wrote:
On 07.04.2020 20:31, Don Koch wrote:
On Thu, 19 Dec 2019 13:50:19 +0300 Nikolay Shirokovskiy wrote:
If we use fake reboot then domain goes thru running->shutdown->running state changes with shutdown state only for short period of time. At least this is implementation details leaking into API. And also there is one real case when this is not convinient. I'm doing a backup with the help of temporary block snapshot (with the help of qemu's API which is used in the newly created libvirt's backup API). If guest is shutdowned I want to continue to backup so I don't kill the process and domain is in shutdown state. Later when backup is finished I want to destroy qemu process. So I check if it is in shutdowned state and destroy it if it is. Now if instead of shutdown domain got fake reboot then I can destroy process in the middle of fake reboot process.
After shutdown event we also get stop event and now as domain state is running it will be transitioned to paused state and back to running later. Though this is not critical for the described case I guess it is better not to leak these details to user too. So let's leave domain in running state on stop event if fake reboot is in process.
Reconnection code handles this patch without modification. It detects that qemu is not running due to shutdown and then calls qemuProcessShutdownOrReboot which reboots as fake reboot flag is set.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Question regarding this change: in the on-line documentation for virDomainReboot(), it states:
Due to implementation limitations in some drivers (the qemu driver, for instance) it is not advised to migrate or save a guest that is rebooting as a result of this API. Migrating such a guest can lead to a plain shutdown on the destination.
Is this still the case?
Hi, Don. Yeah this is still the case.
In any event, how does one know when the reboot has finished (assuming the VM reacted to it)?
Unfortunately there is no event for reboot. Before the patch there was SHUTDOWN event but only when reboot is done thru ACPI. Now when fake reboot impl is more incapsulated there is no more SHUTDOWN event just as for reboot from guest.
Nikolay
There was also a startup/resume event. Maybe add a reboot event at that point? I don't think anyone really cares about when the shutdown occurs but rather know about the resume. A side question is: is there a problem with doing a migration if, say, the reboot was done internally (i.e. someone issued a "reboot" command from within the VM)? -d
On 08.04.2020 16:08, Don Koch wrote:
On Wed, 8 Apr 2020 10:34:08 +0300 nshirokovskiy wrote:
On 07.04.2020 20:31, Don Koch wrote:
On Thu, 19 Dec 2019 13:50:19 +0300 Nikolay Shirokovskiy wrote:
If we use fake reboot then domain goes thru running->shutdown->running state changes with shutdown state only for short period of time. At least this is implementation details leaking into API. And also there is one real case when this is not convinient. I'm doing a backup with the help of temporary block snapshot (with the help of qemu's API which is used in the newly created libvirt's backup API). If guest is shutdowned I want to continue to backup so I don't kill the process and domain is in shutdown state. Later when backup is finished I want to destroy qemu process. So I check if it is in shutdowned state and destroy it if it is. Now if instead of shutdown domain got fake reboot then I can destroy process in the middle of fake reboot process.
After shutdown event we also get stop event and now as domain state is running it will be transitioned to paused state and back to running later. Though this is not critical for the described case I guess it is better not to leak these details to user too. So let's leave domain in running state on stop event if fake reboot is in process.
Reconnection code handles this patch without modification. It detects that qemu is not running due to shutdown and then calls qemuProcessShutdownOrReboot which reboots as fake reboot flag is set.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Question regarding this change: in the on-line documentation for virDomainReboot(), it states:
Due to implementation limitations in some drivers (the qemu driver, for instance) it is not advised to migrate or save a guest that is rebooting as a result of this API. Migrating such a guest can lead to a plain shutdown on the destination.
Is this still the case?
Hi, Don. Yeah this is still the case.
In any event, how does one know when the reboot has finished (assuming the VM reacted to it)?
Unfortunately there is no event for reboot. Before the patch there was SHUTDOWN event but only when reboot is done thru ACPI. Now when fake reboot impl is more incapsulated there is no more SHUTDOWN event just as for reboot from guest.
Nikolay
There was also a startup/resume event. Maybe add a reboot event at that point? I don't think anyone really cares about when the shutdown occurs but rather know about the resume.
The point of the patch was to hide these details from client. Just like in case of internal reboot - you will not see suspend/resume events.
A side question is: is there a problem with doing a migration if, say, the reboot was done internally (i.e. someone issued a "reboot" command from within the VM)?
AFAIU from libvirt POV there should be no issues. I can's say for QEMU though. Nikolay
participants (4)
-
Cole Robinson -
Don Koch -
Nikolay Shirokovskiy -
nshirokovskiy