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

[Peter Krempa]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151216/19434257/attachment-0001.sig>