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@xxxxxxxxxx>
>> ---
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
