<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    On 2011年11月23日 11:09, Wen Ruo Lv wrote:
    <blockquote
      cite="mid:1322017756-6121-1-git-send-email-lvroyce@linux.vnet.ibm.com"
      type="cite">
      <pre wrap="">Redefine pool after pool creation failure,give out err msg for non-exsisted vg when starting pool.
ref:
<a class="moz-txt-link-freetext" href="http://www.redhat.com/archives/libvir-list/2011-November/msg01152.html">http://www.redhat.com/archives/libvir-list/2011-November/msg01152.html</a>
<a class="moz-txt-link-freetext" href="http://www.redhat.com/archives/libvir-list/2011-November/msg01152.html">http://www.redhat.com/archives/libvir-list/2011-November/msg01152.html</a>

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 <a class="moz-txt-link-rfc2396E" href="mailto:lvroyce@linux.vnet.ibm.com"><lvroyce@linux.vnet.ibm.com></a>
---
 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;</pre>
    </blockquote>
    <br>
    use virCommand APIs once creating new codes. Please see<br>
    <meta http-equiv="content-type" content="text/html;
      charset=ISO-8859-1">
    <a href="http://libvirt.org/internals/command.html">http://libvirt.org/internals/command.html</a><br>
    <br>
    <blockquote
      cite="mid:1322017756-6121-1-git-send-email-lvroyce@linux.vnet.ibm.com"
      type="cite">
      <pre wrap="">
+
+    if (virRun(cmdargv, NULL) < 0) {
+        virStorageReportError(VIR_ERR_INTERNAL_ERROR,
+                                  _("vg '%s' does not exsist,try pool-build"), pool->def->source.name);
+        return -1;
+    }</pre>
    </blockquote>
    <br>
    I tend to disagree this,  virStorageBackendLogicalSetActive is not
    only<br>
    used for startPool, but also stopPool. It's strange to see "try
    pool-build"<br>
    while the user wants to stop it.<br>
    <br>
    <blockquote
      cite="mid:1322017756-6121-1-git-send-email-lvroyce@linux.vnet.ibm.com"
      type="cite">
      <pre wrap="">
+
     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;</pre>
    </blockquote>
    <br>
    coding style nits. It should be:<br>
                     } else {<br>
                          pool = NULL;<br>
                     }<br>
    <br>
    Please take a look at HACKING before making patches.<br>
    <br>
    NACK, the pool is already redefined with previous
    virStoragePoolObjAssignDef,<br>
    this is just a duplicate work, and this won't fix your problem, the
    pool def<br>
    will be free()ed.<br>
    <br>
    <blockquote
      cite="mid:1322017756-6121-1-git-send-email-lvroyce@linux.vnet.ibm.com"
      type="cite">
      <pre wrap="">
+
+        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;</pre>
    </blockquote>
    <br>
    Likewise.<br>
    <br>
    Regards,<br>
    Osier<br>
  </body>
</html>