[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] PATCH: Remove virStringList & improve virStoragePoolSourceList



Chris' recent patch removed use of the virStringList for the LVM discovery,
but it was still used in the NFS discovery code. I figured we might as well
make NFS discovery  use virStoragePoolSourceList() too. This require a few
changes:

 - Include pool type in virStoragePoolSourceList so XML formatter knows
   what type of source information is available
 - Factor out code to format a single virStoragePoolSourcePtr object
   into a virStoragePoolSourceFormat method
 - Make virStoragePoolDefFormat & virStoragePoolListFormat both call
   this common method
 - Make LVM discovery code set the pool type in source list
 - Switch NFS discovery to use virStoragePoolSourceList

This also has benfit of giving proper indentation in the XML

# ./virsh  find-storage-pool-sources-as logical
<sources>
  <source>
    <device path='/dev/sda2'/>
    <name>HostVG</name>
    <format type='lvm2'/>
  </source>
</sources>

# ./virsh  find-storage-pool-sources-as --host aubergine.redhat.com netfs
<sources>
  <source>
    <host name='aubergine.redhat.com'/>
    <dir path='/mnt/export'/>
    <format type='nfs'/>
  </source>
  <source>
    <host name='aubergine.redhat.com'/>
    <dir path='/home'/>
    <format type='nfs'/>
  </source>
</sources>

 internal.h                |   12 ---
 libvirt.c                 |   40 ------------
 storage_backend_fs.c      |   46 +++++++-------
 storage_backend_logical.c |    3 
 storage_conf.c            |  150 +++++++++++++++++++++++++++-------------------
 storage_conf.h            |    1 
 6 files changed, 120 insertions(+), 132 deletions(-)

Daniel

diff -r 32908ef883dc src/internal.h
--- a/src/internal.h	Wed Oct 29 11:26:37 2008 +0000
+++ b/src/internal.h	Wed Oct 29 11:29:16 2008 +0000
@@ -371,18 +371,6 @@
 int __virDomainMigratePerform (virDomainPtr domain, const char *cookie, int cookielen, const char *uri, unsigned long flags, const char *dname, unsigned long bandwidth);
 virDomainPtr __virDomainMigrateFinish (virConnectPtr dconn, const char *dname, const char *cookie, int cookielen, const char *uri, unsigned long flags);
 
-typedef struct _virStringList virStringList;
-
-struct _virStringList {
-    char *val;
-    int len;
-    struct _virStringList *next;
-};
-
-char *__virStringListJoin(const virStringList *list, const char *pre,
-                        const char *post, const char *sep);
-void __virStringListFree(virStringList *list);
-
 /**
  * Domain Event Notification
  */
diff -r 32908ef883dc src/libvirt.c
--- a/src/libvirt.c	Wed Oct 29 11:26:37 2008 +0000
+++ b/src/libvirt.c	Wed Oct 29 11:29:16 2008 +0000
@@ -5305,46 +5305,6 @@
 
 
 
-
-/* Not for public use.  Combines the elements of a virStringList
- * into a single string.
- */
-char *__virStringListJoin(const virStringList *list, const char *pre,
-                        const char *post, const char *sep)
-{
-    size_t pre_len = strlen(pre);
-    size_t sep_len = strlen(sep);
-    size_t len = pre_len + strlen(post);
-    const virStringList *p;
-    char *retval;
-
-    for (p = list; p; p = p->next)
-        len += p->len + sep_len;
-    if (VIR_ALLOC_N(retval, len+1) < 0)
-        return NULL;
-    strcpy(retval, pre);
-    len = pre_len;
-    for (p = list; p; p = p->next) {
-        strcpy(retval + len, p->val);
-        len += p->len;
-        strcpy(retval + len, sep);
-        len += sep_len;
-    }
-    strcpy(retval + len, post);
-
-    return retval;
-}
-
-
-void __virStringListFree(virStringList *list)
-{
-    while (list) {
-        virStringList *p = list->next;
-        VIR_FREE(list);
-        list = p;
-    }
-}
-
 /*
  * Domain Event Notification
  */
