On Wed, Nov 23, 2011 at 11:09:16AM +0800, Wen Ruo Lv wrote: > Redefine pool after pool creation failure,give out err msg for non-exsisted vg when starting pool. > ref: > http://www.redhat.com/archives/libvir-list/2011-November/msg01152.html > http://www.redhat.com/archives/libvir-list/2011-November/msg01152.html > > src/storage/storage_backend_logical.c:just give out err msg for non-exsisted vg,not combine pool-build in pool-create ,since undo build for kinds of pool(iscsi,multipath) when err occours cause a lot of trouble. > > src/storage/storage_driver.c:add redefinition of pool,pool may change in pool-start,redefine it after free the pool > > Signed-off-by: Wen Ruo Lv <lvroyce@xxxxxxxxxxxxxxxxxx> > --- > src/storage/storage_backend_logical.c | 10 ++++++++++ > src/storage/storage_driver.c | 29 +++++++++++++++++++++++++---- > 2 files changed, 35 insertions(+), 4 deletions(-) > > diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c > index 3c3e736..994c792 100644 > --- a/src/storage/storage_backend_logical.c > +++ b/src/storage/storage_backend_logical.c > @@ -50,6 +50,16 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool, > { > const char *cmdargv[4]; > > + cmdargv[0] = VGS; > + cmdargv[1] = pool->def->source.name; > + cmdargv[2] = NULL; > + > + if (virRun(cmdargv, NULL) < 0) { > + virStorageReportError(VIR_ERR_INTERNAL_ERROR, > + _("vg '%s' does not exsist,try pool-build"), pool->def->source.name); > + return -1; > + } > + > cmdargv[0] = VGCHANGE; > cmdargv[1] = on ? "-ay" : "-an"; > cmdargv[2] = pool->def->source.name; > diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c > index 8c2d6e1..a5cbafe 100644 > --- a/src/storage/storage_driver.c > +++ b/src/storage/storage_driver.c > @@ -526,6 +526,7 @@ storagePoolCreate(virConnectPtr conn, > virStoragePoolObjPtr pool = NULL; > virStoragePoolPtr ret = NULL; > virStorageBackendPtr backend; > + int duppool; > > virCheckFlags(0, NULL); > > @@ -533,7 +534,7 @@ storagePoolCreate(virConnectPtr conn, > if (!(def = virStoragePoolDefParseString(xml))) > goto cleanup; > > - if (virStoragePoolObjIsDuplicate(&driver->pools, def, 1) < 0) > + if ((duppool = virStoragePoolObjIsDuplicate(&driver->pools, def, 1)) < 0) > goto cleanup; > > if (virStoragePoolSourceFindDuplicate(&driver->pools, def) < 0) > @@ -544,26 +545,46 @@ storagePoolCreate(virConnectPtr conn, > > if (!(pool = virStoragePoolObjAssignDef(&driver->pools, def))) > goto cleanup; > - def = NULL; > > if (backend->startPool && > backend->startPool(conn, pool) < 0) { > virStoragePoolObjRemove(&driver->pools, pool); > - pool = NULL; > + > + if (duppool) { > + if (!(def = virStoragePoolDefParseString(xml))) > + goto cleanup; > + > + pool = virStoragePoolObjAssignDef(&driver->pools, def); > + } > + else > + pool = NULL; > + > + def = NULL; > goto cleanup; > } > > if (backend->refreshPool(conn, pool) < 0) { > if (backend->stopPool) > backend->stopPool(conn, pool); > + > virStoragePoolObjRemove(&driver->pools, pool); > - pool = NULL; > + > + if (duppool) { > + if (!(def = virStoragePoolDefParseString(xml))) > + goto cleanup; > + pool = virStoragePoolObjAssignDef(&driver->pools, def); > + } > + else > + pool = NULL; > + > + def = NULL; > goto cleanup; > } > VIR_INFO("Creating storage pool '%s'", pool->def->name); > pool->active = 1; > > ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid); > + def = NULL; > I don't really like this approach of re-parsing XML & recreating the object, because I don't think this guarentees we get back to the original state. The real fix is to not delete the original object in the first place. 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list