On Fri, Nov 14, 2008 at 05:44:29PM +0000, Daniel P. Berrange wrote:
As per our earlier discussion today, this patch expands the callback for domain events so that it also gets a event type specific 'detail' field. This is also kept as an int, and we define enumerations for the possible values associated with each type. [...]
Okay, that made the overall callbacks set clearer, ACK
If a event type has no detail, 0 is passed.
Actually I would define a detail enum for all event type just to make clear what the value will be and expose how it's intended to be extended if needed.
I don't make use of 'CRASHED' in QEMU driver yet. It might be useful in Xen though - when a PV guest crashes, Xen stops the domain running, but leaves it there in a shutoff state, but marked as crashed.
Now using the C event-test program you can see the effects:
myDomainEventCallback1 EVENT: Domain F9x86_64(2) Started Booted myDomainEventCallback2 EVENT: Domain F9x86_64(2) Started Booted myDomainEventCallback1 EVENT: Domain F9x86_64(-1) Stopped Destroyed myDomainEventCallback2 EVENT: Domain F9x86_64(-1) Stopped Destroyed myDomainEventCallback1 EVENT: Domain F9x86_64(3) Started Booted myDomainEventCallback2 EVENT: Domain F9x86_64(3) Started Booted myDomainEventCallback1 EVENT: Domain F9x86_64(3) Suspended myDomainEventCallback2 EVENT: Domain F9x86_64(3) Suspended myDomainEventCallback1 EVENT: Domain F9x86_64(3) Resumed myDomainEventCallback2 EVENT: Domain F9x86_64(3) Resumed myDomainEventCallback1 EVENT: Domain F9x86_64(-1) Stopped Shutdown myDomainEventCallback2 EVENT: Domain F9x86_64(-1) Stopped Shutdown
Of the following sequence of actions
virsh start F9x86_64 virsh destroy F9x86_64 virsh start F9x86_64 virsh suspend F9x86_64 virsh resume F9x86_64 virsh shutdown F9x86_64
For the last 'shutdown' operation, you'll see the same if you just run a graceful shutdown inside the guest itself.
okay
NB, I've not tested saved/restored because my install of KVM is not new enough to support that correctly, but I expect it to work without trouble. Likewise for migration.
A word about migration...
- The destination host first gets a STARTED event, with detail MIGRATED when it starts running - The source host then gets a STOPPED event with detail MIGRATED when it completes
What about 'live' migration, i.e. events sent when the migration flows begin but the domain is stil running. I don't know if KVM has this but on Xen I would expect to be able to notice this. On the target host that could be indicated by SUSPENDED + VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED because it will consume the memory resource like a suspended domain but no actual CPU cycle (well except for migration itself). On the source host this is a bit harder to indicate, maybe this isn't needed as the resource usage doesn't really change at that point.
- The destination host then gets a RESUMED event, on success, and a STOPPED event with detail FAILED if migration aborts.
okay
+static const char *eventDetailToString(int event, int detail) { + const char *ret = ""; + switch(event) { + case VIR_DOMAIN_EVENT_DEFINED: + if (detail == VIR_DOMAIN_EVENT_DEFINED_ADDED) + ret = "Added"; + else if (detail == VIR_DOMAIN_EVENT_DEFINED_UPDATED) + ret = "Updated"; + break; + case VIR_DOMAIN_EVENT_STARTED: + switch (detail) { + case VIR_DOMAIN_EVENT_STARTED_BOOTED: + ret = "Booted"; + break; + case VIR_DOMAIN_EVENT_STARTED_MIGRATED: + ret = "Migrated"; + break; + case VIR_DOMAIN_EVENT_STARTED_RESTORED: + ret = "Restored"; + break; + } + break; + case VIR_DOMAIN_EVENT_STOPPED: + switch (detail) { + case VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN: + ret = "Shutdown"; + break; + case VIR_DOMAIN_EVENT_STOPPED_DESTROYED: + ret = "Destroyed"; + break; + case VIR_DOMAIN_EVENT_STOPPED_CRASHED: + ret = "Crashed"; + break; + case VIR_DOMAIN_EVENT_STOPPED_MIGRATED: + ret = "Migrated"; + break; + case VIR_DOMAIN_EVENT_STOPPED_SAVED: + ret = "Failed"; + break; + case VIR_DOMAIN_EVENT_STOPPED_FAILED: + ret = "Failed"; + break; + } + break; } return ret;
One more reason to add enums for all cases would be to catch here with a warning missing addition to the enums. [...] Patch looks fine to me, I would just add enums for all type but I think this is still okay as-is too as this doesn't change the API/ABI in an incompatible way. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/