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

[John Ferlan]

On 12/16/2015 07:01 AM, Peter Krempa wrote:
> 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.
> 

FWIW: This is what I got from the cover letter:

Lack of the event become a problem for virt-manager
https://bugzilla.redhat.com/show_bug.cgi?id=1081148


Without the event, it seems virt-manager doesn't "see" the change and
presents the data prior to snapshot

John

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