[libvirt] [PATCH 0/1] Make NPIV support work with older kernels
Here is a patch that makes NPIV support work with older kernels that have vport_create and delete in /sys/class/scsi_host/hostN instead of /sys/class/fc_host/hostN It doesn't look to me like there is a single place to find those files that works with both old and new kernels, so the new code checks both places at runtime. Dave
* src/node_device_hal_linux.c, src/node_device.c: Older kernels had vport_create and delete in /sys/class/scsi_host not /sys/class/fc_host. This patch causes libvirt to look in both places. --- src/node_device.c | 23 +++++- src/node_device.h | 2 +- src/node_device_hal_linux.c | 186 +++++++++++++++++++++++++----------------- 3 files changed, 134 insertions(+), 77 deletions(-) diff --git a/src/node_device.c b/src/node_device.c index d01695d..4a936de 100644 --- a/src/node_device.c +++ b/src/node_device.c @@ -369,6 +369,7 @@ nodeDeviceVportCreateDelete(virConnectPtr conn, int operation) { int retval = 0; + struct stat st; char *operation_path = NULL, *vport_name = NULL; const char *operation_file = NULL; @@ -388,7 +389,7 @@ nodeDeviceVportCreateDelete(virConnectPtr conn, } if (virAsprintf(&operation_path, - "%shost%d%s", + "%s/host%d%s", LINUX_SYSFS_FC_HOST_PREFIX, parent_host, operation_file) < 0) { @@ -398,6 +399,26 @@ nodeDeviceVportCreateDelete(virConnectPtr conn, goto cleanup; } + if (stat(operation_path, &st) != 0) { + VIR_FREE(operation_path); + if (virAsprintf(&operation_path, + "%s/host%d%s", + LINUX_SYSFS_SCSI_HOST_PREFIX, + parent_host, + operation_file) < 0) { + + virReportOOMError(conn); + retval = -1; + goto cleanup; + } + } + + if (stat(operation_path, &st) != 0) { + VIR_ERROR(_("No vport operation path found for host%d"), parent_host); + retval = -1; + goto cleanup; + } + VIR_DEBUG(_("Vport operation path is '%s'"), operation_path); if (virAsprintf(&vport_name, diff --git a/src/node_device.h b/src/node_device.h index db01624..e745eb4 100644 --- a/src/node_device.h +++ b/src/node_device.h @@ -30,7 +30,7 @@ #define LINUX_SYSFS_SCSI_HOST_PREFIX "/sys/class/scsi_host" #define LINUX_SYSFS_SCSI_HOST_POSTFIX "device" -#define LINUX_SYSFS_FC_HOST_PREFIX "/sys/class/fc_host/" +#define LINUX_SYSFS_FC_HOST_PREFIX "/sys/class/fc_host" #define VPORT_CREATE 0 #define VPORT_DELETE 1 diff --git a/src/node_device_hal_linux.c b/src/node_device_hal_linux.c index b76235d..b669a3a 100644 --- a/src/node_device_hal_linux.c +++ b/src/node_device_hal_linux.c @@ -34,58 +34,82 @@ #ifdef __linux__ -int check_fc_host_linux(union _virNodeDevCapData *d) + +static int fc_file_exists(const char *prefix, + int host, + const char *file) { + int retval = 0; char *sysfs_path = NULL; - char *wwnn_path = NULL; - char *wwpn_path = NULL; - char *p = NULL; - int fd = -1, retval = 0; - char buf[64]; struct stat st; - VIR_DEBUG(_("Checking if host%d is an FC HBA"), d->scsi_host.host); - - if (virAsprintf(&sysfs_path, "%s/host%d", - LINUX_SYSFS_FC_HOST_PREFIX, - d->scsi_host.host) < 0) { + if (virAsprintf(&sysfs_path, "%s/host%d%s", prefix, host, file) < 0) { virReportOOMError(NULL); retval = -1; goto out; } - if (stat(sysfs_path, &st) != 0) { - /* Not an FC HBA; not an error, either. */ - goto out; + if (stat(sysfs_path, &st) == 0) { + + retval = 1; + VIR_ERROR(_("'%s' exists"), sysfs_path); + + } else { + VIR_ERROR(_("'%s' does not exist"), sysfs_path); } - d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; +out: + VIR_FREE(sysfs_path); + return retval; +} - if (virAsprintf(&wwnn_path, "%s/node_name", - sysfs_path) < 0) { + +static int open_wwn_file(const char *prefix, + int host, + const char *file, + int *fd) +{ + int retval = 0; + char *wwn_path = NULL; + + if (virAsprintf(&wwn_path, "%s/host%d/%s", prefix, host, file) < 0) { virReportOOMError(NULL); retval = -1; goto out; } - if ((fd = open(wwnn_path, O_RDONLY)) < 0) { - retval = -1; - VIR_ERROR(_("Failed to open WWNN path '%s' for reading"), - wwnn_path); - goto out; + if ((*fd = open(wwn_path, O_RDONLY)) != -1) { + VIR_ERROR(_("Opened WWN path '%s' for reading"), + wwn_path); + } else { + VIR_ERROR(_("Failed to open WWN path '%s' for reading"), + wwn_path); + } + +out: + VIR_FREE(wwn_path); + return retval; +} + + +static int get_wwn(int host, const char *file, char **wwn) +{ + char *p = NULL; + int fd = -1, retval = 0; + char buf[64]; + + if (open_wwn_file(LINUX_SYSFS_FC_HOST_PREFIX, host, file, &fd) < 0) { + goto out; } memset(buf, 0, sizeof(buf)); if (saferead(fd, buf, sizeof(buf)) < 0) { retval = -1; - VIR_ERROR(_("Failed to read WWNN from '%s'"), - wwnn_path); + VIR_DEBUG(_("Failed to read WWN for host%d '%s'"), + host, file); goto out; } - close(fd); - fd = -1; - p = strstr(buf, "0x"); if (p != NULL) { p += strlen("0x"); @@ -93,97 +117,109 @@ int check_fc_host_linux(union _virNodeDevCapData *d) p = buf; } - d->scsi_host.wwnn = strndup(p, sizeof(buf)); - if (d->scsi_host.wwnn == NULL) { + *wwn = strndup(p, sizeof(buf)); + if (*wwn == NULL) { virReportOOMError(NULL); retval = -1; goto out; } - p = strchr(d->scsi_host.wwnn, '\n'); + p = strchr(*wwn, '\n'); if (p != NULL) { *p = '\0'; } - if (virAsprintf(&wwpn_path, "%s/port_name", - sysfs_path) < 0) { - virReportOOMError(NULL); - retval = -1; - goto out; +out: + if (fd != -1) { + close(fd); } + return retval; +} - if ((fd = open(wwpn_path, O_RDONLY)) < 0) { + +int check_fc_host_linux(union _virNodeDevCapData *d) +{ + int fc_host = 0; + int retval = 0; + + VIR_DEBUG(_("Checking if host%d is an FC HBA"), d->scsi_host.host); + + fc_host = fc_file_exists(LINUX_SYSFS_FC_HOST_PREFIX, + d->scsi_host.host, ""); + + if (fc_host == -1) { retval = -1; - VIR_ERROR(_("Failed to open WWPN path '%s' for reading"), - wwpn_path); goto out; } - memset(buf, 0, sizeof(buf)); - if (saferead(fd, buf, sizeof(buf)) < 0) { - retval = -1; - VIR_ERROR(_("Failed to read WWPN from '%s'"), - wwpn_path); + if (fc_host == 0) { + /* Not an FC HBA; not an error, either. */ goto out; } - close(fd); - fd = -1; + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; - p = strstr(buf, "0x"); - if (p != NULL) { - p += strlen("0x"); - } else { - p = buf; - } + retval = get_wwn(d->scsi_host.host, + "node_name", + &d->scsi_host.wwnn); - d->scsi_host.wwpn = strndup(p, sizeof(buf)); - if (d->scsi_host.wwpn == NULL) { - virReportOOMError(NULL); - retval = -1; + if (retval == -1) { goto out; } - p = strchr(d->scsi_host.wwpn, '\n'); - if (p != NULL) { - *p = '\0'; + VIR_DEBUG(_("WWNN: '%s'"), d->scsi_host.wwnn); + + retval = get_wwn(d->scsi_host.host, + "port_name", + &d->scsi_host.wwpn); + + if (retval == -1) { + goto out; } + VIR_DEBUG(_("WWPN: '%s'"), d->scsi_host.wwpn); + out: - if (fd != -1) { - close(fd); - } - VIR_FREE(sysfs_path); - VIR_FREE(wwnn_path); - VIR_FREE(wwpn_path); return 0; } int check_vport_capable_linux(union _virNodeDevCapData *d) { - char *sysfs_path = NULL; - struct stat st; - int retval = 0; + int retval = 0, file_exists = 0; - if (virAsprintf(&sysfs_path, "%s/host%d/vport_create", - LINUX_SYSFS_FC_HOST_PREFIX, - d->scsi_host.host) < 0) { - virReportOOMError(NULL); + VIR_DEBUG(_("Checking if host%d is vport capable"), d->scsi_host.host); + + file_exists = fc_file_exists(LINUX_SYSFS_FC_HOST_PREFIX, + d->scsi_host.host, + LINUX_SYSFS_VPORT_CREATE_POSTFIX); + + if (file_exists == -1) { retval = -1; goto out; } - if (stat(sysfs_path, &st) != 0) { - /* Not a vport capable HBA; not an error, either. */ + if (file_exists == 1) { + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS; goto out; } - d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS; + file_exists = fc_file_exists(LINUX_SYSFS_SCSI_HOST_PREFIX, + d->scsi_host.host, + LINUX_SYSFS_VPORT_CREATE_POSTFIX); + + if (file_exists == -1) { + retval = -1; + goto out; + } + + if (file_exists == 1) { + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS; + } out: - VIR_FREE(sysfs_path); return retval; } #endif /* __linux__ */ + -- 1.6.0.6
On Thu, Jun 18, 2009 at 09:40:21PM -0400, David Allan wrote:
* src/node_device_hal_linux.c, src/node_device.c: Older kernels had vport_create and delete in /sys/class/scsi_host not /sys/class/fc_host. This patch causes libvirt to look in both places.
Important patch as it allows NPIV support to work both in Fedora and older RHEL kernels.
diff --git a/src/node_device.c b/src/node_device.c index d01695d..4a936de 100644 --- a/src/node_device.c +++ b/src/node_device.c
--- a/src/node_device.h +++ b/src/node_device.h @@ -30,7 +30,7 @@
Okay that part of the patch looks just fine.
diff --git a/src/node_device_hal_linux.c b/src/node_device_hal_linux.c index b76235d..b669a3a 100644 --- a/src/node_device_hal_linux.c +++ b/src/node_device_hal_linux.c @@ -34,58 +34,82 @@
#ifdef __linux__
-int check_fc_host_linux(union _virNodeDevCapData *d) + +static int fc_file_exists(const char *prefix, + int host, + const char *file)
But diff makes an horrible mess here ! After looking at the file changes, I get that get_wwn() disapears, it's inlined in check_fc_host_linux() twice and diff gets lost :-) I'm not 100% sure why node_device_hal_linux.c needs such a treatment since the test for both paths actually occurs in src/node_device.c nodeDeviceVportCreateDelete() but the resulting code is actually smaller, Looks fine to me though, ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Fri, Jun 19, 2009 at 10:07:36AM +0200, Daniel Veillard wrote:
On Thu, Jun 18, 2009 at 09:40:21PM -0400, David Allan wrote:
* src/node_device_hal_linux.c, src/node_device.c: Older kernels had vport_create and delete in /sys/class/scsi_host not /sys/class/fc_host. This patch causes libvirt to look in both places.
Important patch as it allows NPIV support to work both in Fedora and older RHEL kernels.
diff --git a/src/node_device.c b/src/node_device.c index d01695d..4a936de 100644 --- a/src/node_device.c +++ b/src/node_device.c
--- a/src/node_device.h +++ b/src/node_device.h @@ -30,7 +30,7 @@
Okay that part of the patch looks just fine.
diff --git a/src/node_device_hal_linux.c b/src/node_device_hal_linux.c index b76235d..b669a3a 100644 --- a/src/node_device_hal_linux.c +++ b/src/node_device_hal_linux.c @@ -34,58 +34,82 @@
#ifdef __linux__
-int check_fc_host_linux(union _virNodeDevCapData *d) + +static int fc_file_exists(const char *prefix, + int host, + const char *file)
But diff makes an horrible mess here !
After looking at the file changes, I get that get_wwn() disapears, it's inlined in check_fc_host_linux() twice and diff gets lost :-) I'm not 100% sure why node_device_hal_linux.c needs such a treatment since the test for both paths actually occurs in src/node_device.c nodeDeviceVportCreateDelete() but the resulting code is actually smaller,
I had pretty much the same thought. The first part of the patch looks good relating to the initial subject. The huge diff following that doesn't seem related to the moving of vport file in sysfs ? Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Daniel P. Berrange wrote:
On Fri, Jun 19, 2009 at 10:07:36AM +0200, Daniel Veillard wrote:
* src/node_device_hal_linux.c, src/node_device.c: Older kernels had vport_create and delete in /sys/class/scsi_host not /sys/class/fc_host. This patch causes libvirt to look in both places. Important patch as it allows NPIV support to work both in Fedora and
On Thu, Jun 18, 2009 at 09:40:21PM -0400, David Allan wrote: older RHEL kernels.
diff --git a/src/node_device.c b/src/node_device.c index d01695d..4a936de 100644 --- a/src/node_device.c +++ b/src/node_device.c --- a/src/node_device.h +++ b/src/node_device.h @@ -30,7 +30,7 @@ Okay that part of the patch looks just fine.
diff --git a/src/node_device_hal_linux.c b/src/node_device_hal_linux.c index b76235d..b669a3a 100644 --- a/src/node_device_hal_linux.c +++ b/src/node_device_hal_linux.c @@ -34,58 +34,82 @@
#ifdef __linux__
-int check_fc_host_linux(union _virNodeDevCapData *d) + +static int fc_file_exists(const char *prefix, + int host, + const char *file) But diff makes an horrible mess here !
After looking at the file changes, I get that get_wwn() disapears, it's inlined in check_fc_host_linux() twice and diff gets lost :-) I'm not 100% sure why node_device_hal_linux.c needs such a treatment since the test for both paths actually occurs in src/node_device.c nodeDeviceVportCreateDelete() but the resulting code is actually smaller,
I had pretty much the same thought. The first part of the patch looks good relating to the initial subject. The huge diff following that doesn't seem related to the moving of vport file in sysfs ?
Sorry about that--one of the people who was hitting the problem was reporting that the WWNs were not being detected, so I modified that part of the code to make it easy to look in multiple places for those files as well. Since those files have not moved, at least on my RHEL system, those changes are probably irrelevant to this fix, and I will split this patch into two, a small one for the moved vport ops files and one for the other changes which also remove some duplicated code. Dave
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Dave Allan -
David Allan