[libvirt] [PATCH v2] qemu: Report shutdown event details

[Martin Kletzander]

On Tue, May 16, 2017 at 01:51:17PM -0500, Eric Blake wrote:
>On 05/16/2017 12:20 PM, Daniel P. Berrange wrote:
>
>>>> @@ -678,6 +699,7 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>>>>
>>>>   unlock:
>>>>      virObjectUnlock(vm);
>>>> +    qemuDomainEventQueue(driver, pre_event);
>>>>      qemuDomainEventQueue(driver, event);
>>>>      virObjectUnref(cfg);
>>>
>>> Nice - you send the same event as always so old clients don't break, but
>>> new clients can now look for the new cause.
>>
>> I don't think that's right actually. IMHO, we should onyl be sending the
>> new event, not the original event. These are intended to indicate state
>> changes, and having two VIR_DOMAIN_EVENT_SHUTDOWN events sent with
>> different details is not really representing a state change.
>>
>> WRT to compatibility, clients should always expect that 'detail' may
>> change or new 'detail' enum values may be added - indeed we've done
>> that many many times int he past. So I don't think we need to duplicate
>> the existing event
>
>That may be my fault for causing a mis-understanding of
>back-compatibility handling on my review of v1. In the past, when we've
>had an event that returns a too-small struct, the only way to return
>more information is to create a second event with the additional info,
>then fire off both events at the same time (for the clients expecting
>the old event semantics, and for new clients) - which really means two
>separate RPC events.  But here, we already have sufficient lifecycle
>event parameters to return details without having to add a new RPC event
>(proven by the fact that you didn't have to touch src/remote at all).
>So now I'm agreeing with Daniel - the fact that we have new information
>means we don't need to be back-compat to older clients: they will see
>the same lifecycle event they have always seen, and not care that the
>cause has changed, while new clients will see a plain cause where no
>information was available from older qemu, or one of the two new causes
>where qemu gave it to us.
>

Sure, I'll give that a respin ;)

Thanks for the review.

>--
>Eric Blake, Principal Software Engineer
>Red Hat, Inc.           +1-919-301-3266
>Virtualization:  qemu.org | libvirt.org
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170516/7b792b94/attachment-0001.sig>