On 12/21/13 05:14, Adam Walters wrote: > This implements a two-tier driver loading system into libvirt. The two > classes of drivers are "Libvirt" drivers and "Hypervisor" drivers. Hypervisor > drivers are fairly self-explanatory, they provide domain services. Libvirt > drivers are sort of the backend drivers for those, like the secret and > storage drivers. In the two-tier loading system here, the "Libvirt" drivers > are all loaded and auto-started. Once those have finished, the "Hypervisor" > drivers are loaded and auto-started. By doing things in this manner, we do > not have to hard-code a driver loading order or roll our own dynamic > dependency-based loading algorithm, while still gaining the benefits of a > more orderly driver loading approach, which should help minimize the > possibility of a race condition during startup. If another race condition is > found, the code can be extended to provide any number of extra tiers without > much trouble.
Again, as in 2/3, please break the long line into some paragraphs. > > Signed-off-by: Adam Walters <[email protected]> > --- > src/libvirt.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 49 insertions(+), 8 deletions(-) > > diff --git a/src/libvirt.c b/src/libvirt.c > index 77f481e..9c00491 100644 > --- a/src/libvirt.c > +++ b/src/libvirt.c > @@ -837,31 +837,72 @@ int virStateInitialize(bool privileged, > void *opaque) > { > size_t i; > + virStateDriverPtr virLibvirtStateDriverTab[MAX_DRIVERS]; > + int virLibvirtStateDriverTabCount = 0; > + virStateDriverPtr virHypervisorStateDriverTab[MAX_DRIVERS]; > + int virHypervisorStateDriverTabCount = 0; > > if (virInitialize() < 0) > return -1; > > for (i = 0; i < virStateDriverTabCount; i++) { > - if (virStateDriverTab[i]->stateInitialize) { > + switch (virStateDriverTab[i]->stateType) { > + case VIR_DRV_STATE_DRV_LIBVIRT: > + virLibvirtStateDriverTab[virLibvirtStateDriverTabCount++] = > + virStateDriverTab[i]; > + break; > + case VIR_DRV_STATE_DRV_HYPERVISOR: > + virHypervisorStateDriverTab[virHypervisorStateDriverTabCount++] = > + virStateDriverTab[i]; > + break; > + } > + } Hmmm, this duplicates the loading code for each driver tier. As we don't really need to copy the driver structure pointers into separate arrays to load each driver tier I'd suggest the following multi-pass algorithm: for (driverTier = 0; driverTier < driverTierLast; driverTier++) { for (i = 0; i < virStateDriverTabCount i++) { if (virStateDriverTab[i]->stateType != driverTier) continue; ... ALL THE EXISTING LOADER CODE ... } } This'd save a lot of the code duplication and still would keep the ordering you are trying to introduce. I like the overal idea of this series. Peter
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
