When HBA kernel modules are loaded while libvirt is running, libvirt will often read bogus WWNs from sysfs as at least some modules trigger the HAL event notifying libvirt of adapters' existence before the sysfs info is correct. The following patch causes libvirt to reread the WWNs whenever dumpxml is called so the capability information about the WWNs will be up to date. I pulled the logic to read the WWNs into two separate functions, and the patch is slightly more invasive as a result. The resulting code is smaller and cleaner for it, though, so I think this is the right way to go. (The diff is also very ugly; it should make sense once applied.) Dave
--- src/node_device.c | 34 +++++++++++ src/node_device_hal.h | 4 + src/node_device_hal_linux.c | 134 +++++++++++++++++++++---------------------- 3 files changed, 104 insertions(+), 68 deletions(-) diff --git a/src/node_device.c b/src/node_device.c index 8de6495..cda14fa 100644 --- a/src/node_device.c +++ b/src/node_device.c @@ -33,6 +33,7 @@ #include "memory.h" #include "logging.h" #include "node_device_conf.h" +#include "node_device_hal.h" #include "node_device.h" #include "storage_backend.h" /* For virWaitForDevices */ @@ -49,6 +50,37 @@ static int dev_has_cap(const virNodeDeviceObjPtr dev, const char *cap) return 0; } + +static int update_caps(virNodeDeviceObjPtr dev) +{ + virNodeDevCapsDefPtr cap = dev->def->caps; + + while (cap) { + /* The only cap that currently needs updating is the WWN of FC HBAs. */ + if (cap->type == VIR_NODE_DEV_CAP_SCSI_HOST) { + if (cap->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { + if (read_wwn(cap->data.scsi_host.host, + "port_name", + &cap->data.scsi_host.wwpn) == -1) { + VIR_ERROR(_("Failed to refresh WWPN for host%d"), + cap->data.scsi_host.host); + } + + if (read_wwn(cap->data.scsi_host.host, + "node_name", + &cap->data.scsi_host.wwnn) == -1) { + VIR_ERROR(_("Failed to refresh WWNN for host%d"), + cap->data.scsi_host.host); + } + } + cap = cap->next; + } + } + + return 0; +} + + #ifdef __linux__ static int update_driver_name(virConnectPtr conn, virNodeDeviceObjPtr dev) @@ -253,6 +285,8 @@ static char *nodeDeviceDumpXML(virNodeDevicePtr dev, } update_driver_name(dev->conn, obj); + update_caps(obj); + ret = virNodeDeviceDefFormat(dev->conn, obj->def); cleanup: diff --git a/src/node_device_hal.h b/src/node_device_hal.h index 0b4a2ef..c859fe3 100644 --- a/src/node_device_hal.h +++ b/src/node_device_hal.h @@ -30,10 +30,14 @@ int check_fc_host_linux(union _virNodeDevCapData *d); #define check_vport_capable(d) check_vport_capable_linux(d) int check_vport_capable_linux(union _virNodeDevCapData *d); +#define read_wwn(host, file, wwn) read_wwn_linux(host, file, wwn) +int read_wwn_linux(int host, const char *file, char **wwn); + #else /* __linux__ */ #define check_fc_host(d) #define check_vport_capable(d) +#define read_wwn(host, file, wwn) #endif /* __linux__ */ diff --git a/src/node_device_hal_linux.c b/src/node_device_hal_linux.c index 2de9afe..96b2ecd 100644 --- a/src/node_device_hal_linux.c +++ b/src/node_device_hal_linux.c @@ -34,58 +34,53 @@ #ifdef __linux__ -int check_fc_host_linux(union _virNodeDevCapData *d) +static int open_wwn_file(const char *prefix, + int host, + const char *file, + int *fd) { - 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); + int retval = 0; + char *wwn_path = NULL; - if (virAsprintf(&sysfs_path, "%s/host%d", - LINUX_SYSFS_FC_HOST_PREFIX, - d->scsi_host.host) < 0) { + if (virAsprintf(&wwn_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; + /* fd will be closed by caller */ + 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); } - d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; +out: + VIR_FREE(wwn_path); + return retval; +} - if (virAsprintf(&wwnn_path, "%s/node_name", - sysfs_path) < 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; +int read_wwn_linux(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,69 +88,72 @@ 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; +} + + +int check_fc_host_linux(union _virNodeDevCapData *d) +{ + char *sysfs_path = NULL; + int retval = 0; + struct stat st; - if ((fd = open(wwpn_path, O_RDONLY)) < 0) { + 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) { + virReportOOMError(NULL); 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 (stat(sysfs_path, &st) != 0) { + /* Not an FC HBA; not an error, either. */ goto out; } - close(fd); - fd = -1; - - p = strstr(buf, "0x"); - if (p != NULL) { - p += strlen("0x"); - } else { - p = buf; - } + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST; - d->scsi_host.wwpn = strndup(p, sizeof(buf)); - if (d->scsi_host.wwpn == NULL) { - virReportOOMError(NULL); + if (read_wwn(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; } - p = strchr(d->scsi_host.wwpn, '\n'); - if (p != NULL) { - *p = '\0'; + if (read_wwn(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; } out: - if (fd != -1) { - close(fd); + if (retval == -1) { + VIR_FREE(d->scsi_host.wwnn); + VIR_FREE(d->scsi_host.wwpn); } VIR_FREE(sysfs_path); - VIR_FREE(wwnn_path); - VIR_FREE(wwpn_path); return 0; } -- 1.6.0.6
On Thu, Jun 25, 2009 at 11:20:24AM -0400, David Allan wrote:
--- src/node_device.c | 34 +++++++++++ src/node_device_hal.h | 4 + src/node_device_hal_linux.c | 134 +++++++++++++++++++++---------------------- 3 files changed, 104 insertions(+), 68 deletions(-)
ACK 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 :|
On Fri, Jun 26, 2009 at 10:55:16AM +0100, Daniel P. Berrange wrote:
On Thu, Jun 25, 2009 at 11:20:24AM -0400, David Allan wrote:
--- src/node_device.c | 34 +++++++++++ src/node_device_hal.h | 4 + src/node_device_hal_linux.c | 134 +++++++++++++++++++++---------------------- 3 files changed, 104 insertions(+), 68 deletions(-)
ACK
Okidoc, applied and commited, 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/
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
David Allan