src/qemu/qemu_domain.c | 34 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 7 +++++++ src/qemu/qemu_driver.c | 26 +++----------------------- src/qemu/qemu_migration.c | 42 ++++++------------------------------------ src/qemu/qemu_migration.h | 4 ---- src/qemu/qemu_process.c | 38 +++++++++++++++++++++++++------------- 6 files changed, 75 insertions(+), 76 deletions(-)
Patches 1 and 2 are already Reviewed-by: John. Patch 3 needs Peter comments. Diff from v1: ============ - minor rebase changes - minor changes according to review [1] PATCH v1 : https://www.redhat.com/archives/libvir-list/2018-October/msg00591.html Nikolay Shirokovskiy (3): qemu: Pass stop reason from qemuProcessStopCPUs to stop handler qemu: Map suspended state reason to suspended event detail qemu: Don't duplicate suspend events and state changes src/qemu/qemu_domain.c | 34 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 7 +++++++ src/qemu/qemu_driver.c | 26 +++----------------------- src/qemu/qemu_migration.c | 42 ++++++------------------------------------ src/qemu/qemu_migration.h | 4 ---- src/qemu/qemu_process.c | 38 +++++++++++++++++++++++++------------- 6 files changed, 75 insertions(+), 76 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 2/8/19 2:52 AM, Nikolay Shirokovskiy wrote: > Patches 1 and 2 are already Reviewed-by: John. Patch 3 needs Peter comments. > Right - feel free to add my : Reviewed-by: John Ferlan <jferlan@redhat.com> to the first 2 patches for sure. To help push this along, Peter is again CC'd and of importance is the v1 review of patch3: https://www.redhat.com/archives/libvir-list/2018-October/msg00831.html where I noted a specific commit and case to be considered. John > Diff from v1: > ============ > - minor rebase changes > - minor changes according to review > > [1] PATCH v1 : https://www.redhat.com/archives/libvir-list/2018-October/msg00591.html > > Nikolay Shirokovskiy (3): > qemu: Pass stop reason from qemuProcessStopCPUs to stop handler > qemu: Map suspended state reason to suspended event detail > qemu: Don't duplicate suspend events and state changes > > src/qemu/qemu_domain.c | 34 ++++++++++++++++++++++++++++++++++ > src/qemu/qemu_domain.h | 7 +++++++ > src/qemu/qemu_driver.c | 26 +++----------------------- > src/qemu/qemu_migration.c | 42 ++++++------------------------------------ > src/qemu/qemu_migration.h | 4 ---- > src/qemu/qemu_process.c | 38 +++++++++++++++++++++++++------------- > 6 files changed, 75 insertions(+), 76 deletions(-) > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
ping On 13.02.2019 19:55, John Ferlan wrote: > > > On 2/8/19 2:52 AM, Nikolay Shirokovskiy wrote: >> Patches 1 and 2 are already Reviewed-by: John. Patch 3 needs Peter comments. >> > > Right - feel free to add my : > > Reviewed-by: John Ferlan <jferlan@redhat.com> > > to the first 2 patches for sure. > > To help push this along, Peter is again CC'd and of importance is the v1 > review of patch3: > > https://www.redhat.com/archives/libvir-list/2018-October/msg00831.html > > where I noted a specific commit and case to be considered. > > > John > >> Diff from v1: >> ============ >> - minor rebase changes >> - minor changes according to review >> >> [1] PATCH v1 : https://www.redhat.com/archives/libvir-list/2018-October/msg00591.html >> >> Nikolay Shirokovskiy (3): >> qemu: Pass stop reason from qemuProcessStopCPUs to stop handler >> qemu: Map suspended state reason to suspended event detail >> qemu: Don't duplicate suspend events and state changes >> >> src/qemu/qemu_domain.c | 34 ++++++++++++++++++++++++++++++++++ >> src/qemu/qemu_domain.h | 7 +++++++ >> src/qemu/qemu_driver.c | 26 +++----------------------- >> src/qemu/qemu_migration.c | 42 ++++++------------------------------------ >> src/qemu/qemu_migration.h | 4 ---- >> src/qemu/qemu_process.c | 38 +++++++++++++++++++++++++------------- >> 6 files changed, 75 insertions(+), 76 deletions(-) >> -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 3/27/19 7:53 AM, Nikolay Shirokovskiy wrote:
> ping
>
> On 13.02.2019 19:55, John Ferlan wrote:
>>
>>
>> On 2/8/19 2:52 AM, Nikolay Shirokovskiy wrote:
>>> Patches 1 and 2 are already Reviewed-by: John. Patch 3 needs Peter comments.
>>>
>>
>> Right - feel free to add my :
>>
>> Reviewed-by: John Ferlan <jferlan@redhat.com>
>>
>> to the first 2 patches for sure.
>>
>> To help push this along, Peter is again CC'd and of importance is the v1
>> review of patch3:
>>
>> https://www.redhat.com/archives/libvir-list/2018-October/msg00831.html
>>
>> where I noted a specific commit and case to be considered.
>>
To dredge up more context, John is talking about Peter's commit:
commit f569b87f5193a69c50ca714734013cc4f82250f1
Author: Peter Krempa <pkrempa@redhat.com>
Date: Mon Oct 8 19:38:44 2012 +0200
snapshot: qemu: Add support for external checkpoints
Which added this chunk:
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 9358ad99ed..128b98ee22 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1673,6 +1673,9 @@ static int qemudDomainSuspend(virDomainPtr dom) {
> if (priv->job.asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) {
> reason = VIR_DOMAIN_PAUSED_MIGRATION;
> eventDetail = VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED;
> + } else if (priv->job.asyncJob == QEMU_ASYNC_JOB_SNAPSHOT) {
> + reason = VIR_DOMAIN_PAUSED_SNAPSHOT;
> + eventDetail = -1; /* don't create lifecycle events when doing snapshot */
> } else {
> reason = VIR_DOMAIN_PAUSED_USER;
> eventDetail = VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
> @@ -1687,9 +1690,12 @@ static int qemudDomainSuspend(virDomainPtr dom) {
> if (qemuProcessStopCPUs(driver, vm, reason, QEMU_ASYNC_JOB_NONE) < 0) {
> goto endjob;
> }
> - event = virDomainEventNewFromObj(vm,
> - VIR_DOMAIN_EVENT_SUSPENDED,
> - eventDetail);
> +
> + if (eventDetail >= 0) {
> + event = virDomainEventNewFromObj(vm,
> + VIR_DOMAIN_EVENT_SUSPENDED,
> + eventDetail);
> + }
> }
> if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0)
> goto endjob;
Which disables sending a VIR_DOMAIN_EVENT_SUSPENDED event when we
suspend the VM as an implementation detail of creating an external
snapshot checkpoint. Nikolay's series removes that case, so AFAICT
checkpoints will now send SUSPENDED events.
Current git doesn't perform the same event skippage in the managed
save case (where SUSPENDED event is emitted), or the internal snapshot
case (where SUSPEND+RESUME are emitted when taking a live snapshot).
I can't think of any particular reason why the checkpoint case needs to
be special here. Maybe that's an argument for fixing all of these cases,
though virt-manager which uses internal snapshots has had no problem
with SUSPEND/RESUME events in this area so there's some proof apps
aren't going to choke on a similar case
IMO patches have been reviewed for almost 2 months, no one has objected,
so push em :)
Thanks,
Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.