From: Daniel Berrange <berrange@redhat.com> NodeDeviceCreateXML and NodeDeviceDestroy methods added for NPIV were using the wrong privateData field for the remote driver. This doesn't impact KVM, since the remote driver handles everything, thus privateData == devMonPrivateData. It does impact Xen though, because the remote driver only handles a subset of methods and thus privateData != devMonPrivateData. --- src/remote/remote_driver.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index cb0d8e1..a945710 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6164,6 +6164,8 @@ remoteNodeDeviceDettach (virNodeDevicePtr dev) { int rv = -1; remote_node_device_dettach_args args; + /* This method is unusual in that it uses the HV driver, not the devMon driver + * hence its use of privateData, instead of devMonPrivateData */ struct private_data *priv = dev->conn->privateData; remoteDriverLock(priv); @@ -6187,6 +6189,8 @@ remoteNodeDeviceReAttach (virNodeDevicePtr dev) { int rv = -1; remote_node_device_re_attach_args args; + /* This method is unusual in that it uses the HV driver, not the devMon driver + * hence its use of privateData, instead of devMonPrivateData */ struct private_data *priv = dev->conn->privateData; remoteDriverLock(priv); @@ -6210,6 +6214,8 @@ remoteNodeDeviceReset (virNodeDevicePtr dev) { int rv = -1; remote_node_device_reset_args args; + /* This method is unusual in that it uses the HV driver, not the devMon driver + * hence its use of privateData, instead of devMonPrivateData */ struct private_data *priv = dev->conn->privateData; remoteDriverLock(priv); @@ -6237,7 +6243,7 @@ remoteNodeDeviceCreateXML(virConnectPtr conn, remote_node_device_create_xml_args args; remote_node_device_create_xml_ret ret; virNodeDevicePtr dev = NULL; - struct private_data *priv = conn->privateData; + struct private_data *priv = conn->devMonPrivateData; remoteDriverLock(priv); @@ -6263,7 +6269,7 @@ remoteNodeDeviceDestroy(virNodeDevicePtr dev) { int rv = -1; remote_node_device_destroy_args args; - struct private_data *priv = dev->conn->privateData; + struct private_data *priv = dev->conn->devMonPrivateData; remoteDriverLock(priv); -- 1.7.2

From: Dave Allan <dallan@redhat.com> Some kernels, such as the one used in RHEL-5, have vport_create and vport_delete operation files in /sys/class/scsi_host/hostN directory instead of /sys/class/fc_host/hostN. Let's check both paths for compatibility reasons. This also removes unnecessary '/' characters from sysfs paths containing LINUX_SYSFS_FC_HOST_PREFIX. --- src/node_device/node_device_driver.c | 21 +++++++++++++++++++ src/node_device/node_device_driver.h | 2 +- src/node_device/node_device_linux_sysfs.c | 31 +++++++++++++++++++++++----- 3 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index b0ff662..a6ac80b 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -392,6 +392,7 @@ nodeDeviceVportCreateDelete(const int parent_host, int retval = 0; char *operation_path = NULL, *vport_name = NULL; const char *operation_file = NULL; + struct stat st; switch (operation) { case VPORT_CREATE: @@ -419,6 +420,26 @@ nodeDeviceVportCreateDelete(const int parent_host, goto cleanup; } + if (stat(operation_path, &st) != 0) { + VIR_FREE(operation_path); + if (virAsprintf(&operation_path, + "%shost%d%s", + LINUX_SYSFS_SCSI_HOST_PREFIX, + parent_host, + operation_file) < 0) { + virReportOOMError(); + 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/node_device_driver.h b/src/node_device/node_device_driver.h index f233641..4721be4 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -28,7 +28,7 @@ # include "driver.h" # include "node_device_conf.h" -# define LINUX_SYSFS_SCSI_HOST_PREFIX "/sys/class/scsi_host" +# 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/" diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index c90e72b..7f09cc7 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -119,7 +119,7 @@ int check_fc_host_linux(union _virNodeDevCapData *d) VIR_DEBUG("Checking if host%d is an FC HBA", d->scsi_host.host); - if (virAsprintf(&sysfs_path, "%s/host%d", + if (virAsprintf(&sysfs_path, "%shost%d", LINUX_SYSFS_FC_HOST_PREFIX, d->scsi_host.host) < 0) { virReportOOMError(); @@ -167,20 +167,39 @@ int check_vport_capable_linux(union _virNodeDevCapData *d) struct stat st; int retval = 0; - if (virAsprintf(&sysfs_path, "%s/host%d/vport_create", + if (virAsprintf(&sysfs_path, + "%shost%d%s", LINUX_SYSFS_FC_HOST_PREFIX, - d->scsi_host.host) < 0) { + d->scsi_host.host, + LINUX_SYSFS_VPORT_CREATE_POSTFIX) < 0) { virReportOOMError(); retval = -1; goto out; } - if (stat(sysfs_path, &st) != 0) { - /* Not a vport capable HBA; not an error, either. */ + if (stat(sysfs_path, &st) == 0) { + 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; + VIR_FREE(sysfs_path); + if (virAsprintf(&sysfs_path, + "%shost%d%s", + LINUX_SYSFS_SCSI_HOST_PREFIX, + d->scsi_host.host, + LINUX_SYSFS_VPORT_CREATE_POSTFIX) < 0) { + virReportOOMError(); + retval = -1; + goto out; + } + + if (stat(sysfs_path, &st) == 0) { + d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS; + } else { + /* Not a vport capable HBA; not an error, either. */ + VIR_DEBUG("No vport operation path found for host%d", + d->scsi_host.host); + } out: VIR_FREE(sysfs_path); -- 1.7.2

