3:47 p.m.
On 01/02/2017 09:18 AM, Ján Tomko wrote:
On Fri, Nov 18, 2016 at 09:26:29AM -0500, John Ferlan wrote:
> If a <parent> is not supplied in the XML used to create a non-persistent
> vHBA, then instead of failing, let's try to find a "vports" capable
node
> device and use that.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/conf/node_device_conf.c | 39
> ++++++++++++++++++++++++++++++++++++
> src/conf/node_device_conf.h | 3 +++
> src/libvirt_private.syms | 1 +
> src/node_device/node_device_driver.c | 15 +++++++++-----
> 4 files changed, 53 insertions(+), 5 deletions(-)
>
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 5396681..3aa77cf 100644
> +
> +int
> +virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs,
> + int *parent_host)
> +{
> + virNodeDeviceObjPtr parent = NULL;
> + int ret;
> +
> + if (!(parent = virNodeDeviceFindByCap(devs, "vports"))) {
Why do we pass the capability as a string?
OK - I'll alter this to:
const char *cap = virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS);
and use "cap" instead of "vports".
NB:
I cannot search on the virNodeDevCapType because it's actually a
"subtype" of the 'scsi_host' type (as seen in virNodeDeviceHasCap)
The virNodeDeviceHasCap should also alter those hardcoded STREQ values
to use the virNodeDevCapTypeToString for VIR_NODE_DEV_CAP_FC_HOST and
VIR_NODE_DEV_CAP_VPORTS...
I'll squash the following in (sorry - inline and will mess with mail
window widths, but I tested this it works):
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 42f6593..b13cb6b 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -75,14 +75,19 @@ virNodeDevCapsDefParseString(const char *xpath,
int virNodeDeviceHasCap(const virNodeDeviceObj *dev, const char *cap)
{
virNodeDevCapsDefPtr caps = dev->def->caps;
+ const char *fc_host_cap =
+ virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_FC_HOST);
+ const char *vports_cap =
+ virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS);
+
while (caps) {
if (STREQ(cap, virNodeDevCapTypeToString(caps->data.type)))
return 1;
else if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST)
- if ((STREQ(cap, "fc_host") &&
+ if ((STREQ(cap, fc_host_cap) &&
(caps->data.scsi_host.flags &
VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST)) ||
- (STREQ(cap, "vports") &&
+ (STREQ(cap, vports_cap) &&
(caps->data.scsi_host.flags &
VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS)))
return 1;
@@ -1851,9 +1856,10 @@ virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs,
int *parent_host)
{
virNodeDeviceObjPtr parent = NULL;
+ const char *cap = virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS);
int ret;
- if (!(parent = virNodeDeviceFindByCap(devs, "vports"))) {
+ if (!(parent = virNodeDeviceFindByCap(devs, cap))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Could not find any vport capable device"));
return -1;
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Could not find any vport capable device"));
> + return -1;
> + }
> +
> diff --git a/src/node_device/node_device_driver.c
> b/src/node_device/node_device_driver.c
> index 91bb142..0e091fe 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -584,11 +584,16 @@ nodeDeviceCreateXML(virConnectPtr conn,
> if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1)
> goto cleanup;
>
> - if (virNodeDeviceGetParentHost(&driver->devs,
> - def->name,
> - def->parent,
> - &parent_host) == -1) {
> - goto cleanup;
> + if (def->parent) {
> + if (virNodeDeviceGetParentHost(&driver->devs,
> + def->name,
> + def->parent,
> + &parent_host) < 0)
> + goto cleanup;
> + } else {
> + /* Try to find "a" vport capable scsi_host when no parent
> supplied */
The quotes make me nervous.
Removed... I think the point of them was it'll be capable, but
there's no check if there's enough ports remaining for use. That'll
fail differently though.
John
> + if
(virNodeDeviceFindVportParentHost(&driver->devs,
> &parent_host) < 0)
> + goto cleanup;
> }
ACK
Jan