https://bugzilla.redhat.com/show_bug.cgi?id=871452 So, you want to create a domain from XML. The domain already exists in libvirt's database of domains. It's okay, because name and UUID matches. However, on domain startup, internal representation of the domain is overwritten with your XML even though we claim that the XML you've provided is a transient one. The bug is to be found across nearly all the drivers. Le sigh. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/bhyve/bhyve_driver.c | 1 + src/libxl/libxl_driver.c | 1 + src/lxc/lxc_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/qemu/qemu_driver.c | 1 + src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vmware/vmware_driver.c | 1 + 8 files changed, 8 insertions(+) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 7f365b1..d44cf2c 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -918,6 +918,7 @@ bhyveDomainCreateXML(virConnectPtr conn, if (!(vm = virDomainObjListAdd(privconn->domains, def, privconn->xmlopt, + VIR_DOMAIN_OBJ_LIST_ADD_LIVE | VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto cleanup; def = NULL; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 5048957..fc6dcec 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -974,6 +974,7 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml, if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, + VIR_DOMAIN_OBJ_LIST_ADD_LIVE | VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto cleanup; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index e5e6c5a..b408be0 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1229,6 +1229,7 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn, if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, + VIR_DOMAIN_OBJ_LIST_ADD_LIVE | VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto cleanup; diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index fc8db7e..d78e2f5 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1093,6 +1093,7 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, if (!(vm = virDomainObjListAdd(driver->domains, vmdef, driver->xmlopt, + VIR_DOMAIN_OBJ_LIST_ADD_LIVE | VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto cleanup; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 30d2d98..2a4b026 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1745,6 +1745,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, + VIR_DOMAIN_OBJ_LIST_ADD_LIVE | VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto cleanup; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b40b079..01ab1e3 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1616,6 +1616,7 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, if (!(dom = virDomainObjListAdd(privconn->domains, def, privconn->xmlopt, + VIR_DOMAIN_OBJ_LIST_ADD_LIVE | VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto cleanup; diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index d4b03b3..14598fc 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1615,6 +1615,7 @@ static virDomainPtr umlDomainCreateXML(virConnectPtr conn, const char *xml, if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, + VIR_DOMAIN_OBJ_LIST_ADD_LIVE | VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto cleanup; diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 152af39..a12b03a 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -704,6 +704,7 @@ vmwareDomainCreateXML(virConnectPtr conn, const char *xml, if (!(vm = virDomainObjListAdd(driver->domains, vmdef, driver->xmlopt, + VIR_DOMAIN_OBJ_LIST_ADD_LIVE | VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto cleanup; -- 2.4.9

