Remove use of virStringList in favour of strongly typed APIs

ChangeLog

@@ -1,3 +1,17 @@
+Tue Nov  4 21:50:31 UTC 2008 Daniel P. Berrange <berrange@redhat.com>
+
+	Remove use of virStringList
+	* src/internal.h, src/libvirt.c: Remove virStringList code
+	* src/storage_backend_fs.c: Replace use of virStringList
+	with virStoragePoolSourceList
+	* src/storage_backend_logical.c: Set format to LVM2 and
+	source type to LOGICAL
+	* src/storage_conf.c: Refactor pool source XML formating
+	into virStoragePoolSourceFormat and make both users call
+	the common code
+	* src/storage_conf.h:  Include pool source type in
+	virStoragePoolSourceList struct
+
 Tue Nov  4 14:58:31 CET 2008 Jim Meyering <meyering@redhat.com>
 
 	don't print an uninitialized integer in a diagnostic

src/internal.h

@@ -371,18 +371,6 @@ int __virDomainMigratePrepare (virConnectPtr dconn, char **cookie, int *cookiele
 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
  */

src/libvirt.c

@@ -5305,46 +5305,6 @@ virStorageVolGetPath(virStorageVolPtr vol)
 
 
 
-
-/* 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
  */

src/storage_backend_fs.c

@@ -295,7 +295,7 @@ static int virStorageBackendProbeFile(virConnectPtr conn,
 #if WITH_STORAGE_FS
 struct _virNetfsDiscoverState {
     const char *host;
-    virStringList *list;
+    virStoragePoolSourceList list;
 };
 
 typedef struct _virNetfsDiscoverState virNetfsDiscoverState;
@@ -307,8 +307,8 @@ virStorageBackendFileSystemNetFindPoolSourcesFunc(virConnectPtr conn ATTRIBUTE_U
                                                   void *data)
 {
     virNetfsDiscoverState *state = data;
-    virStringList *newItem;
     const char *name, *path;
+    virStoragePoolSource *src;
 
     path = groups[0];
 
@@ -325,24 +325,17 @@ virStorageBackendFileSystemNetFindPoolSourcesFunc(virConnectPtr conn ATTRIBUTE_U
         return -1;
     }
 
-    /* Append new XML desc to list */
+    if (VIR_REALLOC_N(state->list.sources, state->list.nsources+1) < 0) {
-
+        virStorageReportError(conn, VIR_ERR_NO_MEMORY, NULL);
-    if (VIR_ALLOC(newItem) != 0) {
-        virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("new xml desc"));
         return -1;
     }
+    memset(state->list.sources + state->list.nsources, 0, sizeof(*state->list.sources));
 
-    if (asprintf(&newItem->val,
+    src = state->list.sources + state->list.nsources++;
-                 "<source><host name='%s'/><dir path='%s'/></source>",
+    if (!(src->host.name = strdup(state->host)) ||
-                 state->host, path) <= 0) {
+        !(src->dir = strdup(path)))
-        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("asprintf failed"));
-        VIR_FREE(newItem);
         return -1;
-    }
+    src->format = VIR_STORAGE_POOL_NETFS_NFS;
-
-    newItem->len = strlen(newItem->val);
-    newItem->next = state->list;
-    state->list = newItem;
 
     return 0;
 }
@@ -368,10 +361,18 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn,
     };
     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 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn,
                                       &state, &exitstatus) < 0)
         goto cleanup;
 
-    retval = __virStringListJoin(state.list, SOURCES_START_TAG,
+    retval = virStoragePoolSourceListFormat(conn, &state.list);
-                                 SOURCES_END_TAG, "\n");
     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;
 }

src/storage_backend_logical.c

@@ -295,6 +295,7 @@ virStorageBackendLogicalFindPoolSourcesFunc(virConnectPtr conn,
 
     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 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn,
     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)

src/storage_conf.c

@@ -472,6 +472,68 @@ virStoragePoolDefParse(virConnectPtr conn,
     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 @@ virStoragePoolDefFormat(virConnectPtr conn,
     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 @@ virStoragePoolDefFormat(virConnectPtr conn,
     virBufferVSprintf(&buf,"  <available>%llu</available>\n",
                       def->available);
 
-    virBufferAddLit(&buf,"  <source>\n");
+    if (virStoragePoolSourceFormat(conn, &buf, options, &def->source) < 0)
-    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");
 
     virBufferAddLit(&buf,"  <target>\n");
 
@@ -1271,22 +1284,41 @@ virStoragePoolObjDeleteDef(virConnectPtr conn,
     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;
 
-    for (i = 0; i < def->nsources; i++) {
+    type = virStorageBackendToString(def->type);
-        virBufferVSprintf(&buf, "<source><name>%s</name>", def->sources[i].name);
+    if (!type) {
-        for (j = 0; j < def->sources[i].ndevice; j++)
+        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
-            virBufferVSprintf(&buf, "<device path='%s'/>", def->sources[i].devices[j].path);
+                              "%s", _("unexpected pool type"));
-        virBufferAddLit(&buf, "</source>");
+        goto cleanup;
     }
 
-    virBufferAddLit(&buf, "</sources>");
+    virBufferAddLit(&buf, "<sources>\n");
+
+    for (i = 0; i < def->nsources; i++) {
+        virStoragePoolSourceFormat(conn, &buf, options, &def->sources[i]);
+    }
+
+    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;
 }

src/storage_conf.h

@@ -252,6 +252,7 @@ struct _virStorageDriverState {
 typedef struct _virStoragePoolSourceList virStoragePoolSourceList;
 typedef virStoragePoolSourceList *virStoragePoolSourceListPtr;
 struct _virStoragePoolSourceList {
+    int type;
     unsigned int nsources;
     virStoragePoolSourcePtr sources;
 };