1:32 p.m.
The post push review/comments for the chap authentication determined that
trying to connect to qemu driver from within the storage auto start would
not be successful, see the following and followups
https://www.redhat.com/archives/libvir-list/2013-July/msg01409.html
These patches will split the virStateInitialize() into two phases - the
first being the bulk of the existing code and the second being just running
the auto start functionality for each of the drivers that needs/support it.
I realize it's probably too late for 1.1.1, but figured it'd be good to be
ready when the barn door opens again.
John Ferlan (2):
Separate out StateAutoStart from StateInitialize
virStateDriver - Separate AutoStart from Initialize
src/driver.h | 4 ++++
src/libvirt.c | 23 ++++++++++++++++++++++-
src/libxl/libxl_driver.c | 18 +++++++++++++++---
src/lxc/lxc_driver.c | 18 ++++++++++++++++--
src/network/bridge_driver.c | 20 +++++++++++++++++++-
src/qemu/qemu_driver.c | 18 ++++++++++++++++--
src/storage/storage_driver.c | 19 ++++++++++++++++++-
src/uml/uml_driver.c | 18 ++++++++++++++++--
8 files changed, 126 insertions(+), 12 deletions(-)
--
1.8.1.4
1:32 p.m.
New subject: [libvirt] [PATCH 1/2] Separate out StateAutoStart from StateInitialize
Separation allows for dependent drivers to be make a connection during
the AutoStart phase of state initialization.
---
src/driver.h | 4 ++++
src/libvirt.c | 23 ++++++++++++++++++++++-
2 files changed, 26 insertions(+), 1 deletion(-)
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..864321f 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 on another driver having been initialized
+ * 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,23 @@ 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;
}
--
1.8.1.4
1:39 p.m.
New subject: [libvirt] [PATCH 1/2] Separate out StateAutoStart from StateInitialize
On Thu, Jul 25, 2013 at 02:32:56PM -0400, John Ferlan wrote:
Separation allows for dependent drivers to be make a connection
during
the AutoStart phase of state initialization.
---
src/driver.h | 4 ++++
src/libvirt.c | 23 ++++++++++++++++++++++-
2 files changed, 26 insertions(+), 1 deletion(-)
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..864321f 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 on another driver having been initialized
+ * 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,23 @@ 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;
}
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
1:40 p.m.
New subject: [libvirt] [PATCH 1/2] Separate out StateAutoStart from StateInitialize
On Thu, Jul 25, 2013 at 07:39:35PM +0100, Daniel P. Berrange wrote:
On Thu, Jul 25, 2013 at 02:32:56PM -0400, John Ferlan wrote:
> Separation allows for dependent drivers to be make a connection during
> the AutoStart phase of state initialization.
> ---
> src/driver.h | 4 ++++
> src/libvirt.c | 23 ++++++++++++++++++++++-
> 2 files changed, 26 insertions(+), 1 deletion(-)
>
> 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);
> +
Actually, why not just make this 'void' return value. None of the
impls return any error code....
> @@ -836,6 +840,23 @@ 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;
> + }
...then all this error handling goes away.
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
1:32 p.m.
New subject: [libvirt] [PATCH 2/2] virStateDriver - Separate AutoStart from Initialize
Adjust these drivers to handle their Autostart functionality after each
of the drivers has gone through their Initialization functions
merge
---
src/libxl/libxl_driver.c | 18 +++++++++++++++---
src/lxc/lxc_driver.c | 18 ++++++++++++++++--
src/network/bridge_driver.c | 20 +++++++++++++++++++-
src/qemu/qemu_driver.c | 18 ++++++++++++++++--
src/storage/storage_driver.c | 19 ++++++++++++++++++-
src/uml/uml_driver.c | 18 ++++++++++++++++--
6 files changed, 100 insertions(+), 11 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 98b1985..3dc8b32 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1348,9 +1348,6 @@ libxlStateInitialize(bool privileged,
NULL, NULL) < 0)
goto error;
- virDomainObjListForEach(libxl_driver->domains, libxlAutostartDomain,
- libxl_driver);
-
virDomainObjListForEach(libxl_driver->domains, libxlDomainManagedSaveLoad,
libxl_driver);
@@ -1369,6 +1366,20 @@ fail:
}
static int
+libxlStateAutoStart(void)
+{
+ if (!libxl_driver)
+ return 0;
+
+ libxlDriverLock(libxl_driver);
+ virDomainObjListForEach(libxl_driver->domains, libxlAutostartDomain,
+ libxl_driver);
+ libxlDriverUnlock(libxl_driver);
+
+ return 0;
+}
+
+static int
libxlStateReload(void)
{
if (!libxl_driver)
@@ -4887,6 +4898,7 @@ static virDriver libxlDriver = {
static virStateDriver libxlStateDriver = {
.name = "LIBXL",
.stateInitialize = libxlStateInitialize,
+ .stateAutoStart = libxlStateAutoStart,
.stateCleanup = libxlStateCleanup,
.stateReload = libxlStateReload,
};
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 21cf2e3..75c03e0 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1455,8 +1455,6 @@ static int lxcStateInitialize(bool privileged,
NULL, NULL) < 0)
goto cleanup;
- virLXCProcessAutostartAll(lxc_driver);
-
virNWFilterRegisterCallbackDriver(&lxcCallbackDriver);
return 0;
@@ -1466,6 +1464,21 @@ cleanup:
return -1;
}
+/**
+ * lxcStateAutoStart:
+ *
+ * Function to autostart the LXC daemons
+ */
+static int lxcStateAutoStart(void)
+{
+ if (!lxc_driver)
+ return 0;
+
+ virLXCProcessAutostartAll(lxc_driver);
+
+ return 0;
+}
+
static void lxcNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque)
{
virLXCDriverPtr driver = opaque;
@@ -4665,6 +4678,7 @@ static virDriver lxcDriver = {
static virStateDriver lxcStateDriver = {
.name = LXC_DRIVER_NAME,
.stateInitialize = lxcStateInitialize,
+ .stateAutoStart = lxcStateAutoStart,
.stateCleanup = lxcStateCleanup,
.stateReload = lxcStateReload,
};
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0bb57ea..98dea9e 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -430,7 +430,6 @@ networkStateInitialize(bool privileged,
networkFindActiveConfigs(driverState);
networkReloadFirewallRules(driverState);
networkRefreshDaemons(driverState);
- networkAutostartConfigs(driverState);
networkDriverUnlock(driverState);
@@ -474,6 +473,24 @@ error:
}
/**
+ * networkStateAutoStart:
+ *
+ * Function to AutoStart the bridge configs
+ */
+static int
+networkStateAutoStart(void)
+{
+ if (!driverState)
+ return 0;
+
+ networkDriverLock(driverState);
+ networkAutostartConfigs(driverState);
+ networkDriverUnlock(driverState);
+
+ return 0;
+}
+
+/**
* networkStateReload:
*
* Function to restart the QEmu daemon, it will recheck the configuration
@@ -3693,6 +3710,7 @@ static virNetworkDriver networkDriver = {
static virStateDriver networkStateDriver = {
.name = "Network",
.stateInitialize = networkStateInitialize,
+ .stateAutoStart = networkStateAutoStart,
.stateCleanup = networkStateCleanup,
.stateReload = networkStateReload,
};
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2adf6e2..ec909b3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -837,8 +837,6 @@ qemuStateInitialize(bool privileged,
if (!qemu_driver->workerPool)
goto error;
- qemuAutostartDomains(qemu_driver);
-
if (conn)
virConnectClose(conn);
@@ -855,6 +853,21 @@ error:
return -1;
}
+/**
+ * qemuStateAutoStart:
+ *
+ * Function to auto start the QEmu daemons
+ */
+static int
+qemuStateAutoStart(void)
+{
+ if (!qemu_driver)
+ return 0;
+
+ qemuAutostartDomains(qemu_driver);
+ return 0;
+}
+
static void qemuNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque)
{
virQEMUDriverPtr driver = opaque;
@@ -16276,6 +16289,7 @@ static virDriver qemuDriver = {
static virStateDriver qemuStateDriver = {
.name = "QEMU",
.stateInitialize = qemuStateInitialize,
+ .stateAutoStart = qemuStateAutoStart,
.stateCleanup = qemuStateCleanup,
.stateReload = qemuStateReload,
.stateStop = qemuStateStop,
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 4f0c631..e71a190 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,23 @@ error:
}
/**
+ * storageStateAutoStart:
+ *
+ * Function to auto start the storage driver
+ */
+static int
+storageStateAutoStart(void)
+{
+ if (!driverState)
+ return -1;
+
+ storageDriverLock(driverState);
+ storageDriverAutostart(driverState);
+ storageDriverUnlock(driverState);
+ return 0;
+}
+
+/**
* storageStateReload:
*
* Function to restart the storage driver, it will recheck the configuration
@@ -2599,6 +2615,7 @@ static virStorageDriver storageDriver = {
static virStateDriver stateDriver = {
.name = "Storage",
.stateInitialize = storageStateInitialize,
+ .stateAutoStart = storageStateAutoStart,
.stateCleanup = storageStateCleanup,
.stateReload = storageStateReload,
};
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 7abcd6b..f5e66ae 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -570,8 +570,6 @@ umlStateInitialize(bool privileged,
umlDriverUnlock(uml_driver);
- umlAutostartConfigs(uml_driver);
-
VIR_FREE(userdir);
virNWFilterRegisterCallbackDriver(¨CallbackDriver);
@@ -588,6 +586,21 @@ error:
return -1;
}
+/**
+ * umlStateAutoStart:
+ *
+ * Function to autostart the Uml daemons
+ */
+static int
+umlStateAutoStart(void)
+{
+ if (!uml_driver)
+ return 0;
+
+ umlAutostartConfigs(uml_driver);
+ return 0;
+}
+
static void umlNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque)
{
struct uml_driver *driver = opaque;
@@ -2885,6 +2898,7 @@ static virDriver umlDriver = {
static virStateDriver umlStateDriver = {
.name = "UML",
.stateInitialize = umlStateInitialize,
+ .stateAutoStart = umlStateAutoStart,
.stateCleanup = umlStateCleanup,
.stateReload = umlStateReload,
};
--
1.8.1.4
1:41 p.m.
New subject: [libvirt] [PATCH 2/2] virStateDriver - Separate AutoStart from Initialize
On Thu, Jul 25, 2013 at 02:32:57PM -0400, John Ferlan wrote:
Adjust these drivers to handle their Autostart functionality after
each
of the drivers has gone through their Initialization functions
merge
---
src/libxl/libxl_driver.c | 18 +++++++++++++++---
src/lxc/lxc_driver.c | 18 ++++++++++++++++--
src/network/bridge_driver.c | 20 +++++++++++++++++++-
src/qemu/qemu_driver.c | 18 ++++++++++++++++--
src/storage/storage_driver.c | 19 ++++++++++++++++++-
src/uml/uml_driver.c | 18 ++++++++++++++++--
6 files changed, 100 insertions(+), 11 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 98b1985..3dc8b32 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1348,9 +1348,6 @@ libxlStateInitialize(bool privileged,
NULL, NULL) < 0)
goto error;
- virDomainObjListForEach(libxl_driver->domains, libxlAutostartDomain,
- libxl_driver);
-
virDomainObjListForEach(libxl_driver->domains, libxlDomainManagedSaveLoad,
libxl_driver);
@@ -1369,6 +1366,20 @@ fail:
}
static int
+libxlStateAutoStart(void)
+{
+ if (!libxl_driver)
+ return 0;
+
+ libxlDriverLock(libxl_driver);
+ virDomainObjListForEach(libxl_driver->domains, libxlAutostartDomain,
+ libxl_driver);
+ libxlDriverUnlock(libxl_driver);
+
+ return 0;
+}
+
+static int
libxlStateReload(void)
{
if (!libxl_driver)
@@ -4887,6 +4898,7 @@ static virDriver libxlDriver = {
static virStateDriver libxlStateDriver = {
.name = "LIBXL",
.stateInitialize = libxlStateInitialize,
+ .stateAutoStart = libxlStateAutoStart,
.stateCleanup = libxlStateCleanup,
.stateReload = libxlStateReload,
};
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 21cf2e3..75c03e0 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1455,8 +1455,6 @@ static int lxcStateInitialize(bool privileged,
NULL, NULL) < 0)
goto cleanup;
- virLXCProcessAutostartAll(lxc_driver);
-
virNWFilterRegisterCallbackDriver(&lxcCallbackDriver);
return 0;
@@ -1466,6 +1464,21 @@ cleanup:
return -1;
}
+/**
+ * lxcStateAutoStart:
+ *
+ * Function to autostart the LXC daemons
+ */
+static int lxcStateAutoStart(void)
+{
+ if (!lxc_driver)
+ return 0;
+
+ virLXCProcessAutostartAll(lxc_driver);
+
+ return 0;
+}
+
static void lxcNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque)
{
virLXCDriverPtr driver = opaque;
@@ -4665,6 +4678,7 @@ static virDriver lxcDriver = {
static virStateDriver lxcStateDriver = {
.name = LXC_DRIVER_NAME,
.stateInitialize = lxcStateInitialize,
+ .stateAutoStart = lxcStateAutoStart,
.stateCleanup = lxcStateCleanup,
.stateReload = lxcStateReload,
};
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0bb57ea..98dea9e 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -430,7 +430,6 @@ networkStateInitialize(bool privileged,
networkFindActiveConfigs(driverState);
networkReloadFirewallRules(driverState);
networkRefreshDaemons(driverState);
- networkAutostartConfigs(driverState);
networkDriverUnlock(driverState);
@@ -474,6 +473,24 @@ error:
}
/**
+ * networkStateAutoStart:
+ *
+ * Function to AutoStart the bridge configs
+ */
+static int
+networkStateAutoStart(void)
+{
+ if (!driverState)
+ return 0;
+
+ networkDriverLock(driverState);
+ networkAutostartConfigs(driverState);
+ networkDriverUnlock(driverState);
+
+ return 0;
+}
+
+/**
* networkStateReload:
*
* Function to restart the QEmu daemon, it will recheck the configuration
@@ -3693,6 +3710,7 @@ static virNetworkDriver networkDriver = {
static virStateDriver networkStateDriver = {
.name = "Network",
.stateInitialize = networkStateInitialize,
+ .stateAutoStart = networkStateAutoStart,
.stateCleanup = networkStateCleanup,
.stateReload = networkStateReload,
};
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2adf6e2..ec909b3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -837,8 +837,6 @@ qemuStateInitialize(bool privileged,
if (!qemu_driver->workerPool)
goto error;
- qemuAutostartDomains(qemu_driver);
-
if (conn)
virConnectClose(conn);
@@ -855,6 +853,21 @@ error:
return -1;
}
+/**
+ * qemuStateAutoStart:
+ *
+ * Function to auto start the QEmu daemons
+ */
+static int
+qemuStateAutoStart(void)
+{
+ if (!qemu_driver)
+ return 0;
+
+ qemuAutostartDomains(qemu_driver);
+ return 0;
+}
+
static void qemuNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque)
{
virQEMUDriverPtr driver = opaque;
@@ -16276,6 +16289,7 @@ static virDriver qemuDriver = {
static virStateDriver qemuStateDriver = {
.name = "QEMU",
.stateInitialize = qemuStateInitialize,
+ .stateAutoStart = qemuStateAutoStart,
.stateCleanup = qemuStateCleanup,
.stateReload = qemuStateReload,
.stateStop = qemuStateStop,
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 4f0c631..e71a190 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,23 @@ error:
}
/**
+ * storageStateAutoStart:
+ *
+ * Function to auto start the storage driver
+ */
+static int
+storageStateAutoStart(void)
+{
+ if (!driverState)
+ return -1;
This should have been '0' not '-1' like all the other changes.
That said, I think this function should in fact just be void
removing any return value requirement
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
3:24 p.m.
New subject: [libvirt] [PATCH 2/2] virStateDriver - Separate AutoStart from Initialize
On 07/25/2013 02:41 PM, Daniel P. Berrange wrote:
On Thu, Jul 25, 2013 at 02:32:57PM -0400, John Ferlan wrote:
> Adjust these drivers to handle their Autostart functionality after each
> of the drivers has gone through their Initialization functions
>
> merge
> ---
> src/libxl/libxl_driver.c | 18 +++++++++++++++---
> src/lxc/lxc_driver.c | 18 ++++++++++++++++--
> src/network/bridge_driver.c | 20 +++++++++++++++++++-
> src/qemu/qemu_driver.c | 18 ++++++++++++++++--
> src/storage/storage_driver.c | 19 ++++++++++++++++++-
> src/uml/uml_driver.c | 18 ++++++++++++++++--
> 6 files changed, 100 insertions(+), 11 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 98b1985..3dc8b32 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -1348,9 +1348,6 @@ libxlStateInitialize(bool privileged,
> NULL, NULL) < 0)
> goto error;
>
> - virDomainObjListForEach(libxl_driver->domains, libxlAutostartDomain,
> - libxl_driver);
> -
> virDomainObjListForEach(libxl_driver->domains, libxlDomainManagedSaveLoad,
> libxl_driver);
>
> @@ -1369,6 +1366,20 @@ fail:
> }
>
> static int
> +libxlStateAutoStart(void)
> +{
> + if (!libxl_driver)
> + return 0;
> +
> + libxlDriverLock(libxl_driver);
> + virDomainObjListForEach(libxl_driver->domains, libxlAutostartDomain,
> + libxl_driver);
> + libxlDriverUnlock(libxl_driver);
> +
> + return 0;
> +}
> +
> +static int
> libxlStateReload(void)
> {
> if (!libxl_driver)
> @@ -4887,6 +4898,7 @@ static virDriver libxlDriver = {
> static virStateDriver libxlStateDriver = {
> .name = "LIBXL",
> .stateInitialize = libxlStateInitialize,
> + .stateAutoStart = libxlStateAutoStart,
> .stateCleanup = libxlStateCleanup,
> .stateReload = libxlStateReload,
> };
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 21cf2e3..75c03e0 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -1455,8 +1455,6 @@ static int lxcStateInitialize(bool privileged,
> NULL, NULL) < 0)
> goto cleanup;
>
> - virLXCProcessAutostartAll(lxc_driver);
> -
> virNWFilterRegisterCallbackDriver(&lxcCallbackDriver);
> return 0;
>
> @@ -1466,6 +1464,21 @@ cleanup:
> return -1;
> }
>
> +/**
> + * lxcStateAutoStart:
> + *
> + * Function to autostart the LXC daemons
> + */
> +static int lxcStateAutoStart(void)
> +{
> + if (!lxc_driver)
> + return 0;
> +
> + virLXCProcessAutostartAll(lxc_driver);
> +
> + return 0;
> +}
> +
> static void lxcNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque)
> {
> virLXCDriverPtr driver = opaque;
> @@ -4665,6 +4678,7 @@ static virDriver lxcDriver = {
> static virStateDriver lxcStateDriver = {
> .name = LXC_DRIVER_NAME,
> .stateInitialize = lxcStateInitialize,
> + .stateAutoStart = lxcStateAutoStart,
> .stateCleanup = lxcStateCleanup,
> .stateReload = lxcStateReload,
> };
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 0bb57ea..98dea9e 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -430,7 +430,6 @@ networkStateInitialize(bool privileged,
> networkFindActiveConfigs(driverState);
> networkReloadFirewallRules(driverState);
> networkRefreshDaemons(driverState);
> - networkAutostartConfigs(driverState);
>
> networkDriverUnlock(driverState);
>
> @@ -474,6 +473,24 @@ error:
> }
>
> /**
> + * networkStateAutoStart:
> + *
> + * Function to AutoStart the bridge configs
> + */
> +static int
> +networkStateAutoStart(void)
> +{
> + if (!driverState)
> + return 0;
> +
> + networkDriverLock(driverState);
> + networkAutostartConfigs(driverState);
> + networkDriverUnlock(driverState);
> +
> + return 0;
> +}
> +
> +/**
> * networkStateReload:
> *
> * Function to restart the QEmu daemon, it will recheck the configuration
> @@ -3693,6 +3710,7 @@ static virNetworkDriver networkDriver = {
> static virStateDriver networkStateDriver = {
> .name = "Network",
> .stateInitialize = networkStateInitialize,
> + .stateAutoStart = networkStateAutoStart,
> .stateCleanup = networkStateCleanup,
> .stateReload = networkStateReload,
> };
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 2adf6e2..ec909b3 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -837,8 +837,6 @@ qemuStateInitialize(bool privileged,
> if (!qemu_driver->workerPool)
> goto error;
>
> - qemuAutostartDomains(qemu_driver);
> -
> if (conn)
> virConnectClose(conn);
>
> @@ -855,6 +853,21 @@ error:
> return -1;
> }
>
> +/**
> + * qemuStateAutoStart:
> + *
> + * Function to auto start the QEmu daemons
> + */
> +static int
> +qemuStateAutoStart(void)
> +{
> + if (!qemu_driver)
> + return 0;
> +
> + qemuAutostartDomains(qemu_driver);
> + return 0;
> +}
> +
> static void qemuNotifyLoadDomain(virDomainObjPtr vm, int newVM, void *opaque)
> {
> virQEMUDriverPtr driver = opaque;
> @@ -16276,6 +16289,7 @@ static virDriver qemuDriver = {
> static virStateDriver qemuStateDriver = {
> .name = "QEMU",
> .stateInitialize = qemuStateInitialize,
> + .stateAutoStart = qemuStateAutoStart,
> .stateCleanup = qemuStateCleanup,
> .stateReload = qemuStateReload,
> .stateStop = qemuStateStop,
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 4f0c631..e71a190 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,23 @@ error:
> }
>
> /**
> + * storageStateAutoStart:
> + *
> + * Function to auto start the storage driver
> + */
> +static int
> +storageStateAutoStart(void)
> +{
> + if (!driverState)
> + return -1;
This should have been '0' not '-1' like all the other changes.
That said, I think this function should in fact just be void
removing any return value requirement
Daniel
I think I was following the Reload function here for return value,
although I'm fine with void too. I went back and forth on int or void
for all of them and then for this one back and forth on return value.
E.G.:
static int
storageStateReload(void) {
if (!driverState)
return -1;
John
1:37 p.m.
On Thu, Jul 25, 2013 at 02:32:55PM -0400, John Ferlan wrote:
The post push review/comments for the chap authentication determined
that
trying to connect to qemu driver from within the storage auto start would
not be successful, see the following and followups
https://www.redhat.com/archives/libvir-list/2013-July/msg01409.html
These patches will split the virStateInitialize() into two phases - the
first being the bulk of the existing code and the second being just running
the auto start functionality for each of the drivers that needs/support it.
I realize it's probably too late for 1.1.1, but figured it'd be good to be
ready when the barn door opens again.
I don't know about others, but I feel the change is simple enough
that I wouldn't be against including it in 1.1.1, given that it
solves a real world bug we have in startup ordering.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:19 a.m.
On 25.07.2013 20:37, Daniel P. Berrange wrote:
On Thu, Jul 25, 2013 at 02:32:55PM -0400, John Ferlan wrote:
> The post push review/comments for the chap authentication determined that
> trying to connect to qemu driver from within the storage auto start would
> not be successful, see the following and followups
>
> https://www.redhat.com/archives/libvir-list/2013-July/msg01409.html
>
> These patches will split the virStateInitialize() into two phases - the
> first being the bulk of the existing code and the second being just running
> the auto start functionality for each of the drivers that needs/support it.
>
> I realize it's probably too late for 1.1.1, but figured it'd be good to be
> ready when the barn door opens again.
I don't know about others, but I feel the change is simple enough
that I wouldn't be against including it in 1.1.1, given that it
solves a real world bug we have in startup ordering.
Agreed.
Michal
7:29 a.m.
On Fri, Jul 26, 2013 at 09:19:48AM +0200, Michal Privoznik wrote:
On 25.07.2013 20:37, Daniel P. Berrange wrote:
> On Thu, Jul 25, 2013 at 02:32:55PM -0400, John Ferlan wrote:
>> The post push review/comments for the chap authentication determined that
>> trying to connect to qemu driver from within the storage auto start would
>> not be successful, see the following and followups
>>
>> https://www.redhat.com/archives/libvir-list/2013-July/msg01409.html
>>
>> These patches will split the virStateInitialize() into two phases - the
>> first being the bulk of the existing code and the second being just running
>> the auto start functionality for each of the drivers that needs/support it.
>>
>> I realize it's probably too late for 1.1.1, but figured it'd be good to
be
>> ready when the barn door opens again.
>
> I don't know about others, but I feel the change is simple enough
> that I wouldn't be against including it in 1.1.1, given that it
> solves a real world bug we have in startup ordering.
>
Agreed.
ACK then, John please push :-)
Daniel
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
4173
days inactive
4174
days old
9 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
John Ferlan -
Michal Privoznik