[libvirt] [PATCH] redefine pool after pool creation failure
Osier Yang
jyang at redhat.com
Wed Nov 23 03:57:22 UTC 2011
On 2011?11?23? 11:09, 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 at linux.vnet.ibm.com>
> ---
> 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;
use virCommand APIs once creating new codes. Please see
http://libvirt.org/internals/command.html
> +
> + if (virRun(cmdargv, NULL)< 0) {
> + virStorageReportError(VIR_ERR_INTERNAL_ERROR,
> + _("vg '%s' does not exsist,try pool-build"), pool->def->source.name);
> + return -1;
> + }
I tend to disagree this, virStorageBackendLogicalSetActive is not only
used for startPool, but also stopPool. It's strange to see "try pool-build"
while the user wants to stop it.
> +
> 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;
coding style nits. It should be:
} else {
pool = NULL;
}
Please take a look at HACKING before making patches.
NACK, the pool is already redefined with previous
virStoragePoolObjAssignDef,
this is just a duplicate work, and this won't fix your problem, the pool def
will be free()ed.
> +
> + 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;
Likewise.
Regards,
Osier
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111123/d240f44e/attachment-0001.htm>
More information about the libvir-list
mailing list