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

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.