[libvirt] [PATCH v5 0/2] notify about reverting to a snapshot
The original idea of this patch-set was to notify API clients about changes in domain configuration on revert to a snapshot. But in the last review Cole noticed that updated configuration doesn't saved to a file and libvirtd reboot drops changes. So in the patch-set v5 configuration is saved and event is emitted for all cases when persistent domain configuration was updated from a snapshot. Dmitry Andreev (2): Introduce new VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT sub-event qemuDomainRevertToSnapshot: save domain configuration examples/object-events/event-test.c | 2 ++ include/libvirt/libvirt-domain.h | 1 + src/qemu/qemu_driver.c | 9 +++++++++ tools/virsh-domain.c | 3 ++- 4 files changed, 14 insertions(+), 1 deletion(-) -- 1.8.3.1
VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT event should be emitted when domain configuration was changed on revert to 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 8ea3df6..d6cdfca 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2346,6 +2346,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 41c749d..d60eafe 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11410,7 +11410,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
Reverting to a snapshot may change domain configuration. New configuration should be saved if domain has persistent flag. VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT is emitted in case of configuration update. --- src/qemu/qemu_driver.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a0d6596..edf39d9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15546,6 +15546,15 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } else if (snap) { snap->def->current = false; } + if (ret == 0 && config && vm->persistent && + !(ret = virDomainSaveConfig(cfg->configDir, driver->caps, + vm->newDef ? vm->newDef : vm->def))) { + detail = VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT; + qemuDomainEventQueue(driver, + virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_DEFINED, + detail)); + } if (event) { qemuDomainEventQueue(driver, event); qemuDomainEventQueue(driver, event2); -- 1.8.3.1
On 03/12/2016 10:39 AM, Dmitry Andreev wrote:
Reverting to a snapshot may change domain configuration. New configuration should be saved if domain has persistent flag.
VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT is emitted in case of configuration update. --- src/qemu/qemu_driver.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a0d6596..edf39d9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15546,6 +15546,15 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } else if (snap) { snap->def->current = false; } + if (ret == 0 && config && vm->persistent && + !(ret = virDomainSaveConfig(cfg->configDir, driver->caps, + vm->newDef ? vm->newDef : vm->def))) { + detail = VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT; + qemuDomainEventQueue(driver, + virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_DEFINED, + detail)); + } if (event) { qemuDomainEventQueue(driver, event); qemuDomainEventQueue(driver, event2);
I was a bit confused by this at first, since you are conditionalizing on 'config' and I thought we only AssignDef'd it for certain code paths... but indeed if config is present we always assign it to some part of the VM. I tested briefly with virt-manager too and this seems to fix things. ACK and pushed. Thanks! - Cole
Макс, можно тебя попросить вмержить этот патч? commit c5e81090eaf40d5735402f94a7446ced3e78a7df commit 8047d45704a3af77a2c4b88f48e3c98d853813b8 А прошлую версию удалить. commit fbd032c5e92b799052c487710b1ce6d8d65269a8 commit d040dafb362ddec8c090d52cbcd2a8f745bfa916 Три месяца с первой версии :О On 15.03.2016 22:20, Cole Robinson wrote:
On 03/12/2016 10:39 AM, Dmitry Andreev wrote:
Reverting to a snapshot may change domain configuration. New configuration should be saved if domain has persistent flag.
VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT is emitted in case of configuration update. --- src/qemu/qemu_driver.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a0d6596..edf39d9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15546,6 +15546,15 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } else if (snap) { snap->def->current = false; } + if (ret == 0 && config && vm->persistent && + !(ret = virDomainSaveConfig(cfg->configDir, driver->caps, + vm->newDef ? vm->newDef : vm->def))) { + detail = VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT; + qemuDomainEventQueue(driver, + virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_DEFINED, + detail)); + } if (event) { qemuDomainEventQueue(driver, event); qemuDomainEventQueue(driver, event2);
I was a bit confused by this at first, since you are conditionalizing on 'config' and I thought we only AssignDef'd it for certain code paths... but indeed if config is present we always assign it to some part of the VM. I tested briefly with virt-manager too and this seems to fix things.
ACK and pushed. Thanks!
- Cole
Sorry, forget to remove cc to libvir-list@redhat.com. On 16.03.2016 11:30, Dmitry Andreev wrote:
Макс, можно тебя попросить вмержить этот патч?
commit c5e81090eaf40d5735402f94a7446ced3e78a7df commit 8047d45704a3af77a2c4b88f48e3c98d853813b8
А прошлую версию удалить.
commit fbd032c5e92b799052c487710b1ce6d8d65269a8 commit d040dafb362ddec8c090d52cbcd2a8f745bfa916
Три месяца с первой версии :О
On 15.03.2016 22:20, Cole Robinson wrote:
On 03/12/2016 10:39 AM, Dmitry Andreev wrote:
Reverting to a snapshot may change domain configuration. New configuration should be saved if domain has persistent flag.
VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT is emitted in case of configuration update. --- src/qemu/qemu_driver.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a0d6596..edf39d9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15546,6 +15546,15 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } else if (snap) { snap->def->current = false; } + if (ret == 0 && config && vm->persistent && + !(ret = virDomainSaveConfig(cfg->configDir, driver->caps, + vm->newDef ? vm->newDef : vm->def))) { + detail = VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT; + qemuDomainEventQueue(driver, + virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_DEFINED, + detail)); + } if (event) { qemuDomainEventQueue(driver, event); qemuDomainEventQueue(driver, event2);
I was a bit confused by this at first, since you are conditionalizing on 'config' and I thought we only AssignDef'd it for certain code paths... but indeed if config is present we always assign it to some part of the VM. I tested briefly with virt-manager too and this seems to fix things.
ACK and pushed. Thanks!
- Cole
participants (2)
-
Cole Robinson -
Dmitry Andreev