Re: [PATCH v2 2/4] storage: Rework virStorageBackendSCSISerial

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 1/29/19 10:14 AM, Ján Tomko wrote:
> On Fri, Jan 18, 2019 at 09:42:35AM -0500, John Ferlan wrote:
>> Alter the code to use the virStorageFileGetSCSIKey helper
>> to fetch the unique key for the SCSI disk. Alter the logic
>> to follow the former code which would return a duplicate
>> of @dev when either the virCommandRun succeeded, but returned
>> an empty string or when WITH_UDEV was not true.
>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/storage/storage_util.c | 34 ++++++++--------------------------
>> 1 file changed, 8 insertions(+), 26 deletions(-)
>>
>> diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
>> index a84ee5b600..aa1af434de 100644
>> --- a/src/storage/storage_util.c
>> +++ b/src/storage/storage_util.c
>> @@ -3758,36 +3758,18 @@
>> virStorageBackendRefreshLocal(virStoragePoolObjPtr pool)
>> static char *
>> virStorageBackendSCSISerial(const char *dev)
>> {
>> +    int rc;
>>     char *serial = NULL;
>> -#ifdef WITH_UDEV
>> -    virCommandPtr cmd = virCommandNewArgList(
>> -        "/lib/udev/scsi_id",
>> -        "--replace-whitespace",
>> -        "--whitelisted",
>> -        "--device", dev,
>> -        NULL
>> -        );
>> -
>> -    /* Run the program and capture its output */
>> -    virCommandSetOutputBuffer(cmd, &serial);
>> -    if (virCommandRun(cmd, NULL) < 0)
>> -        goto cleanup;
>> -#endif
>>
>> -    if (serial && STRNEQ(serial, "")) {
>> -        char *nl = strchr(serial, '\n');
>> -        if (nl)
>> -            *nl = '\0';
>> -    } else {
>> -        VIR_FREE(serial);
>> -        ignore_value(VIR_STRDUP(serial, dev));
>> -    }
>> +    rc = virStorageFileGetSCSIKey(dev, &serial);
>> +    if (rc == 0 && serial)
>> +        return serial;
>>
>> -#ifdef WITH_UDEV
>> - cleanup:
>> -    virCommandFree(cmd);
>> -#endif
>> +    if (rc == -2)
>> +        return NULL;
>>
>> +    virResetLastError();
> 
> Every virReportError call logs the error into the configured log outputs
> and sets the thread-local error object.
> 
> This only resets the error object, there's no way to unlog the error.
> If it's expected operation, we should not log an error in the first
> place.
> 
> Jano
> 

So does the attached resolve the concern/issue you have?

Tks,

John

>From 6f9385248a9e78cc3619d02e4adf3205cbad874d Mon Sep 17 00:00:00 2001
From: John Ferlan <jferlan@xxxxxxxxxx>
Date: Tue, 29 Jan 2019 16:44:01 -0500
Subject: [PATCH] Squash with previous

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
 src/locking/lock_driver_lockd.c |  2 +-
 src/storage/storage_util.c      |  3 +--
 src/util/virstoragefile.c       | 10 +++++++---
 src/util/virstoragefile.h       |  3 ++-
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 16fce551c3..4ffa92fc9b 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -531,7 +531,7 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
         if (STRPREFIX(name, "/dev") &&
             driver->scsiLockSpaceDir) {
             VIR_DEBUG("Trying to find an SCSI ID for %s", name);
-            if (virStorageFileGetSCSIKey(name, &newName) < 0)
+            if (virStorageFileGetSCSIKey(name, &newName, false) < 0)
                 goto cleanup;
 
             if (newName) {
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index d8fd76f6b6..85f1cbb57d 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -3782,14 +3782,13 @@ virStorageBackendSCSISerial(const char *dev)
     int rc;
     char *serial = NULL;
 
-    rc = virStorageFileGetSCSIKey(dev, &serial);
+    rc = virStorageFileGetSCSIKey(dev, &serial, true);
     if (rc == 0 && serial)
         return serial;
 
     if (rc == -2)
         return NULL;
 
-    virResetLastError();
     ignore_value(VIR_STRDUP(serial, dev));
     return serial;
 }
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 2a38a08241..39c8511bef 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1429,6 +1429,7 @@ int virStorageFileGetLVMKey(const char *path,
 /* virStorageFileGetSCSIKey
  * @path: Path to the SCSI device
  * @key: Unique key to be returned
+ * @ignoreError: Used to not report ENOSYS
  *
  * Using a udev specific function, query the @path to get and return a
  * unique @key for the caller to use.
@@ -1441,7 +1442,8 @@ int virStorageFileGetLVMKey(const char *path,
  */
 int
 virStorageFileGetSCSIKey(const char *path,
-                         char **key)
+                         char **key,
+                         bool ignoreError ATTRIBUTE_UNUSED)
 {
     int status;
     virCommandPtr cmd = virCommandNewArgList("/lib/udev/scsi_id",
@@ -1481,9 +1483,11 @@ virStorageFileGetSCSIKey(const char *path,
 }
 #else
 int virStorageFileGetSCSIKey(const char *path,
-                             char **key ATTRIBUTE_UNUSED)
+                             char **key ATTRIBUTE_UNUSED,
+                             bool ignoreError)
 {
-    virReportSystemError(ENOSYS, _("Unable to get SCSI key for %s"), path);
+    if (!ignoreError)
+        virReportSystemError(ENOSYS, _("Unable to get SCSI key for %s"), path);
     return -1;
 }
 #endif
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 1d6161a2c7..03837e9e58 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -390,7 +390,8 @@ bool virStorageIsRelative(const char *backing);
 int virStorageFileGetLVMKey(const char *path,
                             char **key);
 int virStorageFileGetSCSIKey(const char *path,
-                             char **key);
+                             char **key,
+                             bool ignoreError);
 
 void virStorageAuthDefFree(virStorageAuthDefPtr def);
 virStorageAuthDefPtr virStorageAuthDefCopy(const virStorageAuthDef *src);
-- 
2.20.1

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux