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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list