[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 :|