There are two classes of management apps that track events - one that only cares about on/off (and only needs to track EVENT_STARTED and EVENT_STOPPED), and one that cares about paused/running (also tracks EVENT_SUSPENDED/EVENT_RESUMED). To keep both classes happy, any transition that can go from inactive to paused must emit two back-to-back events - one for started and one for suspended (since later resuming of the domain will only send RESUMED, but the first class isn't tracking that). This also fixes a bug where virDomainCreateWithFlags with the VIR_DOMAIN_START_PAUSED flag failed to start paused when restoring from a managed save image. * include/libvirt/libvirt.h.in (VIR_DOMAIN_EVENT_SUSPENDED_RESTORED) (VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT) (VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT): New sub-events. * src/qemu/qemu_driver.c (qemuDomainRevertToSnapshot): Use them. (qemuDomainSaveImageStartVM): Likewise, and add parameter. (qemudDomainCreate, qemuDomainObjStart): Send suspended event when starting paused. (qemuDomainObjRestore): Add parameter. (qemuDomainObjStart, qemuDomainRestoreFlags): Update callers. * examples/domain-events/events-c/event-test.c (eventDetailToString): Map new detail strings. --- I think this addresses Dan's findings about events related to state transitions. It turned a 13-line v3 into a 109-line patch. I'll repost my entire snapshot series as v4 once I finish rebasing on top of this (since I know it introduces some merge conflicts with later patches in the series). examples/domain-events/events-c/event-test.c | 37 +++++++++++++-- include/libvirt/libvirt.h.in | 6 ++- src/qemu/qemu_driver.c | 66 +++++++++++++++++++++----- 3 files changed, 90 insertions(+), 19 deletions(-) diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c index 4766a0d..6a3ed26 100644 --- a/examples/domain-events/events-c/event-test.c +++ b/examples/domain-events/events-c/event-test.c @@ -87,19 +87,45 @@ static const char *eventDetailToString(int event, int detail) { case VIR_DOMAIN_EVENT_STARTED_RESTORED: ret = "Restored"; break; + case VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT: + ret = "Snapshot"; + break; } break; case VIR_DOMAIN_EVENT_SUSPENDED: - if (detail == VIR_DOMAIN_EVENT_SUSPENDED_PAUSED) + switch (detail) { + case VIR_DOMAIN_EVENT_SUSPENDED_PAUSED: ret = "Paused"; - else if (detail == VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED) + break; + case VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED: ret = "Migrated"; + break; + case VIR_DOMAIN_EVENT_SUSPENDED_IOERROR: + ret = "I/O Error"; + break; + case VIR_DOMAIN_EVENT_SUSPENDED_WATCHDOG: + ret = "Watchdog"; + break; + case VIR_DOMAIN_EVENT_SUSPENDED_RESTORED: + ret = "Restored"; + break; + case VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT: + ret = "Snapshot"; + break; + } break; case VIR_DOMAIN_EVENT_RESUMED: - if (detail == VIR_DOMAIN_EVENT_RESUMED_UNPAUSED) + switch (detail) { + case VIR_DOMAIN_EVENT_RESUMED_UNPAUSED: ret = "Unpaused"; - else if (detail == VIR_DOMAIN_EVENT_RESUMED_MIGRATED) + break; + case VIR_DOMAIN_EVENT_RESUMED_MIGRATED: ret = "Migrated"; + break; + case VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT: + ret = "Snapshot"; + break; + } break; case VIR_DOMAIN_EVENT_STOPPED: switch (detail) { @@ -121,6 +147,9 @@ static const char *eventDetailToString(int event, int detail) { case VIR_DOMAIN_EVENT_STOPPED_FAILED: ret = "Failed"; break; + case VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT: + ret = "Snapshot"; + break; } break; } diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 53a2f7d..edd197c 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -116,8 +116,7 @@ typedef enum { VIR_DOMAIN_PAUSED_DUMP = 4, /* paused for offline core dump */ VIR_DOMAIN_PAUSED_IOERROR = 5, /* paused due to a disk I/O error */ VIR_DOMAIN_PAUSED_WATCHDOG = 6, /* paused due to a watchdog event */ - VIR_DOMAIN_PAUSED_FROM_SNAPSHOT = 7, /* restored from a snapshot which was - * taken while domain was paused */ + VIR_DOMAIN_PAUSED_FROM_SNAPSHOT = 7, /* paused after restoring from snapshot */ } virDomainPausedReason; typedef enum { @@ -2024,6 +2023,8 @@ typedef enum { VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED = 1, /* Suspended for offline migration */ VIR_DOMAIN_EVENT_SUSPENDED_IOERROR = 2, /* Suspended due to a disk I/O error */ VIR_DOMAIN_EVENT_SUSPENDED_WATCHDOG = 3, /* Suspended due to a watchdog firing */ + VIR_DOMAIN_EVENT_SUSPENDED_RESTORED = 4, /* Restored from paused state file */ + VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT = 5, /* Restored from paused snapshot */ } virDomainEventSuspendedDetailType; /** @@ -2034,6 +2035,7 @@ typedef enum { typedef enum { VIR_DOMAIN_EVENT_RESUMED_UNPAUSED = 0, /* Normal resume due to admin unpause */ VIR_DOMAIN_EVENT_RESUMED_MIGRATED = 1, /* Resumed for completion of migration */ + VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT = 2, /* Resumed from snapshot */ } virDomainEventResumedDetailType; /** diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 737eec8..6becd31 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1268,6 +1268,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; virDomainEventPtr event = NULL; + virDomainEventPtr event2 = NULL; virCheckFlags(VIR_DOMAIN_START_PAUSED | VIR_DOMAIN_START_AUTODESTROY, NULL); @@ -1315,6 +1316,16 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_BOOTED); + if (event && (flags & VIR_DOMAIN_START_PAUSED)) { + /* There are two classes of event-watching clients - those + * that only care about on/off (and must see a started event + * no matter what, but don't care about suspend events), and + * those that also care about running/paused. To satisfy both + * client types, we have to send two events. */ + event2 = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_SUSPENDED, + VIR_DOMAIN_EVENT_SUSPENDED_PAUSED); + } virDomainAuditStart(vm, "booted", true); dom = virGetDomain(conn, vm->def->name, vm->def->uuid); @@ -1328,8 +1339,11 @@ cleanup: virDomainDefFree(def); if (vm) virDomainObjUnlock(vm); - if (event) + if (event) { qemuDomainEventQueue(driver, event); + if (event2) + qemuDomainEventQueue(driver, event2); + } qemuDriverUnlock(driver); return dom; } @@ -3890,7 +3904,8 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, virDomainObjPtr vm, int *fd, const struct qemud_save_header *header, - const char *path) + const char *path, + bool start_paused) { int ret = -1; virDomainEventPtr event; @@ -3961,8 +3976,8 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, qemuDomainEventQueue(driver, event); - /* If it was running before, resume it now. */ - if (header->was_running) { + /* If it was running before, resume it now unless caller requested pause. */ + if (header->was_running && !start_paused) { if (qemuProcessStartCPUs(driver, vm, conn, VIR_DOMAIN_RUNNING_RESTORED, QEMU_ASYNC_JOB_NONE) < 0) { @@ -3975,6 +3990,14 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, VIR_WARN("Failed to save status on vm %s", vm->def->name); goto out; } + } else { + int detail = (start_paused ? VIR_DOMAIN_EVENT_SUSPENDED_PAUSED : + VIR_DOMAIN_EVENT_SUSPENDED_RESTORED); + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_SUSPENDED, + detail); + if (event) + qemuDomainEventQueue(driver, event); } ret = 0; @@ -4026,7 +4049,8 @@ qemuDomainRestoreFlags(virConnectPtr conn, if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; - ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, &header, path); + ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, &header, path, + false); if (virFileDirectFdClose(directFd) < 0) VIR_WARN("Failed to close %s", path); @@ -4151,6 +4175,7 @@ qemuDomainObjRestore(virConnectPtr conn, struct qemud_driver *driver, virDomainObjPtr vm, const char *path, + bool start_paused, bool bypass_cache) { virDomainDefPtr def = NULL; @@ -4181,7 +4206,8 @@ qemuDomainObjRestore(virConnectPtr conn, virDomainObjAssignDef(vm, def, true); def = NULL; - ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, &header, path); + ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, &header, path, + start_paused); if (virFileDirectFdClose(directFd) < 0) VIR_WARN("Failed to close %s", path); @@ -4459,7 +4485,7 @@ qemuDomainObjStart(virConnectPtr conn, if (virFileExists(managed_save)) { ret = qemuDomainObjRestore(conn, driver, vm, managed_save, - bypass_cache); + start_paused, bypass_cache); if ((ret == 0) && (unlink(managed_save) < 0)) VIR_WARN("Failed to remove the managed state %s", managed_save); @@ -4475,8 +4501,16 @@ qemuDomainObjStart(virConnectPtr conn, virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_BOOTED); - if (event) + if (event) { qemuDomainEventQueue(driver, event); + if (start_paused) { + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_SUSPENDED, + VIR_DOMAIN_EVENT_SUSPENDED_PAUSED); + if (event) + qemuDomainEventQueue(driver, event); + } + } } cleanup: @@ -8744,6 +8778,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virDomainSnapshotObjPtr snap = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainEventPtr event = NULL; + virDomainEventPtr event2 = NULL; qemuDomainObjPrivatePtr priv; int rc; @@ -8800,6 +8835,9 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, goto endjob; } + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STARTED, + VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT); if (snap->def->state == VIR_DOMAIN_PAUSED) { /* qemu unconditionally starts the domain running again after * loadvm, so let's pause it to keep consistency @@ -8810,14 +8848,13 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, QEMU_ASYNC_JOB_NONE); if (rc < 0) goto endjob; + event2 = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_SUSPENDED, + VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT); } else { virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_FROM_SNAPSHOT); } - - event = virDomainEventNewFromObj(vm, - VIR_DOMAIN_EVENT_STARTED, - VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT); } else { /* qemu is a little funny with running guests and the restoration * of snapshots. If the snapshot was taken online, @@ -8860,8 +8897,11 @@ cleanup: } else if (snap) { snap->def->current = false; } - if (event) + if (event) { qemuDomainEventQueue(driver, event); + if (event2) + qemuDomainEventQueue(driver, event2); + } if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); -- 1.7.4.4 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list