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