Re: [libvirt] [PATCHv2 2/2] qemu: emit 'defined' event after reverted to snapshot

On Wed, Dec 16, 2015 at 12:09:42 +0300, Dmitry Andreev wrote: > If config file was changed VIR_DOMAIN_EVENT_DEFINED should be emitted drop "file" here. The important part is the config itself being changed. The file is just a implication of that. Additionally, commit messages here or in the previous patch don't provide any justification for this change. I'd like to have a statement why is this important to change.

> --- > src/qemu/qemu_driver.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 783a7cd..1474eaa 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -15512,7 +15512,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, > VIR_DOMAIN_EVENT_STARTED, > detail); > if (rc < 0) > - goto endjob; > + goto defined; > } > > /* Touch up domain state. */ > @@ -15534,7 +15534,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, > if (!virDomainObjIsActive(vm)) { > virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > _("guest unexpectedly quit")); > - goto endjob; > + goto defined; > } > rc = qemuProcessStartCPUs(driver, vm, snapshot->domain->conn, > VIR_DOMAIN_RUNNING_FROM_SNAPSHOT, > @@ -15636,6 +15636,15 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, > > ret = 0; > > + defined: > + if (config) { > + virObjectEventPtr define_event; > + define_event = virDomainEventLifecycleNewFromObj(vm, > + VIR_DOMAIN_EVENT_DEFINED, > + VIR_DOMAIN_EVENT_DEFINED_FROM_SNAPSHOT); > + qemuDomainEventQueue(driver, define_event); > + } This will emit the event even if the configuration itself didn't change. We do a similar thing when you switch off the VM. The next-start config replaces the current config. This is implied by the events of starting and stopping of the VM. Since the internal snapshot code doesn't restart qemu in some cases, that's when the configuration can't change (which actually might be implemented wrong in a few places). For snapshot state transitions that change domain state, the appropriate events are already used. As of the reasons above, I'd like you to provide some justification why this is a good thing to do. Peter -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list