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 :|

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