Stop accessing driver->caps directly in QEMU driver

The 'driver->caps' pointer can be changed on the fly. Accessing
it currently requires the global driver lock. Isolate this
access in a single helper, so a future patch can relax the
locking constraints.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

src/qemu/qemu_command.c

@@ -4396,7 +4396,7 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver,
                    bool *hasHwVirt,
                    bool migrating)
 {
-    const virCPUDefPtr host = driver->caps->host.cpu;
+    virCPUDefPtr host = NULL;
     virCPUDefPtr guest = NULL;
     virCPUDefPtr cpu = NULL;
     size_t ncpus = 0;
@@ -4408,9 +4408,15 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver,
     int ret = -1;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     int i;
+    virCapsPtr caps = NULL;
 
     *hasHwVirt = false;
 
+    if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+        goto cleanup;
+
+    host = caps->host.cpu;
+
     if (def->os.arch == VIR_ARCH_I686)
         default_model = "qemu32";
     else
@@ -4595,7 +4601,7 @@ cleanup:
         cpuDataFree(host->arch, data);
     virCPUDefFree(guest);
     virCPUDefFree(cpu);
-
+    virObjectUnref(caps);
     return ret;
 
 no_memory:

src/qemu/qemu_conf.c

@@ -526,6 +526,106 @@ virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver)
     return virObjectRef(driver->config);
 }
 
+
+virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver)
+{
+    size_t i;
+    virCapsPtr caps;
+    virSecurityManagerPtr *sec_managers = NULL;
+    /* Security driver data */
+    const char *doi, *model;
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+
+    /* Basic host arch / guest machine capabilities */
+    if (!(caps = virQEMUCapsInit(driver->qemuCapsCache))) {
+        virReportOOMError();
+        virObjectUnref(cfg);
+        return NULL;
+    }
+
+    if (cfg->allowDiskFormatProbing) {
+        caps->defaultDiskDriverName = NULL;
+        caps->defaultDiskDriverType = VIR_STORAGE_FILE_AUTO;
+    } else {
+        caps->defaultDiskDriverName = "qemu";
+        caps->defaultDiskDriverType = VIR_STORAGE_FILE_RAW;
+    }
+
+    qemuDomainSetPrivateDataHooks(caps);
+    qemuDomainSetNamespaceHooks(caps);
+
+    if (virGetHostUUID(caps->host.host_uuid)) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       "%s", _("cannot get the host uuid"));
+        goto err_exit;
+    }
+
+    /* access sec drivers and create a sec model for each one */
+    sec_managers = virSecurityManagerGetNested(driver->securityManager);
+    if (sec_managers == NULL) {
+        goto err_exit;
+    }
+
+    /* calculate length */
+    for (i = 0; sec_managers[i]; i++)
+        ;
+    caps->host.nsecModels = i;
+
+    if (VIR_ALLOC_N(caps->host.secModels, caps->host.nsecModels) < 0)
+        goto no_memory;
+
+    for (i = 0; sec_managers[i]; i++) {
+        doi = virSecurityManagerGetDOI(sec_managers[i]);
+        model = virSecurityManagerGetModel(sec_managers[i]);
+        if (!(caps->host.secModels[i].model = strdup(model)))
+            goto no_memory;
+        if (!(caps->host.secModels[i].doi = strdup(doi)))
+            goto no_memory;
+        VIR_DEBUG("Initialized caps for security driver \"%s\" with "
+                  "DOI \"%s\"", model, doi);
+    }
+    VIR_FREE(sec_managers);
+
+    virObjectUnref(cfg);
+    return caps;
+
+no_memory:
+    virReportOOMError();
+err_exit:
+    VIR_FREE(sec_managers);
+    virObjectUnref(caps);
+    virObjectUnref(cfg);
+    return NULL;
+}
+
+
+/**
+ * virQEMUDriverGetCapabilities:
+ *
+ * Get a reference to the virCapsPtr instance for the
+ * driver. If @refresh is true, the capabilities will be
+ * rebuilt first
+ *
+ * The caller must release the reference with virObjetUnref
+ *
+ * Returns: a reference to a virCapsPtr instance or NULL
+ */
+virCapsPtr virQEMUDriverGetCapabilities(virQEMUDriverPtr driver,
+                                        bool refresh)
+{
+    if (refresh) {
+        virCapsPtr caps = NULL;
+        if ((caps = virQEMUDriverCreateCapabilities(driver)) == NULL)
+            return NULL;
+
+        virObjectUnref(driver->caps);
+        driver->caps = caps;
+    }
+
+    return virObjectRef(driver->caps);
+}
+
+
 static void
 qemuDriverCloseCallbackFree(void *payload,
                             const void *name ATTRIBUTE_UNUSED)

