[libvirt] [PATCH]Fix vPort Manage of FC vHBA creation

[Ján Tomko]

On 06/28/2013 11:39 AM, Daniel P. Berrange wrote:
> On Fri, Jun 28, 2013 at 05:30:14PM +0800, Dennis Chen wrote:
>> On 06/28/2013 04:39 PM, Ján Tomko wrote:
>>> On 06/28/2013 03:22 AM, Dennis Chen wrote:
>>>> When create a virtual FC HBA with virsh/libvirt API, an error message will be
>>>> returned:"error: Node device not found",
>>>> also the 'nodedev-dumpxml' shows wrong information of wwpn & wwnn for the new
>>>> created device.
>>>>
>>>> Signed-off-by:xschen at tnsoft.com.cn
>>>> ---
>>>> src/util/virutil.c |    4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/util/virutil.c b/src/util/virutil.c
>>>> index 6fa0212e..569d035 100644
>>>> --- a/src/util/virutil.c
>>>> +++ b/src/util/virutil.c
>>>> @@ -1792,8 +1792,8 @@ virManageVport(const int parent_host,
>>>>      if (virAsprintf(&vport_name,
>>>>                      "%s:%s",
>>>> -                    wwnn,
>>>> -                    wwpn) < 0) {
>>>> +                    wwpn,
>>>> +                    wwnn) < 0) {
>>>>          virReportOOMError();
>>>>          goto cleanup;
>>>>      }
>>>>
>>> Hmm, this is what we've had before commit f90af69 [1]
>>> but according to scsi_fc_transport.txt [2] in kernel docs, it should be
>>> <WWPN>:<WWNN>. I wonder what that commit was trying to fix.
>>>
>>> Jan
>>>
>>> [1] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=f90af69
>>> [2] https://www.kernel.org/doc/Documentation/scsi/scsi_fc_transport.txt
>>>
>> Interesting! According to my testing result (kernel version 2.6.32),
>> kernel docs is correct,it should be <WWPN>:<WWNN>. It's causing me
>> trouble when creating the device with virsh after that commit...
> 
> I say ACK to your patch.  Your patch matches the kernel documentation
> and you've confirmed that it fixes a clear bug.  The original patch
> of Osiers has zero information about what it was fixing, so there's
> little reason to assume it was a correct change based on your feedback.

I found it: it was being called with the parameters swapped in the scsi
storage backend.

Pushing with this squashed in:

diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index 285c5cb..3deceda 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -659,8 +659,8 @@ createVport(virStoragePoolSourceAdapter adapter)
     if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0)
         return -1;

-    if (virManageVport(parent_host, adapter.data.fchost.wwnn,
-                       adapter.data.fchost.wwpn, VPORT_CREATE) < 0)
+    if (virManageVport(parent_host, adapter.data.fchost.wwpn,
+                       adapter.data.fchost.wwnn, VPORT_CREATE) < 0)
         return -1;

     virFileWaitForDevices();
@@ -682,8 +682,8 @@ deleteVport(virStoragePoolSourceAdapter adapter)
     if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0)
         return -1;

-    if (virManageVport(parent_host, adapter.data.fchost.wwnn,
-                       adapter.data.fchost.wwpn, VPORT_DELETE) < 0)
+    if (virManageVport(parent_host, adapter.data.fchost.wwpn,
+                       adapter.data.fchost.wwnn, VPORT_DELETE) < 0)
         return -1;

     return 0;

Jan