On 09/15/2011 08:17 AM, Serge E. Hallyn wrote: Yeah, ignoring a prefix seems like the right approach, since we can't rely on non-capturing regex extensions. 'make syntax-check' should have called you on this; you should be using STRPREFIX here. Something like (untested): diff --git i/src/storage/storage_backend.c w/src/storage/storage_backend.c index 7a352da..3415918 100644 --- i/src/storage/storage_backend.c +++ w/src/storage/storage_backend.c @@ -1460,16 +1460,17 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, } while (fgets(line, sizeof(line), list) != NULL) { - char *p; + char *p = NULL; /* Strip trailing newline */ int len = strlen(line); if (len && line[len-1] == '\n') line[len-1] = '\0'; - p = line; /* if cmd_to_ignore is specified, then ignore it */ - if (strncmp(line, cmd_to_ignore, strlen(cmd_to_ignore)) == 0) - p += strlen(cmd_to_ignore); + if (cmd_to_ignore) + p = STRSKIP(line, cmd_to_ignore); + if (!p) + p = line; for (i = 0 ; i <= maxReg && i < nregex ; i++) { if (regexec(&reg[i], p, nvars[i]+1, vars, 0) == 0) { -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

[ Thanks for the feedback, Eric. Hopefully I correctly incorporated it in this version. This version still tests fine with and without lvm.conf command_names=1 ] If the regexes supported (?:pvs)?, then we could handle this by optionally matching but not returning the initial command name. But it doesn't. So add a new char* argument to virStorageBackendRunProgRegex(). If that argument is NULL then we act as usual. Otherwise, if the string at that argument is found at the start of a returned line, we drop that before running the regex. With this patch, virt-manager shows me lvs with command_names 1 or 0. The definitions of PVS_BASE etc may want to be moved into the configure scripts (though given how PVS is found, IIUC that could only happen if pvs was a link to pvs_real), but in any case no sense dealing with that until we're sure this is an ok way to handle it. Changelog: Sep 28: Use STRSKIP and make cmd_to_ignore arg const, as per Eric's feedback. Signed-off-by: Serge Hallyn <serge.hallyn(a)canonical.com&gt; --- src/storage/storage_backend.c | 15 +++++++++++---- src/storage/storage_backend.h | 2 +- src/storage/storage_backend_fs.c | 2 +- src/storage/storage_backend_iscsi.c | 4 ++-- src/storage/storage_backend_logical.c | 9 ++++++--- 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index d125504..dd7e36b 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1400,7 +1400,7 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, const char **regex, int *nvars, virStorageBackendListVolRegexFunc func, - void *data) + void *data, const char *cmd_to_ignore) { int fd = -1, err, ret = -1; FILE *list = NULL; @@ -1460,13 +1460,20 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, } while (fgets(line, sizeof(line), list) != NULL) { + char *p = NULL; /* Strip trailing newline */ int len = strlen(line); if (len && line[len-1] == '\n') line[len-1] = '\0'; + /* if cmd_to_ignore is specified, then ignore it */ + if (cmd_to_ignore) + p = STRSKIP(line, cmd_to_ignore); + if (!p) + p = line; + for (i = 0 ; i <= maxReg && i < nregex ; i++) { - if (regexec(&reg[i], line, nvars[i]+1, vars, 0) == 0) { + if (regexec(&reg[i], p, nvars[i]+1, vars, 0) == 0) { maxReg++; if (i == 0) @@ -1475,9 +1482,9 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, /* NULL terminate each captured group in the line */ for (j = 0 ; j < nvars[i] ; j++) { /* NB vars[0] is the full pattern, so we offset j by 1 */ - line[vars[j+1].rm_eo] = '\0'; + p[vars[j+1].rm_eo] = '\0'; if ((groups[ngroup++] = - strdup(line + vars[j+1].rm_so)) == NULL) { + strdup(p + vars[j+1].rm_so)) == NULL) { virReportOOMError(); goto cleanup; } diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 67ac32c..75ed676 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -140,7 +140,7 @@ int virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, const char **regex, int *nvars, virStorageBackendListVolRegexFunc func, - void *data); + void *data, const char *cmd_to_ignore); int virStorageBackendRunProgNul(virStoragePoolObjPtr pool, const char **prog, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index da98f87..d678625 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -266,7 +266,7 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE if (virStorageBackendRunProgRegex(NULL, prog, 1, regexes, vars, virStorageBackendFileSystemNetFindPoolSourcesFunc, - &state) < 0) + &state, NULL) < 0) goto cleanup; retval = virStoragePoolSourceListFormat(&state.list); diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 346e698..99e69c9 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -160,7 +160,7 @@ virStorageBackendISCSISession(virStoragePoolObjPtr pool, regexes, vars, virStorageBackendISCSIExtractSession, - &session) < 0) + &session, NULL) < 0) return NULL; if (session == NULL && @@ -517,7 +517,7 @@ virStorageBackendISCSIScanTargets(const char *portal, regexes, vars, virStorageBackendISCSIGetTargets, - &list) < 0) { + &list, NULL) < 0) { return -1; } diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 23d80cb..7923b71 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -205,13 +205,14 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, pool->def->source.name, NULL }; +#define LVS_BASE "lvs" if (virStorageBackendRunProgRegex(pool, prog, 1, regexes, vars, virStorageBackendLogicalMakeVol, - vol) < 0) { + vol, LVS_BASE) < 0) { return -1; } @@ -327,9 +328,10 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, memset(&sourceList, 0, sizeof(sourceList)); sourceList.type = VIR_STORAGE_POOL_LOGICAL; +#define PVS_BASE "pvs" if (virStorageBackendRunProgRegex(NULL, prog, 1, regexes, vars, virStorageBackendLogicalFindPoolSourcesFunc, - &sourceList) < 0) + &sourceList, PVS_BASE) < 0) return NULL; retval = virStoragePoolSourceListFormat(&sourceList); @@ -496,6 +498,7 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } +#define VGS_BASE "vgs" /* Now get basic volgrp metadata */ if (virStorageBackendRunProgRegex(pool, prog, @@ -503,7 +506,7 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, regexes, vars, virStorageBackendLogicalRefreshPoolFunc, - NULL) < 0) { + NULL, VGS_BASE) < 0) { virStoragePoolObjClearVols(pool); return -1; } -- 1.7.5.4