--- src/conf/node_device_conf.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 6583570..0b080ce 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1283,8 +1283,8 @@ virNodeDeviceGetWWNs(virNodeDeviceDefPtr def, ret = -1; } else if (*wwnn == NULL || *wwpn == NULL) { /* Free the other one, if allocated... */ - VIR_FREE(wwnn); - VIR_FREE(wwpn); + VIR_FREE(*wwnn); + VIR_FREE(*wwpn); ret = -1; virReportOOMError(); } -- 1.7.2

From: Daniel Veillard <veillard@redhat.com> Doing `virsh schedinfo rhel5u3 --cap 65535' the hypervisor does the call, but does not change the value nor raise an error. Best is just to consider it's not in the allowed values. The problem is that the error won't be output since the xend driver will then be called and raise an error error: this function is not supported by the hypervisor: unsupported in xendConfigVersion < 4 which will override the useful information from xenUnifiedDomainSetSchedulerParameters(). So best is to also invert the order in which the xen sub-drivers are called. * src/xen/xen_hypervisor.c: mark 65535 cap value as out of bound * src/xen/xen_hypervisor.c: reverse the order of the calls to the xen sub drivers to get the error message if needed --- src/xen/xen_driver.c | 3 ++- src/xen/xen_hypervisor.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index ddbfa7a..56ba41b 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1588,7 +1588,8 @@ xenUnifiedDomainSetSchedulerParameters (virDomainPtr dom, GET_PRIVATE(dom->conn); int i, ret; - for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i) { + /* do the hypervisor call last to get better error */ + for (i = XEN_UNIFIED_NR_DRIVERS - 1; i >= 0; i--) { if (priv->opened[i] && drivers[i]->domainSetSchedulerParameters) { ret = drivers[i]->domainSetSchedulerParameters(dom, params, nparams); if (ret == 0) diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 67d0f4b..6246513 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1394,8 +1394,8 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, } else if (STREQ (params[i].field, str_cap) && params[i].type == VIR_DOMAIN_SCHED_FIELD_UINT) { val = params[i].value.ui; - if (val > USHRT_MAX) { - snprintf(buf, sizeof(buf), _("Credit scheduler cap parameter (%d) is out of range (0-65535)"), val); + if (val >= USHRT_MAX) { + snprintf(buf, sizeof(buf), _("Credit scheduler cap parameter (%d) is out of range (0-65534)"), val); virXenErrorFunc(VIR_ERR_INVALID_ARG, __FUNCTION__, buf, val); return(-1); } -- 1.7.2