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

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@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;