src/qemu/qemu_conf.h

@@ -183,7 +183,9 @@ struct _virQEMUDriver {
     /* Immutable pointer, lockless APIs. Pointless abstraction */
     ebtablesContext *ebtables;
 
-    /* Require lock while using. Unsafe. XXX */
+    /* Require lock to get a reference on the object,
+     * lockless access thereafter
+     */
     virCapsPtr caps;
 
     /* Immutable pointer, self-locking APIs */
@@ -244,6 +246,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 
 virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver);
 
+virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver);
+virCapsPtr virQEMUDriverGetCapabilities(virQEMUDriverPtr driver,
+                                        bool refresh);
+
 struct qemuDomainDiskInfo {
     bool removable;
     bool locked;

src/qemu/qemu_domain.c

@@ -664,13 +664,19 @@ static void
 qemuDomainObjSaveJob(virQEMUDriverPtr driver, virDomainObjPtr obj)
 {
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    virCapsPtr caps = NULL;
+
+    if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+        goto cleanup;
 
     if (virDomainObjIsActive(obj)) {
-        if (virDomainSaveStatus(driver->caps, cfg->stateDir, obj) < 0)
+        if (virDomainSaveStatus(caps, cfg->stateDir, obj) < 0)
             VIR_WARN("Failed to save status on vm %s", obj->def->name);
     }
 
+cleanup:
     virObjectUnref(cfg);
+    virObjectUnref(caps);
 }
 
 void
@@ -1244,21 +1250,24 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver,
     virCPUDefPtr def_cpu = def->cpu;
     virDomainControllerDefPtr *controllers = NULL;
     int ncontrollers = 0;
+    virCapsPtr caps = NULL;
+
+    if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+        goto cleanup;
 
     /* Update guest CPU requirements according to host CPU */
     if ((flags & VIR_DOMAIN_XML_UPDATE_CPU) &&
         def_cpu &&
         (def_cpu->mode != VIR_CPU_MODE_CUSTOM || def_cpu->model)) {
-        if (!driver->caps ||
-            !driver->caps->host.cpu ||
-            !driver->caps->host.cpu->model) {
+        if (!caps->host.cpu ||
+            !caps->host.cpu->model) {
             virReportError(VIR_ERR_OPERATION_FAILED,
                            "%s", _("cannot get host CPU capabilities"));
             goto cleanup;
         }
 
         if (!(cpu = virCPUDefCopy(def_cpu)) ||
-            cpuUpdate(cpu, driver->caps->host.cpu) < 0)
+            cpuUpdate(cpu, caps->host.cpu) < 0)
             goto cleanup;
         def->cpu = cpu;
     }
@@ -1310,6 +1319,7 @@ cleanup:
         def->controllers = controllers;
         def->ncontrollers = ncontrollers;
     }
+    virObjectUnref(caps);
     return ret;
 }
 
