On 07/31/2011 10:58 PM, Lei Li wrote:
Make sure the unique storage pool defined and create from different directory to avoid inconsistent version of volume pool created.
Wrap your commit messages; typically at 70 columns or so (since 'git log' adds some indentation, but you want the end result to still fit in 80 columns for legibility).
Signed-off-by: Lei Li<lilei@linux.vnet.ibm.com> --- src/conf/storage_conf.c | 36 ++++++++++++++++++++++++++++++++++++ src/conf/storage_conf.h | 4 ++++ src/libvirt_private.syms | 2 ++ src/storage/storage_driver.c | 6 ++++++ 4 files changed, 48 insertions(+), 0 deletions(-)
+virStoragePoolObjPtr +virStoragePoolObjFindByPath(virStoragePoolObjListPtr pools, + const char *path) { + unsigned int i; + + for (i = 0 ; i< pools->count ; i++) { + virStoragePoolObjLock(pools->objs[i]); + if (STREQ(pools->objs[i]->def->target.path, path)) + return pools->objs[i]; + virStoragePoolObjUnlock(pools->objs[i]); + } + + return NULL; +}
This one is good; in fact, we may even want to expose it as a public API, parallel to other virStoragePoolLookupBy* functions (but not until after 0.9.4 is released)
+int virStoragePoolTargetDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def) +{ + int ret = 1; + virStoragePoolObjPtr pool = NULL; + + /* Check the pool list if defined target path already exist */
s/exist/exists/ However, I'm not sure if this warrants a separate function. Instead, should we just fold this additional logic search into virStoragePoolObjIsDuplicate?
+++ b/src/libvirt_private.syms @@ -937,7 +937,9 @@ virStoragePoolObjClearVols; virStoragePoolObjDeleteDef; virStoragePoolObjFindByName; virStoragePoolObjFindByUUID; +virStoragePoolObjFindByPath; virStoragePoolObjIsDuplicate; +virStoragePoolTargetDuplicate;
Sort these lines (or just the virStoragePoolObjFindByPath, if we go with inlining the duplicate target search into virStoragePoolObjIsDuplicate).
+++ b/src/storage/storage_driver.c @@ -536,6 +536,9 @@ storagePoolCreate(virConnectPtr conn, if (virStoragePoolObjIsDuplicate(&driver->pools, def, 1)< 0) goto cleanup;
+ if (virStoragePoolTargetDuplicate(&driver->pools, def)< 0) + goto cleanup; +
Given my consolidation ideas, you wouldn't even have to touch storage_driver.c. While I like the idea, I think that it was first proposed too late after the 0.9.4 RC1, and it isn't a show-stopper bug, so I'd feel better deferring this to post-release and a v3 patch. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org