diff -r 32908ef883dc src/storage_backend_fs.c
--- a/src/storage_backend_fs.c	Wed Oct 29 11:26:37 2008 +0000
+++ b/src/storage_backend_fs.c	Wed Oct 29 11:29:16 2008 +0000
@@ -295,7 +295,7 @@
 #if WITH_STORAGE_FS
 struct _virNetfsDiscoverState {
     const char *host;
-    virStringList *list;
+    virStoragePoolSourceList list;
 };
 
 typedef struct _virNetfsDiscoverState virNetfsDiscoverState;
@@ -307,8 +307,8 @@
                                                   void *data)
 {
     virNetfsDiscoverState *state = data;
-    virStringList *newItem;
     const char *name, *path;
+    virStoragePoolSource *src;
 
     path = groups[0];
 
@@ -325,24 +325,17 @@
         return -1;
     }
 
-    /* Append new XML desc to list */
-
-    if (VIR_ALLOC(newItem) != 0) {
-        virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("new xml desc"));
+    if (VIR_REALLOC_N(state->list.sources, state->list.nsources+1) < 0) {
+        virStorageReportError(conn, VIR_ERR_NO_MEMORY, NULL);
         return -1;
     }
+    memset(state->list.sources + state->list.nsources, 0, sizeof(*state->list.sources));
 
-    if (asprintf(&newItem->val,
-                 "<source><host name='%s'/><dir path='%s'/></source>",
-                 state->host, path) <= 0) {
-        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("asprintf failed"));
-        VIR_FREE(newItem);
+    src = state->list.sources + state->list.nsources++;
+    if (!(src->host.name = strdup(state->host)) ||
+        !(src->dir = strdup(path)))
         return -1;
-    }
-
-    newItem->len = strlen(newItem->val);
-    newItem->next = state->list;
-    state->list = newItem;
+    src->format = VIR_STORAGE_POOL_NETFS_NFS;
 
     return 0;
 }
@@ -368,10 +361,18 @@
     };
     xmlDocPtr doc = NULL;
     xmlXPathContextPtr xpath_ctxt = NULL;
-    virNetfsDiscoverState state = { .host = NULL, .list = NULL };
+    virNetfsDiscoverState state = {
+        .host = NULL,
+        .list = {
+            .type = VIR_STORAGE_POOL_NETFS,
+            .nsources = 0,
+            .sources = NULL
+        }
+    };
     const char *prog[] = { SHOWMOUNT, "--no-headers", "--exports", NULL, NULL };
     int exitstatus;
     char *retval = NULL;
+    unsigned int i;
 
     doc = xmlReadDoc((const xmlChar *)srcSpec, "srcSpec.xml", NULL,
                      XML_PARSE_NOENT | XML_PARSE_NONET |
@@ -400,18 +401,21 @@
                                       &state, &exitstatus) < 0)
         goto cleanup;
 
-    retval = __virStringListJoin(state.list, SOURCES_START_TAG,
-                                 SOURCES_END_TAG, "\n");
+    retval = virStoragePoolSourceListFormat(conn, &state.list);
     if (retval == NULL) {
         virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("retval"));
         goto cleanup;
     }
 
  cleanup:
+    for (i = 0; i < state.list.nsources; i++)
+        virStoragePoolSourceFree(&state.list.sources[i]);
+
+    VIR_FREE(state.list.sources);
+    VIR_FREE(state.host);
+
     xmlFreeDoc(doc);
     xmlXPathFreeContext(xpath_ctxt);
-    VIR_FREE(state.host);
-    __virStringListFree(state.list);
 
     return retval;
 }
diff -r 32908ef883dc src/storage_backend_logical.c
--- a/src/storage_backend_logical.c	Wed Oct 29 11:26:37 2008 +0000
+++ b/src/storage_backend_logical.c	Wed Oct 29 11:29:16 2008 +0000
@@ -295,6 +295,7 @@
 
     dev = &thisSource->devices[thisSource->ndevice];
     thisSource->ndevice++;
+    thisSource->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
 
     memset(dev, 0, sizeof(*dev));
     dev->path = pvname;
@@ -331,6 +332,8 @@
     int i;
 
     memset(&sourceList, 0, sizeof(sourceList));
