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 eventThat 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
