On 11.12.2012 21:41, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> 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.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list