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.
ACK if you fix this.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list