3:06 p.m.
On 07/24/2013 05:57 AM, Daniel P. Berrange wrote:
> On Wed, Jul 24, 2013 at 10:25:06AM +0200, Ján Tomko wrote:
<...snip...>
>
> Both secret and qemu drivers are registered after the storage driver on
> libvirtd startup, so autostarting these pools will only work on storage driver
> reload. On libvirtd startup it fails with:
> qemuConnectOpen:1033 : internal error qemu state driver is not active
>
> (And it seems nwfilter only opens the qemu:// connection on reload)
Oh damn, yes, that pretty much dooms us. We can't change the order of
the drivers either, because autostarting of QEMU guests, requires that
the storage pools be autostarted already.
To fix this would require that we split virStateInitialize into two
parts, virStateInitialize() and virStateAutoStart(). That's too big
a change todo for this release, but we could do it for next release
without too much trouble.
Daniel
Could we just do it for storage driver? It seems you are indicating
that the following would suffice, right? Or does this problem go deeper?
John
diff --git a/src/driver.h b/src/driver.h
index cc03e9f..b416902 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1801,6 +1801,9 @@ typedef int
void *opaque);
typedef int
+(*virDrvStateAutoStart)(void);
+
+typedef int
(*virDrvStateCleanup)(void);
typedef int
@@ -1815,6 +1818,7 @@ typedef virStateDriver *virStateDriverPtr;
struct _virStateDriver {
const char *name;
virDrvStateInitialize stateInitialize;
+ virDrvStateAutoStart stateAutoStart;
virDrvStateCleanup stateCleanup;
virDrvStateReload stateReload;
virDrvStateStop stateStop;
diff --git a/src/libvirt.c b/src/libvirt.c
index 444c1c3..e0bd7aa 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -808,7 +808,11 @@ virRegisterStateDriver(virStateDriverPtr driver)
* @callback: callback to invoke to inhibit shutdown of the daemon
* @opaque: data to pass to @callback
*
- * Initialize all virtualization drivers.
+ * Initialize all virtualization drivers. Accomplished in two phases,
+ * the first being state and structure initialization followed by any
+ * auto start supported by the driver. This is done to ensure dependencies
+ * that some drivers may have will exist. Such as the storage driver's need
+ * to use the secret driver.
*
* Returns 0 if all succeed, -1 upon any failure.
*/
@@ -836,6 +840,22 @@ int virStateInitialize(bool privileged,
}
}
}
+
+ for (i = 0; i < virStateDriverTabCount; i++) {
+ if (virStateDriverTab[i]->stateAutoStart) {
+ VIR_DEBUG("Running global auto start for %s state driver",
+ virStateDriverTab[i]->name);
+ if (virStateDriverTab[i]->stateAutoStart() < 0) {
+ virErrorPtr err = virGetLastError();
+ VIR_ERROR(_("Auto start for %s driver failed: %s"),
+ virStateDriverTab[i]->name,
+ err && err->message ? err->message :
+ _("Unknown problem"));
+ if (virStateDriverTab[i]->stateCleanup)
+ ignore_value(virStateDriverTab[i]->stateCleanup());
+ return -1;
+ }
+ }
+ }
return 0;
}
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 4f0c631..390e196 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -182,7 +182,6 @@ storageStateInitialize(bool privileged,
driverState->configDir,
driverState->autostartDir) < 0)
goto error;
- storageDriverAutostart(driverState);
storageDriverUnlock(driverState);
return 0;
@@ -195,6 +194,20 @@ error:
}
/**
+ * storageStateAutoStart:
+ *
+ * Function to auto start the storage driver
+ */
+static int
+storageStateAutoStart(void)
+{
+ storageDriverLock(driverState);
+ storageDriverAutostart(driverState);
+ storageDriverUnlock(driverState);
+ return 0;
+}
+
+/**
* storageStateReload:
*
* Function to restart the storage driver, it will recheck the configuration
@@ -2599,6 +2612,7 @@ static virStorageDriver storageDriver = {
static virStateDriver stateDriver = {
.name = "Storage",
.stateInitialize = storageStateInitialize,
+ .stateAutoStart = storageStateAutoStart,
.stateCleanup = storageStateCleanup,
.stateReload = storageStateReload,
};