[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]