[libvirt] [PATCHv3 0/2] notify about reverting to a snapshot

Reverting to snapshot may change domain configuration but there will be no events about that. Lack of the event become a problem for virt-manager https://bugzilla.redhat.com/show_bug.cgi?id=1081148 This patch-set introduces new event and emits it in qemuDomainRevertToSnapshot. v3: [2/2] event is emited only in the case when stopped domain is reverted to a stopped domain. Dmitry Andreev (2): Introduce new VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT sub-event qemu: emit 'defined' event after reverted to snapshot without state changes examples/object-events/event-test.c | 2 ++ include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_driver.c | 8 +++++++- tools/virsh-domain.c | 3 ++- 4 files changed, 12 insertions(+), 2 deletions(-) -- 1.8.3.1

When domain is reverted to a snapshot and domain state is changed libvirt emits <NEW STATE>_FROM_SNAPSHOT event. This new introduced event could be emitted if there was no changes in state but domain configuration was changed by a snapshot. --- examples/object-events/event-test.c | 2 ++ include/libvirt/libvirt-domain.h | 1 + tools/virsh-domain.c | 3 ++- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c index dcae981..afac100 100644 --- a/examples/object-events/event-test.c +++ b/examples/object-events/event-test.c @@ -110,6 +110,8 @@ static const char *eventDetailToString(int event, int detail) { ret = "Updated"; else if (detail == VIR_DOMAIN_EVENT_DEFINED_RENAMED) ret = "Renamed"; + else if (detail == VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT) + ret = "Snapshot"; break; case VIR_DOMAIN_EVENT_UNDEFINED: if (detail == VIR_DOMAIN_EVENT_UNDEFINED_REMOVED) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a1ea6a5..2023df0 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2330,6 +2330,7 @@ typedef enum { VIR_DOMAIN_EVENT_DEFINED_ADDED = 0, /* Newly created config file */ VIR_DOMAIN_EVENT_DEFINED_UPDATED = 1, /* Changed config file */ VIR_DOMAIN_EVENT_DEFINED_RENAMED = 2, /* Domain was renamed */ + VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT = 3, /* Config was restored from a snapshot */ # ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_EVENT_DEFINED_LAST diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index edbbc34..9afc525 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11922,7 +11922,8 @@ VIR_ENUM_IMPL(virshDomainEventDefined, VIR_DOMAIN_EVENT_DEFINED_LAST, N_("Added"), N_("Updated"), - N_("Renamed")) + N_("Renamed"), + N_("Snapshot")) VIR_ENUM_DECL(virshDomainEventUndefined) VIR_ENUM_IMPL(virshDomainEventUndefined, -- 1.8.3.1

When domain is reverted to a snapshot it's configuration and state may be changed. If the domain state was changed libvirt emits one or more <NEW STATE>_FROM_SNAPSHOT events. In case when domain and target states both are offline there will be no state changes and no events. Lack of the event become a problem for virt-manager https://bugzilla.redhat.com/show_bug.cgi?id=1081148 This commit adds DEFINED event emission in this signle case and only if snapshot have the domain configuration. --- src/qemu/qemu_driver.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e8ba3a6..c336600 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15306,7 +15306,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, VIR_DOMAIN_SNAPSHOT_REVERT_FORCE, -1); /* We have the following transitions, which create the following events: - * 1. inactive -> inactive: none + * 1. inactive -> inactive: EVENT_DEFINED if snapshot has config * 2. inactive -> running: EVENT_STARTED * 3. inactive -> paused: EVENT_STARTED, EVENT_PAUSED * 4. running -> inactive: EVENT_STOPPED @@ -15592,6 +15592,12 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, VIR_DOMAIN_EVENT_SUSPENDED, detail); } + } else if (config && !event) { + /* Transition 1 */ + detail = VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT; + event = virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_DEFINED, + detail); } break; -- 1.8.3.1

On 12/23/2015 09:25 AM, Dmitry Andreev wrote:
Reverting to snapshot may change domain configuration but there will be no events about that.
Lack of the event become a problem for virt-manager https://bugzilla.redhat.com/show_bug.cgi?id=1081148
This patch-set introduces new event and emits it in qemuDomainRevertToSnapshot.
v3: [2/2] event is emited only in the case when stopped domain is reverted to a stopped domain.
Dmitry Andreev (2): Introduce new VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT sub-event qemu: emit 'defined' event after reverted to snapshot without state changes
examples/object-events/event-test.c | 2 ++ include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_driver.c | 8 +++++++- tools/virsh-domain.c | 3 ++- 4 files changed, 12 insertions(+), 2 deletions(-)
This seems to be a reasonable approach; however, since Peter understands the snapshot code better than I do, I expect he will want to chime in (I've CC'd him). It does seem though it's important to note somehow in the documentation of the new event that it is only useful for the offline -> offline revert. Think patch 1's comment and commit message. w/r/t patch 2 - the new else statement ... * Don't see any way that event could be defined... * Doesn't seem to matter if the FORCE flag was used or not so it seems just an "else if (config)" could be done. John

On Tue, Jan 05, 2016 at 17:19:19 -0500, John Ferlan wrote:
On 12/23/2015 09:25 AM, Dmitry Andreev wrote:
Reverting to snapshot may change domain configuration but there will be no events about that.
I'm afraid there will be more places like this ...
Lack of the event become a problem for virt-manager https://bugzilla.redhat.com/show_bug.cgi?id=1081148
This patch-set introduces new event and emits it in qemuDomainRevertToSnapshot.
[...]
This seems to be a reasonable approach; however, since Peter understands the snapshot code better than I do, I expect he will want to chime in (I've CC'd him).
I don't currently see a problem with emiting the event. We indeed replace the complete definition with something else so a 'DEFINED' event is probably appropriate. Peter

On 06.01.2016 01:19, John Ferlan wrote:
On 12/23/2015 09:25 AM, Dmitry Andreev wrote:
Reverting to snapshot may change domain configuration but there will be no events about that.
Lack of the event become a problem for virt-manager https://bugzilla.redhat.com/show_bug.cgi?id=1081148
This patch-set introduces new event and emits it in qemuDomainRevertToSnapshot.
v3: [2/2] event is emited only in the case when stopped domain is reverted to a stopped domain.
Dmitry Andreev (2): Introduce new VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT sub-event qemu: emit 'defined' event after reverted to snapshot without state changes
examples/object-events/event-test.c | 2 ++ include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_driver.c | 8 +++++++- tools/virsh-domain.c | 3 ++- 4 files changed, 12 insertions(+), 2 deletions(-)
This seems to be a reasonable approach; however, since Peter understands the snapshot code better than I do, I expect he will want to chime in (I've CC'd him).
It does seem though it's important to note somehow in the documentation of the new event that it is only useful for the offline -> offline revert. Think patch 1's comment and commit message.
w/r/t patch 2 - the new else statement ...
* Don't see any way that event could be defined...
There will be VIR_DOMAIN_EVENT_STOPPED event for transitions 4,7 (from running/paused to stopped).
* Doesn't seem to matter if the FORCE flag was used or not
so it seems just an "else if (config)" could be done.
John
participants (3)
-
Dmitry Andreev
-
John Ferlan
-
Peter Krempa