An updated patch for adding <device> tags to logical

findPoolSources.  Given danpb's last feedback, I completely removed the XML
parsing and did it all with structures.  The result should (hopefully) be a lot
easier on the eyes, and is a little more generic.

Signed-off-by: Chris Lalancette <clalance@redhat.com>

ChangeLog

@@ -1,3 +1,8 @@
+Thu Oct 23 13:38:00 CEST 2008 Chris Lalancette <clalance@redhat.com>
+	* src/storage_backend_logical.c src/storage_conf.c
+	src/storage_conf.h: Add <device> tags to the XML returned by
+	virConnectFindPoolSources for storage_backend_logical.
+
 Thu Oct 23 13:31:00 CEST 2008 Chris Lalancette <clalance@redhat.com>
 	* src/storage_backend.h src/storage_backend_disk.c
 	src/storage_backend_fs.c src/storage_backend_logical.c

src/storage_backend_logical.c

@@ -242,33 +242,70 @@ virStorageBackendLogicalRefreshPoolFunc(virConnectPtr conn ATTRIBUTE_UNUSED,
 
 
 static int
-virStorageBackendLogicalFindPoolSourcesFunc(virConnectPtr conn ATTRIBUTE_UNUSED,
+virStorageBackendLogicalFindPoolSourcesFunc(virConnectPtr conn,
                                             virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
                                             char **const groups,
                                             void *data)
 {
-    virStringList **rest = data;
+    virStoragePoolSourceListPtr sourceList = data;
-    virStringList *newItem;
+    char *pvname = NULL;
-    const char *name = groups[0];
+    char *vgname = NULL;
+    int i;
+    virStoragePoolSourceDevicePtr dev;
+    virStoragePoolSource *thisSource;
 
-    /* Append new XML desc to list */
+    pvname = strdup(groups[0]);
+    vgname = strdup(groups[1]);
 
-    if (VIR_ALLOC(newItem) != 0) {
+    if (pvname == NULL || vgname == NULL) {
-        virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("new xml desc"));
+        virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s",
-        return -1;
+                              _("allocating pvname or vgname"));
+        goto err_no_memory;
     }
 
-    if (asprintf(&newItem->val, "<source><name>%s</name></source>", name) <= 0) {
+    thisSource = NULL;
-        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("asprintf failed"));
+    for (i = 0 ; i < sourceList->nsources; i++) {
-        VIR_FREE(newItem);
+        if (STREQ(sourceList->sources[i].name, vgname)) {
-        return -1;
+            thisSource = &sourceList->sources[i];
+            break;
+        }
     }
 
-    newItem->len = strlen(newItem->val);
+    if (thisSource == NULL) {
-    newItem->next = *rest;
+        if (VIR_REALLOC_N(sourceList->sources, sourceList->nsources + 1) != 0) {
-    *rest = newItem;
+            virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s",
+                                  _("allocating new source"));
+            goto err_no_memory;
+        }
+
+        thisSource = &sourceList->sources[sourceList->nsources];
+        sourceList->nsources++;
+
+        memset(thisSource, 0, sizeof(*thisSource));
+        thisSource->name = vgname;
+    }
+    else
+        VIR_FREE(vgname);
+
+    if (VIR_REALLOC_N(thisSource->devices, thisSource->ndevice + 1) != 0) {
+        virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s",
+                              _("allocating new device"));
+        goto err_no_memory;
+    }
+
+    dev = &thisSource->devices[thisSource->ndevice];
+    thisSource->ndevice++;
+
+    memset(dev, 0, sizeof(*dev));
+    dev->path = pvname;
 
     return 0;
+
+ err_no_memory:
+    VIR_FREE(pvname);
+    VIR_FREE(vgname);
+
+    return -1;
 }
 
 static char *
@@ -277,34 +314,40 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn,
                                         unsigned int flags ATTRIBUTE_UNUSED)
 {
     /*
-     * # vgs --noheadings -o vg_name
+     * # pvs --noheadings -o pv_name,vg_name
-     *   VolGroup00
+     *   /dev/sdb
-     *   VolGroup01
+     *   /dev/sdc VolGroup00
      */
     const char *regexes[] = {
-        "^\\s*(\\S+)\\s*$"
+        "^\\s*(\\S+)\\s+(\\S+)\\s*$"
     };
     int vars[] = {
-        1
+        2
     };
-    virStringList *descs = NULL;
+    const char *const prog[] = { PVS, "--noheadings", "-o", "pv_name,vg_name", NULL };
-    const char *prog[] = { VGS, "--noheadings", "-o", "vg_name", NULL };
     int exitstatus;
     char *retval = NULL;
+    virStoragePoolSourceList sourceList;
+    int i;
 
+    memset(&sourceList, 0, sizeof(sourceList));
     if (virStorageBackendRunProgRegex(conn, NULL, prog, 1, regexes, vars,
                                       virStorageBackendLogicalFindPoolSourcesFunc,
-                                      &descs, &exitstatus) < 0)
+                                      &sourceList, &exitstatus) < 0)
         return NULL;
 
