4:18 p.m.
On 09/28/2011 02:08 PM, Serge E. Hallyn wrote:
[ 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 ]
Glad that it passed testing (as I had not tested my suggestions).
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.
I'll tweak the commit a bit (information like this is useful to the
patch review, but less useful in 'git log' - so the best place to stick
it is after the --- divider, so that 'git am' will know that it is not
part of the official commit message).
@@ -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 */
I'll tweak this comment slightly to mention that we are skipping a
prefix, if it is present, and not ignoring the entire command line.
+++ 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) {
I'm not a big fan of in-function #define, especially if it's for a
one-shot string literal. I generally either float the #define up to the
top of the file (out of the function), or in this case, inline the
string constant into its lone usage point (we can refactor into a
#define later, if we need to use the string more than once).
ACK and pushed with this squashed in:
diff --git i/src/storage/storage_backend.c w/src/storage/storage_backend.c
index dd7e36b..64c35c2 100644
--- i/src/storage/storage_backend.c
+++ w/src/storage/storage_backend.c
@@ -1400,7 +1400,7 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr
pool,
const char **regex,
int *nvars,
virStorageBackendListVolRegexFunc func,
- void *data, const char *cmd_to_ignore)
+ void *data, const char *prefix)
{
int fd = -1, err, ret = -1;
FILE *list = NULL;
@@ -1466,9 +1466,9 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr
pool,
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);
+ /* ignore any command prefix */
+ if (prefix)
+ p = STRSKIP(line, prefix);
if (!p)
p = line;
diff --git i/src/storage/storage_backend_logical.c
w/src/storage/storage_backend_logical.c
index 7923b71..589f486 100644
--- i/src/storage/storage_backend_logical.c
+++ w/src/storage/storage_backend_logical.c
@@ -205,14 +205,13 @@
virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
pool->def->source.name, NULL
};
-#define LVS_BASE "lvs"
if (virStorageBackendRunProgRegex(pool,
prog,
1,
regexes,
vars,
virStorageBackendLogicalMakeVol,
- vol, LVS_BASE) < 0) {
+ vol, "lvs") < 0) {
return -1;
}
@@ -328,10 +327,9 @@
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, PVS_BASE) < 0)
+ &sourceList, "pvs") < 0)
return NULL;
retval = virStoragePoolSourceListFormat(&sourceList);
@@ -498,7 +496,6 @@ virStorageBackendLogicalRefreshPool(virConnectPtr
conn ATTRIBUTE_UNUSED,
return -1;
}
-#define VGS_BASE "vgs"
/* Now get basic volgrp metadata */
if (virStorageBackendRunProgRegex(pool,
prog,
@@ -506,7 +503,7 @@ virStorageBackendLogicalRefreshPool(virConnectPtr
conn ATTRIBUTE_UNUSED,
regexes,
vars,
virStorageBackendLogicalRefreshPoolFunc,
- NULL, VGS_BASE) < 0) {
+ NULL, "vgs") < 0) {
virStoragePoolObjClearVols(pool);
return -1;
}
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org