[libvirt] [PATCH 2/2] virStateDriver - Separate AutoStart from Initialize

[John Ferlan]

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