From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; The current virStorageFileGet{LVM,SCSI}Key methods return the key as the return value. Unfortunately it is desirable for "NULL" to be a valid return value, as well as an error indicator. Thus the returned key must instead be provided as an out-parameter. 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. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/util/storage_file.c | 74 ++++++++++++++++++++++++++++++++----------------- src/util/storage_file.h | 6 ++-- 2 files changed, 53 insertions(+), 27 deletions(-) diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 17a47cf..fe7e8b8 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -1192,62 +1192,75 @@ int virStorageFileIsClusterFS(const char *path) } #ifdef LVS -char *virStorageFileGetLVMKey(const char *path) +int virStorageFileGetLVMKey(const char *path, + char **key) { /* * # lvs --noheadings --unbuffered --nosuffix --options "uuid" LVNAME * 06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky */ - char *key = NULL; + int status; virCommandPtr cmd = virCommandNewArgList( LVS, "--noheadings", "--unbuffered", "--nosuffix", "--options", "uuid", path, NULL ); + int ret = -1; + + *key = NULL; /* Run the program and capture its output */ - virCommandSetOutputBuffer(cmd, &key); - if (virCommandRun(cmd, NULL) < 0) + virCommandSetOutputBuffer(cmd, key); + if (virCommandRun(cmd, &status) < 0) goto cleanup; - if (key) { + /* Explicitly check status == 0, rather than passing NULL + * to virCommandRun because we don't want to raise an actual + * error in this scenario, just return a NULL key. + */ + + if (status == 0 && *key) { char *nl; - char *tmp = key; + char *tmp = *key; /* Find first non-space character */ while (*tmp && c_isspace(*tmp)) { tmp++; } /* Kill leading spaces */ - if (tmp != key) - memmove(key, tmp, strlen(tmp)+1); + if (tmp != *key) + memmove(*key, tmp, strlen(tmp)+1); /* Kill trailing newline */ - if ((nl = strchr(key, '\n'))) + if ((nl = strchr(*key, '\n'))) *nl = '\0'; } - if (key && STREQ(key, "")) - VIR_FREE(key); + ret = 0; cleanup: + if (*key && STREQ(*key, "")) + VIR_FREE(*key); + virCommandFree(cmd); - return key; + return ret; } #else -char *virStorageFileGetLVMKey(const char *path) +int virStorageFileGetLVMKey(const char *path, + char **key) { virReportSystemError(ENOSYS, _("Unable to get LVM key for %s"), path); - return NULL; + return -1; } #endif #ifdef HAVE_UDEV -char *virStorageFileGetSCSIKey(const char *path) +int virStorageFileGetSCSIKey(const char *path, + char **key) { - char *key = NULL; + int status; virCommandPtr cmd = virCommandNewArgList( "/lib/udev/scsi_id", "--replace-whitespace", @@ -1255,30 +1268,41 @@ char *virStorageFileGetSCSIKey(const char *path) "--device", path, NULL ); + int ret = -1; + + *key = NULL; /* Run the program and capture its output */ - virCommandSetOutputBuffer(cmd, &key); - if (virCommandRun(cmd, NULL) < 0) + virCommandSetOutputBuffer(cmd, key); + if (virCommandRun(cmd, &status) < 0) goto cleanup; - if (key && STRNEQ(key, "")) { - char *nl = strchr(key, '\n'); + /* Explicitly check status == 0, rather than passing NULL + * to virCommandRun because we don't want to raise an actual + * error in this scenario, just return a NULL key. + */ + if (status == 0 && *key) { + char *nl = strchr(*key, '\n'); if (nl) *nl = '\0'; - } else { - VIR_FREE(key); } + ret = 0; + cleanup: + if (*key && STREQ(*key, "")) + VIR_FREE(*key); + virCommandFree(cmd); - return key; + return ret; } #else -char *virStorageFileGetSCSIKey(const char *path) +int virStorageFileGetSCSIKey(const char *path, + char **key) { virReportSystemError(ENOSYS, _("Unable to get SCSI key for %s"), path); - return NULL; + return -1; } #endif diff --git a/src/util/storage_file.h b/src/util/storage_file.h index 9e4516e..6fbd275 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -101,7 +101,9 @@ int virStorageFileIsClusterFS(const char *path); int virStorageFileIsSharedFSType(const char *path, int fstypes); -char *virStorageFileGetLVMKey(const char *path); -char *virStorageFileGetSCSIKey(const char *path); +int virStorageFileGetLVMKey(const char *path, + char **key); +int virStorageFileGetSCSIKey(const char *path, + char **key); #endif /* __VIR_STORAGE_FILE_H__ */ -- 1.7.11.7