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
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
