9:39 a.m.
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
9:39 a.m.
New subject: [libvirt] [PATCH v5 1/2] Introduce new VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT sub-event
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
9:39 a.m.
New subject: [libvirt] [PATCH v5 2/2] qemuDomainRevertToSnapshot: save domain configuration
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
2:20 p.m.
New subject: [libvirt] [PATCH v5 2/2] qemuDomainRevertToSnapshot: save domain configuration
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
3:30 a.m.
New subject: [libvirt] [PATCH v5 2/2] qemuDomainRevertToSnapshot: save domain configuration
Макс, можно тебя попросить вмержить этот патч?
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
3:34 a.m.
New subject: [libvirt] [PATCH v5 2/2] qemuDomainRevertToSnapshot: save domain configuration
Sorry, forget to remove cc to libvir-list(a)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
>
3165
days inactive
3169
days old
5 comments
2 participants
participants (2)
-
Cole Robinson -
Dmitry Andreev