So far we have the following pattern occurring over and over again: if (!vm->persistent) qemuDomainRemoveInactive(driver, vm); It's safe to put the check into the function and save some LoC. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 9 ++++++++- src/qemu/qemu_driver.c | 42 ++++++++++++++++-------------------------- src/qemu/qemu_migration.c | 14 +++++--------- src/qemu/qemu_process.c | 12 ++++-------- 4 files changed, 33 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7d92f3a..367d662 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2632,7 +2632,14 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver, { bool haveJob = true; char *snapDir; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virQEMUDriverConfigPtr cfg; + + if (vm->persistent) { + /* Short-circuit, we don't want to remove a persistent domain */ + return; + } + + cfg = virQEMUDriverGetConfig(driver); if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) haveJob = false; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2a4b026..3532973 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1753,8 +1753,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, def = NULL; if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) { - if (!vm->persistent) - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm); goto cleanup; } @@ -1764,8 +1763,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, start_flags) < 0) { virDomainAuditStart(vm, "booted", false); qemuDomainObjEndJob(driver, vm); - if (!vm->persistent) - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm); goto cleanup; } @@ -2250,7 +2248,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, ret = 0; endjob: qemuDomainObjEndJob(driver, vm); - if (ret == 0 && !vm->persistent) + if (ret == 0) qemuDomainRemoveInactive(driver, vm); cleanup: @@ -3273,7 +3271,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, } } qemuDomainObjEndAsyncJob(driver, vm); - if (ret == 0 && !vm->persistent) + if (ret == 0) qemuDomainRemoveInactive(driver, vm); cleanup: @@ -3774,7 +3772,7 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom, } qemuDomainObjEndAsyncJob(driver, vm); - if (ret == 0 && flags & VIR_DUMP_CRASH && !vm->persistent) + if (ret == 0 && flags & VIR_DUMP_CRASH) qemuDomainRemoveInactive(driver, vm); cleanup: @@ -4054,8 +4052,7 @@ processGuestPanicEvent(virQEMUDriverPtr driver, virDomainAuditStop(vm, "destroyed"); - if (!vm->persistent) - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm); break; case VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_RESTART: @@ -6831,7 +6828,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, VIR_WARN("Failed to close %s", path); qemuDomainObjEndJob(driver, vm); - if (ret < 0 && !vm->persistent) + if (ret < 0) qemuDomainRemoveInactive(driver, vm); cleanup: @@ -7526,6 +7523,7 @@ static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml } else { /* Brand new domain. Remove it */ VIR_INFO("Deleting domain '%s'", vm->def->name); + vm->persistent = 0; qemuDomainRemoveInactive(driver, vm); } goto cleanup; @@ -7651,11 +7649,9 @@ qemuDomainUndefineFlags(virDomainPtr dom, * domainDestroy and domainShutdown will take care of removing the * domain obj from the hash table. */ - if (virDomainObjIsActive(vm)) { - vm->persistent = 0; - } else { + vm->persistent = 0; + if (!virDomainObjIsActive(vm)) qemuDomainRemoveInactive(driver, vm); - } ret = 0; @@ -15550,12 +15546,9 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } if (qemuDomainSnapshotRevertInactive(driver, vm, snap) < 0) { - if (!vm->persistent) { - qemuDomainObjEndJob(driver, vm); - qemuDomainRemoveInactive(driver, vm); - goto cleanup; - } - goto endjob; + qemuDomainObjEndJob(driver, vm); + qemuDomainRemoveInactive(driver, vm); + goto cleanup; } if (config) virDomainObjAssignDef(vm, config, false, NULL); @@ -15575,12 +15568,9 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, start_flags); virDomainAuditStart(vm, "from-snapshot", rc >= 0); if (rc < 0) { - if (!vm->persistent) { - qemuDomainObjEndJob(driver, vm); - qemuDomainRemoveInactive(driver, vm); - goto cleanup; - } - goto endjob; + qemuDomainObjEndJob(driver, vm); + qemuDomainRemoveInactive(driver, vm); + goto cleanup; } detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT; event = virDomainEventLifecycleNewFromObj(vm, diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3dd3718..7440108 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3483,8 +3483,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, VIR_FREE(priv->origname); virPortAllocatorRelease(driver->migrationPorts, priv->nbdPort); priv->nbdPort = 0; - if (!vm->persistent) - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm); } virDomainObjEndAPI(&vm); qemuDomainEventQueue(driver, event); @@ -3873,8 +3872,7 @@ qemuMigrationConfirm(virConnectPtr conn, flags, cancelled); qemuMigrationJobFinish(driver, vm); - if (!virDomainObjIsActive(vm) && - (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE))) { + if (!virDomainObjIsActive(vm)) { if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); qemuDomainRemoveInactive(driver, vm); @@ -5357,9 +5355,7 @@ qemuMigrationPerformJob(virQEMUDriverPtr driver, } qemuMigrationJobFinish(driver, vm); - if (!virDomainObjIsActive(vm) && - (!vm->persistent || - (ret == 0 && (flags & VIR_MIGRATE_UNDEFINE_SOURCE)))) { + if (!virDomainObjIsActive(vm) && ret == 0) { if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); qemuDomainRemoveInactive(driver, vm); @@ -5435,7 +5431,7 @@ qemuMigrationPerformPhase(virQEMUDriverPtr driver, qemuMigrationJobFinish(driver, vm); else qemuMigrationJobContinue(vm); - if (!virDomainObjIsActive(vm) && !vm->persistent) + if (!virDomainObjIsActive(vm)) qemuDomainRemoveInactive(driver, vm); cleanup: @@ -5804,7 +5800,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver, VIR_WARN("Unable to encode migration cookie"); qemuMigrationJobFinish(driver, vm); - if (!vm->persistent && !virDomainObjIsActive(vm)) + if (!virDomainObjIsActive(vm)) qemuDomainRemoveInactive(driver, vm); cleanup: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7187dc1..c9beadd 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -323,8 +323,7 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, qemuProcessStop(driver, vm, stopReason, stopFlags); virDomainAuditStop(vm, auditReason); - if (!vm->persistent) - qemuDomainRemoveInactive(driver, vm); + qemuDomainRemoveInactive(driver, vm); cleanup: virObjectUnlock(vm); @@ -3898,8 +3897,7 @@ qemuProcessReconnect(void *opaque) qemuProcessStop(driver, obj, state, stopFlags); } - if (!obj->persistent) - qemuDomainRemoveInactive(driver, obj); + qemuDomainRemoveInactive(driver, obj); cleanup: virDomainObjEndAPI(&obj); @@ -3943,8 +3941,7 @@ qemuProcessReconnectHelper(virDomainObjPtr obj, "might be incomplete")); /* We can't spawn a thread and thus connect to monitor. Kill qemu. */ qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_FAILED, 0); - if (!obj->persistent) - qemuDomainRemoveInactive(src->driver, obj); + qemuDomainRemoveInactive(src->driver, obj); virDomainObjEndAPI(&obj); virObjectUnref(data->conn); @@ -5749,8 +5746,7 @@ qemuProcessAutoDestroy(virDomainObjPtr dom, qemuDomainObjEndJob(driver, dom); - if (!dom->persistent) - qemuDomainRemoveInactive(driver, dom); + qemuDomainRemoveInactive(driver, dom); qemuDomainEventQueue(driver, event); -- 2.4.9

So while working on my previous patches, I've noticed that virDomainRestore implementation in qemu and test drivers has the same problem as I am fixing. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 4 ++-- src/test/test_driver.c | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3532973..562a0b6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6828,8 +6828,6 @@ qemuDomainRestoreFlags(virConnectPtr conn, VIR_WARN("Failed to close %s", path); qemuDomainObjEndJob(driver, vm); - if (ret < 0) - qemuDomainRemoveInactive(driver, vm); cleanup: virDomainDefFree(def); @@ -6837,6 +6835,8 @@ qemuDomainRestoreFlags(virConnectPtr conn, VIR_FREE(xml); VIR_FREE(xmlout); virFileWrapperFdFree(wrapperFd); + if (vm && ret < 0) + qemuDomainRemoveInactive(driver, vm); virDomainObjEndAPI(&vm); virNWFilterUnlockFilterUpdates(); return ret; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 01ab1e3..9ccd567 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2132,8 +2132,13 @@ testDomainRestoreFlags(virConnectPtr conn, goto cleanup; def = NULL; - if (testDomainStartState(privconn, dom, VIR_DOMAIN_RUNNING_RESTORED) < 0) + if (testDomainStartState(privconn, dom, VIR_DOMAIN_RUNNING_RESTORED) < 0) { + if (!dom->persistent) { + virDomainObjListRemove(privconn->domains, dom); + dom = NULL; + } goto cleanup; + } event = virDomainEventLifecycleNewFromObj(dom, VIR_DOMAIN_EVENT_STARTED, -- 2.4.9