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

[Michal Privoznik]

On 11.12.2012 21:41, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> 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;
> 

I agree with Peter, I think you must return an error to be consistent
with !HAVE_UDEV version of these functions.