[libvirt] [PATCH v4 0/2] notify about reverting to a snapshot
Reverting to a snapshot may change domain configuration but there will be no events about that in some cases. 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. v4: rebase 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 65f1618..fc1e2f4 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 6dd75e2..440d84e 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11386,7 +11386,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 a762521..1fb41e3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15179,7 +15179,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 @@ -15465,6 +15465,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 02/18/2016 05:06 AM, Dmitry Andreev wrote:
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 a762521..1fb41e3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15179,7 +15179,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 @@ -15465,6 +15465,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;
I just tested these patches, and they do fix the reported issue. However I noticed another pre-existing issue in this area: if guest XML is updated by a snapshot, it isn't actually written to disk. So if you do: - shutdown VM - take snapshot - alter XML - revert to snapshot - restart libvirt the VM will have the altered XML, and _not_ the XML from the snapshot. This is definitely wrong. Any time we alter the VM XML via a snapshot we need to be saving it to disk as well, so that the persistent XML always matches the VM disk image contents. Basically any time in RevertToSnapshot that we call virDomainObjAssignDef, we should be queuing up a DEFINED event. This means some code paths will need to handle 2 events, since we also need to dispatch the state change event. And right before finishing the function we need to save the config to disk. These patches shouldn't be comitted now because they are an API addition, and we are in freeze, but I'd also rather see the complete solution. If you send patches I'll give timely review, otherwise I'll probably implement it myself in a couple weeks. Thanks, Cole
participants (2)
-
Cole Robinson -
Dmitry Andreev