Re: [libvirt] [PATCH 01/14] Avoid polluting logs when querying LVM/SCSI ID

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; When we invoke lvs or scsi_id to extract ID for block devices, we don't want virCommandWait logging errors messages. Thus we must explicitly check 'status != 0', rather than letting virCommandWait do it. Also move the check for converting from "" to NULL, after the cleanup label, since virCommandRun may set the key to "", even on failure --- src/util/storage_file.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 4417404..776d4f2 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -1199,6 +1199,7 @@ const char *virStorageFileGetLVMKey(const char *path) * 06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky */ char *key = NULL; + int status; virCommandPtr cmd = virCommandNewArgList( LVS, "--noheadings", "--unbuffered", "--nosuffix", @@ -1208,7 +1209,13 @@ const char *virStorageFileGetLVMKey(const char *path) /* Run the program and capture its output */ virCommandSetOutputBuffer(cmd, &key); - if (virCommandRun(cmd, NULL) < 0) + if (virCommandRun(cmd, &status) < 0) + goto cleanup; + + /* Explicitly check, rather than passing NULL to virCommandRun + * because we don't want to pollute logs with an error message + */ + if (status != 0)

goto cleanup; if (key) { @@ -1228,10 +1235,10 @@ const char *virStorageFileGetLVMKey(const char *path) *nl = '\0'; } +cleanup: if (key && STREQ(key, "")) VIR_FREE(key); -cleanup: virCommandFree(cmd); return key; @@ -1248,6 +1255,7 @@ const char *virStorageFileGetLVMKey(const char *path) const char *virStorageFileGetSCSIKey(const char *path) { char *key = NULL; + int status; virCommandPtr cmd = virCommandNewArgList( "/lib/udev/scsi_id", "--replace-whitespace", @@ -1258,9 +1266,16 @@ const char *virStorageFileGetSCSIKey(const char *path) /* Run the program and capture its output */ virCommandSetOutputBuffer(cmd, &key); - if (virCommandRun(cmd, NULL) < 0) + if (virCommandRun(cmd, &status) < 0) goto cleanup; + /* Explicitly check, rather than passing NULL to virCommandRun + * because we don't want to pollute logs with an error message + */ + if (status != 0)

+ goto cleanup; + +cleanup: if (key && STRNEQ(key, "")) { char *nl = strchr(key, '\n'); if (nl) @@ -1269,7 +1284,6 @@ const char *virStorageFileGetSCSIKey(const char *path) VIR_FREE(key); } -cleanup: virCommandFree(cmd); return key;