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

Re: [libvirt] [PATCH 5/9] nodedev: Refactor the helpers



On 2013年01月15日 03:42, Michal Privoznik wrote:
On 14.01.2013 15:34, Osier Yang wrote:
This adds two util functions (virIsCapableFCHost and virIsCapableVport),
and rename helper check_fc_host_linux as detect_scsi_host_caps,
check_capable_vport_linux is removed, as it's abstracted to the util
function virIsCapableVport. detect_scsi_host_caps nows detect both
the fc_host and vport_ops capabilities. "stat(2)" is replaced with
"access(2)" for saving.

* src/util/virutil.h:
   - Declare virIsCapableFCHost and virIsCapableVport
* src/util/virutil.c:
   - Implement virIsCapableFCHost and virIsCapableVport
* src/node_device/node_device_linux_sysfs.c:
   - Remove check_capable_vport_linux
   - Rename check_fc_host_linux as detect_scsi_host_caps, and refactor
     it a bit to detect both fc_host and vport_os capabilities
* src/node_device/node_device_driver.h:
   - Change/remove the related declarations
* src/node_device/node_device_udev.c: (Use detect_scsi_host_caps)
* src/node_device/node_device_hal.c: (Likewise)
* src/node_device/node_device_driver.c (Likewise)
---
  src/libvirt_private.syms                  |    2 +
  src/node_device/node_device_driver.c      |    7 +-
  src/node_device/node_device_driver.h      |   10 +--
  src/node_device/node_device_hal.c         |    4 +-
  src/node_device/node_device_linux_sysfs.c |  135 ++++++++---------------------
  src/node_device/node_device_udev.c        |    3 +-
  src/util/virutil.c                        |   73 ++++++++++++++++
  src/util/virutil.h                        |    3 +
  8 files changed, 123 insertions(+), 114 deletions(-)


diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c
index 742a0bc..054afea 100644
--- a/src/node_device/node_device_linux_sysfs.c
+++ b/src/node_device/node_device_linux_sysfs.c
@@ -1,8 +1,8 @@
  /*
- * node_device_hal_linuc.c: Linux specific code to gather device data
+ * node_device_linux_sysfs.c: Linux specific code to gather device data
   * not available through HAL.
   *
- * Copyright (C) 2009-2011 Red Hat, Inc.
+ * Copyright (C) 2009-2013 Red Hat, Inc.
   *
   * This library is free software; you can redistribute it and/or
   * modify it under the terms of the GNU Lesser General Public
@@ -37,112 +37,53 @@

  #ifdef __linux__

-int check_fc_host_linux(union _virNodeDevCapData *d)
+int
+detect_scsi_host_caps_linux(union _virNodeDevCapData *d)
  {
-    char *sysfs_path = NULL;
-    int retval = 0;
-    struct stat st;
+    int ret = -1;

      VIR_DEBUG("Checking if host%d is an FC HBA", d->scsi_host.host);

-    if (virAsprintf(&sysfs_path, "%shost%d",
-                    LINUX_SYSFS_FC_HOST_PREFIX,
-                    d->scsi_host.host)<  0) {
-        virReportOOMError();
-        retval = -1;
-        goto out;
+    if (virIsCapableFCHost(NULL, d->scsi_host.host)) {
+        d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST;
+
+        if (virReadFCHost(NULL,
+                          d->scsi_host.host,
+                          "port_name",
+&d->scsi_host.wwpn) == -1) {
+            VIR_ERROR(_("Failed to read WWPN for host%d"), d->scsi_host.host);
+            goto cleanup;
+        }
+
+        if (virReadFCHost(NULL,
+                          d->scsi_host.host,
+                          "node_name",
+&d->scsi_host.wwnn) == -1) {
+            VIR_ERROR(_("Failed to read WWNN for host%d"), d->scsi_host.host);
+            goto cleanup;
+        }
+
+        if (virReadFCHost(NULL,
+                          d->scsi_host.host,
+                          "fabric_name",
+&d->scsi_host.fabric_wwn) == -1) {
+            VIR_ERROR(_("Failed to read fabric WWN for host%d"),
+                      d->scsi_host.host);
+            goto cleanup;
+        }
      }

-    if (stat(sysfs_path,&st) != 0) {
-        /* Not an FC HBA; not an error, either. */
-        goto out;
-    }
-
-    d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST;
-
-    if (virReadFCHost(NULL,
-                      d->scsi_host.host,
-                      "port_name",
-&d->scsi_host.wwpn) == -1) {
-        VIR_ERROR(_("Failed to read WWPN for host%d"),
-                  d->scsi_host.host);
-        retval = -1;
-        goto out;
-    }
-
-    if (virReadFCHost(NULL,
-                      d->scsi_host.host,
-                      "node_name",
-&d->scsi_host.wwnn) == -1) {
-        VIR_ERROR(_("Failed to read WWNN for host%d"),
-                  d->scsi_host.host);
-        retval = -1;
-    }
-
-    if (virReadFCHost(NULL,
-                      d->scsi_host.host,
-                      "fabric_name",
-&d->scsi_host.fabric_wwn) == -1) {
-        VIR_ERROR(_("Failed to read fabric WWN for host%d"),
-                  d->scsi_host.host);
-        retval = -1;
-        goto out;
-    }
+    if (virIsCapableVport(NULL, d->scsi_host.host) == 0)
+        d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS;

-out:
-    if (retval == -1) {
+    ret = 0;
+cleanup:
+    if (ret == -1) {

I'd write this as 'if (ret<  0) {' so we don't have to change this line
if we ever invent a new error code for this function. Moreover, it's
common practice over libvirt code.

Okay, it was inherited from the old codes, I will change it when pushing. Thanks.


ACK if you fix this.

Michal


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