+    sourceList.type = VIR_STORAGE_POOL_LOGICAL;
+
     if (virStorageBackendRunProgRegex(conn, NULL, prog, 1, regexes, vars,
                                       virStorageBackendLogicalFindPoolSourcesFunc,
                                       &sourceList, &exitstatus) < 0)
diff -r 32908ef883dc src/storage_conf.c
--- a/src/storage_conf.c	Wed Oct 29 11:26:37 2008 +0000
+++ b/src/storage_conf.c	Wed Oct 29 11:29:16 2008 +0000
@@ -472,6 +472,68 @@
     return NULL;
 }
 
+static int
+virStoragePoolSourceFormat(virConnectPtr conn,
+                           virBufferPtr buf,
+                           virStorageBackendPoolOptionsPtr options,
+                           virStoragePoolSourcePtr src)
+{
+    int i, j;
+
+    virBufferAddLit(buf,"  <source>\n");
+    if ((options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_HOST) &&
+        src->host.name)
+        virBufferVSprintf(buf,"    <host name='%s'/>\n", src->host.name);
+
+    if ((options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE) &&
+        src->ndevice) {
+        for (i = 0 ; i < src->ndevice ; i++) {
+            if (src->devices[i].nfreeExtent) {
+                virBufferVSprintf(buf,"    <device path='%s'>\n",
+                                  src->devices[i].path);
+                for (j = 0 ; j < src->devices[i].nfreeExtent ; j++) {
+                    virBufferVSprintf(buf, "    <freeExtent start='%llu' end='%llu'/>\n",
+                                      src->devices[i].freeExtents[j].start,
+                                      src->devices[i].freeExtents[j].end);
+                }
+                virBufferAddLit(buf,"    </device>\n");
+            }
+            else
+                virBufferVSprintf(buf, "    <device path='%s'/>\n",
+                                  src->devices[i].path);
+        }
+    }
+    if ((options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_DIR) &&
+        src->dir)
+        virBufferVSprintf(buf,"    <dir path='%s'/>\n", src->dir);
+    if ((options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_ADAPTER) &&
+        src->adapter)
+        virBufferVSprintf(buf,"    <adapter name='%s'/>\n", src->adapter);
+    if ((options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_NAME) &&
+        src->name)
+        virBufferVSprintf(buf,"    <name>%s</name>\n", src->name);
+
+    if (options->formatToString) {
+        const char *format = (options->formatToString)(src->format);
+        if (!format) {
+            virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                                  _("unknown pool format number %d"),
+                                  src->format);
+            return -1;
+        }
+        virBufferVSprintf(buf,"    <format type='%s'/>\n", format);
+    }
+
+
+    if (src->authType == VIR_STORAGE_POOL_AUTH_CHAP)
+        virBufferVSprintf(buf,"    <auth type='chap' login='%s' passwd='%s'>\n",
+                          src->auth.chap.login,
+                          src->auth.chap.passwd);
+    virBufferAddLit(buf,"  </source>\n");
+
+    return 0;
+}
+
 
 char *
 virStoragePoolDefFormat(virConnectPtr conn,
@@ -480,7 +542,6 @@
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     const char *type;
     char uuid[VIR_UUID_STRING_BUFLEN];
-    int i, j;
 
     options = virStorageBackendPoolOptionsForType(def->type);
     if (options == NULL)
@@ -505,56 +566,8 @@
     virBufferVSprintf(&buf,"  <available>%llu</available>\n",
                       def->available);
 
-    virBufferAddLit(&buf,"  <source>\n");
-    if ((options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_HOST) &&
-        def->source.host.name)
-        virBufferVSprintf(&buf,"    <host name='%s'/>\n", def->source.host.name);
-
-    if ((options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE) &&
-        def->source.ndevice) {
-        for (i = 0 ; i < def->source.ndevice ; i++) {
-            if (def->source.devices[i].nfreeExtent) {
-                virBufferVSprintf(&buf,"    <device path='%s'>\n",
-                                  def->source.devices[i].path);
-                for (j = 0 ; j < def->source.devices[i].nfreeExtent ; j++) {
-                    virBufferVSprintf(&buf, "    <freeExtent start='%llu' end='%llu'/>\n",
-                                      def->source.devices[i].freeExtents[j].start,
-                                      def->source.devices[i].freeExtents[j].end);
-                }
-                virBufferAddLit(&buf,"    </device>\n");
-            }
-            else
-                virBufferVSprintf(&buf, "    <device path='%s'/>\n",
-                                  def->source.devices[i].path);
-        }
-    }
-    if ((options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_DIR) &&
-        def->source.dir)
-        virBufferVSprintf(&buf,"    <dir path='%s'/>\n", def->source.dir);
-    if ((options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_ADAPTER) &&
-        def->source.adapter)
-        virBufferVSprintf(&buf,"    <adapter name='%s'/>\n", def->source.adapter);
-    if ((options->flags & VIR_STORAGE_BACKEND_POOL_SOURCE_NAME) &&
-        def->source.name)
-        virBufferVSprintf(&buf,"    <name>%s</name>\n", def->source.name);
-
-    if (options->formatToString) {
-        const char *format = (options->formatToString)(def->source.format);
-        if (!format) {
-            virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
-                                  _("unknown pool format number %d"),
-                                  def->source.format);
-            goto cleanup;
-        }
-        virBufferVSprintf(&buf,"    <format type='%s'/>\n", format);
-    }
-
-
-    if (def->source.authType == VIR_STORAGE_POOL_AUTH_CHAP)
-        virBufferVSprintf(&buf,"    <auth type='chap' login='%s' passwd='%s'>\n",
-                          def->source.auth.chap.login,
-                          def->source.auth.chap.passwd);
-    virBufferAddLit(&buf,"  </source>\n");
+    if (virStoragePoolSourceFormat(conn, &buf, options, &def->source) < 0)
+        goto cleanup;
 
     virBufferAddLit(&buf,"  <target>\n");
 