-    retval = virStringListJoin(descs, SOURCES_START_TAG, SOURCES_END_TAG, "\n");
+    retval = virStoragePoolSourceListFormat(conn, &sourceList);
     if (retval == NULL) {
-        virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("retval"));
+        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+                              _("failed to get source from sourceList"));
         goto cleanup;
     }
 
  cleanup:
-    virStringListFree(descs);
+    for (i = 0; i < sourceList.nsources; i++)
+        virStoragePoolSourceFree(&sourceList.sources[i]);
+
+    VIR_FREE(sourceList.sources);
 
     return retval;
 }

src/storage_conf.c

@@ -70,26 +70,35 @@ virStorageVolDefFree(virStorageVolDefPtr def) {
 }
 
 void
-virStoragePoolDefFree(virStoragePoolDefPtr def) {
+virStoragePoolSourceFree(virStoragePoolSourcePtr source) {
     int i;
 
+    if (!source)
+        return;
+
+    VIR_FREE(source->host.name);
+    for (i = 0 ; i < source->ndevice ; i++) {
+        VIR_FREE(source->devices[i].freeExtents);
+        VIR_FREE(source->devices[i].path);
+    }
+    VIR_FREE(source->devices);
+    VIR_FREE(source->dir);
+    VIR_FREE(source->name);
+
+    if (source->authType == VIR_STORAGE_POOL_AUTH_CHAP) {
+        VIR_FREE(source->auth.chap.login);
+        VIR_FREE(source->auth.chap.passwd);
+    }
+}
+
+void
+virStoragePoolDefFree(virStoragePoolDefPtr def) {
     if (!def)
         return;
 
     VIR_FREE(def->name);
-    VIR_FREE(def->source.host.name);
-    for (i = 0 ; i < def->source.ndevice ; i++) {
-        VIR_FREE(def->source.devices[i].freeExtents);
-        VIR_FREE(def->source.devices[i].path);
-    }
-    VIR_FREE(def->source.devices);
-    VIR_FREE(def->source.dir);
-    VIR_FREE(def->source.name);
 
-    if (def->source.authType == VIR_STORAGE_POOL_AUTH_CHAP) {
+    virStoragePoolSourceFree(&def->source);
-        VIR_FREE(def->source.auth.chap.login);
-        VIR_FREE(def->source.auth.chap.passwd);
-    }
 
     VIR_FREE(def->target.path);
     VIR_FREE(def->target.perms.label);
@@ -1261,3 +1270,22 @@ virStoragePoolObjDeleteDef(virConnectPtr conn,
 
     return 0;
 }
+
+char *virStoragePoolSourceListFormat(virConnectPtr conn ATTRIBUTE_UNUSED,
+                                     virStoragePoolSourceListPtr def)
+{
+    int i, j;
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+    virBufferAddLit(&buf, "<source>");
+
+    for (i = 0; i < def->nsources; i++) {
+        virBufferVSprintf(&buf, "<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>");
+
+    return virBufferContentAndReset(&buf);
+}

src/storage_conf.h

@@ -249,6 +249,12 @@ struct _virStorageDriverState {
     char *autostartDir;
 };
 
+typedef struct _virStoragePoolSourceList virStoragePoolSourceList;
+typedef virStoragePoolSourceList *virStoragePoolSourceListPtr;
+struct _virStoragePoolSourceList {
+    unsigned int nsources;
+    virStoragePoolSourcePtr sources;
+};
 
 static inline int virStoragePoolObjIsActive(virStoragePoolObjPtr pool) {
     return pool->active;
@@ -303,10 +309,13 @@ int virStoragePoolObjDeleteDef(virConnectPtr conn,
                                virStoragePoolObjPtr pool);
 
 void virStorageVolDefFree(virStorageVolDefPtr def);
+void virStoragePoolSourceFree(virStoragePoolSourcePtr source);
 void virStoragePoolDefFree(virStoragePoolDefPtr def);
 void virStoragePoolObjFree(virStoragePoolObjPtr pool);
 void virStoragePoolObjListFree(virStoragePoolObjListPtr pools);
 void virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
                              virStoragePoolObjPtr pool);
 
-#endif /* __VIR_STORAGE_DRIVER_H__ */
+char *virStoragePoolSourceListFormat(virConnectPtr conn,
+                                     virStoragePoolSourceListPtr def);
+#endif /* __VIR_STORAGE_CONF_H__ */