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:xsc...@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.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to