[libvirt] [PATCH 5/9] nodedev: Refactor the helpers
Osier Yang
jyang at redhat.com
Tue Jan 15 01:13:40 UTC 2013
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
More information about the libvir-list
mailing list