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

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to