@@ -1271,22 +1284,41 @@
     return 0;
 }
 
-char *virStoragePoolSourceListFormat(virConnectPtr conn ATTRIBUTE_UNUSED,
+char *virStoragePoolSourceListFormat(virConnectPtr conn,
                                      virStoragePoolSourceListPtr def)
 {
-    int i, j;
+    virStorageBackendPoolOptionsPtr options;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
+    const char *type;
+    int i;
 
-    virBufferAddLit(&buf, "<sources>");
+    options = virStorageBackendPoolOptionsForType(def->type);
+    if (options == NULL)
+        return NULL;
+
+    type = virStorageBackendToString(def->type);
+    if (!type) {
+        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                              "%s", _("unexpected pool type"));
+        goto cleanup;
+    }
+
+    virBufferAddLit(&buf, "<sources>\n");
 
     for (i = 0; i < def->nsources; i++) {
-        virBufferVSprintf(&buf, "<source><name>%s</name>", def->sources[i].name);
-        for (j = 0; j < def->sources[i].ndevice; j++)
-            virBufferVSprintf(&buf, "<device path='%s'/>", def->sources[i].devices[j].path);
-        virBufferAddLit(&buf, "</source>");
+        virStoragePoolSourceFormat(conn, &buf, options, &def->sources[i]);
     }
 
-    virBufferAddLit(&buf, "</sources>");
+    virBufferAddLit(&buf, "</sources>\n");
+
+    if (virBufferError(&buf))
+        goto no_memory;
 
     return virBufferContentAndReset(&buf);
+
+ no_memory:
+    virStorageReportError(conn, VIR_ERR_NO_MEMORY, NULL);
+ cleanup:
+    free(virBufferContentAndReset(&buf));
+    return NULL;
 }
diff -r 32908ef883dc src/storage_conf.h
--- a/src/storage_conf.h	Wed Oct 29 11:26:37 2008 +0000
+++ b/src/storage_conf.h	Wed Oct 29 11:29:16 2008 +0000
@@ -252,6 +252,7 @@
 typedef struct _virStoragePoolSourceList virStoragePoolSourceList;
 typedef virStoragePoolSourceList *virStoragePoolSourceListPtr;
 struct _virStoragePoolSourceList {
+    int type;
     unsigned int nsources;
     virStoragePoolSourcePtr sources;
 };


-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]