@@ -1888,16 +1898,22 @@ qemuDomainSetFakeReboot(virQEMUDriverPtr driver,
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    virCapsPtr caps = NULL;
+
+    if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+        goto cleanup;
 
     if (priv->fakeReboot == value)
-        return;
+        goto cleanup;
 
     priv->fakeReboot = value;
 
-    if (virDomainSaveStatus(driver->caps, cfg->stateDir, vm) < 0)
+    if (virDomainSaveStatus(caps, cfg->stateDir, vm) < 0)
         VIR_WARN("Failed to save status on vm %s", vm->def->name);
 
+cleanup:
     virObjectUnref(cfg);
+    virObjectUnref(caps);
 }
 
 int

src/qemu/qemu_migration.c

@@ -742,9 +742,13 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
                             unsigned int flags)
 {
     char uuidstr[VIR_UUID_STRING_BUFLEN];
-    char *tmp;
+    char *tmp = NULL;
     xmlNodePtr *nodes = NULL;
     int i, n;
+    virCapsPtr caps = NULL;
+
+    if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+        goto error;
 
     /* We don't store the uuid, name, hostname, or hostuuid
      * values. We just compare them to local data to do some
@@ -870,7 +874,7 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
                            n);
             goto error;
         }
-        mig->persistent = virDomainDefParseNode(driver->caps, doc, nodes[0],
+        mig->persistent = virDomainDefParseNode(caps, doc, nodes[0],
                                                 -1, VIR_DOMAIN_XML_INACTIVE);
         if (!mig->persistent) {
             /* virDomainDefParseNode already reported
@@ -885,11 +889,13 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
         (!(mig->network = qemuMigrationCookieNetworkXMLParse(ctxt))))
         goto error;
 
+    virObjectUnref(caps);
     return 0;
 
 error:
     VIR_FREE(tmp);
     VIR_FREE(nodes);
+    virObjectUnref(caps);
     return -1;
 }
 
@@ -1434,12 +1440,16 @@ char *qemuMigrationBegin(virQEMUDriverPtr driver,
     qemuMigrationCookiePtr mig = NULL;
     virDomainDefPtr def = NULL;
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    virCapsPtr caps = NULL;
 
     VIR_DEBUG("driver=%p, vm=%p, xmlin=%s, dname=%s,"
               " cookieout=%p, cookieoutlen=%p, flags=%lx",
               driver, vm, NULLSTR(xmlin), NULLSTR(dname),
               cookieout, cookieoutlen, flags);
 
+    if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+        goto cleanup;
+
     /* Only set the phase if we are inside QEMU_ASYNC_JOB_MIGRATION_OUT.
      * Otherwise we will start the async job later in the perform phase losing
      * change protection.
@@ -1484,7 +1494,7 @@ char *qemuMigrationBegin(virQEMUDriverPtr driver,
     }
 
     if (xmlin) {
-        if (!(def = virDomainDefParseString(driver->caps, xmlin,
+        if (!(def = virDomainDefParseString(caps, xmlin,
                                             QEMU_EXPECTED_VIRT_TYPES,
                                             VIR_DOMAIN_XML_INACTIVE)))
             goto cleanup;
@@ -1505,6 +1515,7 @@ char *qemuMigrationBegin(virQEMUDriverPtr driver,
 
 cleanup:
     qemuMigrationCookieFree(mig);
+    virObjectUnref(caps);
     virDomainDefFree(def);
     return rv;
 }
@@ -1555,6 +1566,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
     char *origname = NULL;
     char *xmlout = NULL;
     unsigned int cookieFlags;
+    virCapsPtr caps = NULL;
 
     if (virTimeMillisNow(&now) < 0)
         return -1;
@@ -1581,7 +1593,10 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
         }
     }
 
-    if (!(def = virDomainDefParseString(driver->caps, dom_xml,
+    if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+        goto cleanup;
+
+    if (!(def = virDomainDefParseString(caps, dom_xml,
                                         QEMU_EXPECTED_VIRT_TYPES,
                                         VIR_DOMAIN_XML_INACTIVE)))
         goto cleanup;
@@ -1622,7 +1637,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
                 virDomainDefPtr newdef;
 
                 VIR_DEBUG("Using hook-filtered domain XML: %s", xmlout);
-                newdef = virDomainDefParseString(driver->caps, xmlout,
+                newdef = virDomainDefParseString(caps, xmlout,
                                                  QEMU_EXPECTED_VIRT_TYPES,
                                                  VIR_DOMAIN_XML_INACTIVE);
                 if (!newdef)
@@ -1640,7 +1655,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
     }
 
     if (!(vm = virDomainObjListAdd(driver->domains,
-                                   driver->caps,
+                                   caps,
                                    def,
                                    VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
                                    VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
@@ -1760,6 +1775,7 @@ cleanup:
     if (event)
         qemuDomainEventQueue(driver, event);
     qemuMigrationCookieFree(mig);
+    virObjectUnref(caps);
     return ret;
 
 endjob:
@@ -3323,12 +3339,16 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
     int cookie_flags = 0;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    virCapsPtr caps = NULL;
 
     VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
               "cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d",
               driver, dconn, vm, NULLSTR(cookiein), cookieinlen,
               cookieout, cookieoutlen, flags, retcode);
 
+    if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+        goto cleanup;
+
     if (!qemuMigrationJobIsActive(vm, QEMU_ASYNC_JOB_MIGRATION_IN))
         goto cleanup;
 
@@ -3379,7 +3399,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
             if (mig->persistent)
                 vm->newDef = vmdef = mig->persistent;
             else
-                vmdef = virDomainObjGetPersistentDef(driver->caps, vm);
+                vmdef = virDomainObjGetPersistentDef(caps, vm);
             if (!vmdef || virDomainSaveConfig(cfg->configDir, vmdef) < 0) {
                 /* Hmpf.  Migration was successful, but making it persistent
                  * was not.  If we report successful, then when this domain
@@ -3474,7 +3494,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
         }
 
         if (virDomainObjIsActive(vm) &&
-            virDomainSaveStatus(driver->caps, cfg->stateDir, vm) < 0) {
+            virDomainSaveStatus(caps, cfg->stateDir, vm) < 0) {
             VIR_WARN("Failed to save status on vm %s", vm->def->name);
             goto endjob;
         }
@@ -3513,6 +3533,7 @@ cleanup:
         virSetError(orig_err);
         virFreeError(orig_err);
     }
+    virObjectUnref(caps);
     virObjectUnref(cfg);
     return dom;
 }
@@ -3530,6 +3551,7 @@ int qemuMigrationConfirm(virQEMUDriverPtr driver,
     virDomainEventPtr event = NULL;
     int rv = -1;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    virCapsPtr caps = NULL;
 
     VIR_DEBUG("driver=%p, conn=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
               "flags=%x, retcode=%d",
@@ -3538,6 +3560,9 @@ int qemuMigrationConfirm(virQEMUDriverPtr driver,
 
     virCheckFlags(QEMU_MIGRATION_FLAGS, -1);
 
+    if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+        goto cleanup;
+
     qemuMigrationJobSetPhase(driver, vm,
                              retcode == 0
                              ? QEMU_MIGRATION_PHASE_CONFIRM3
@@ -3578,7 +3603,7 @@ int qemuMigrationConfirm(virQEMUDriverPtr driver,
         event = virDomainEventNewFromObj(vm,
                                          VIR_DOMAIN_EVENT_RESUMED,
                                          VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
-        if (virDomainSaveStatus(driver->caps, cfg->stateDir, vm) < 0) {
+        if (virDomainSaveStatus(caps, cfg->stateDir, vm) < 0) {
             VIR_WARN("Failed to save status on vm %s", vm->def->name);
             goto cleanup;
         }
@@ -3591,6 +3616,7 @@ done:
 cleanup:
     if (event)
         qemuDomainEventQueue(driver, event);
+    virObjectUnref(caps);
     virObjectUnref(cfg);
     return rv;
 }