Skip to content

Conversation

@jsquyres
Copy link
Member

Do not merge until ofiwg/libfabric#2052 is closed.

@jsquyres jsquyres added this to the v2.1.0 milestone Aug 20, 2016
@jsquyres jsquyres self-assigned this Aug 20, 2016
@jsquyres jsquyres force-pushed the pr/update-usnic-to-libfabric-v1.4 branch 3 times, most recently from d39609e to b7b14c6 Compare August 20, 2016 11:09
@jsquyres
Copy link
Member Author

@bturrubiates Please review.

@jsquyres jsquyres force-pushed the pr/update-usnic-to-libfabric-v1.4 branch 2 times, most recently from 8577c25 to 0d4dfaa Compare August 25, 2016 01:25

int ret;
struct fi_info *info_list_local;
ret = fi_getinfo(version, NULL, 0, 0, &hints, &info_list_local);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not:

return fi_getinfo(version, NULL, 0, 0, &hints, *info_list_local);

and get rid of info_list_local and ret?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you meant

return fi_getinfo(version, NULL, 0, 0, &hints, info_list);

But yes, you're right -- that's a bit simpler.

With libfabric v1.4, the usnic provider changed the values of its
fabric and domain name strings (compared to libfabric <v1.4).  Update
the Open MPI usNIC BTL to handle both pre-v1.4 and v1.4 fabric/domain
names.

Signed-off-by: Jeff Squyres <[email protected]>
Signed-off-by: Jeff Squyres <[email protected]>
@jsquyres jsquyres force-pushed the pr/update-usnic-to-libfabric-v1.4 branch from 0d4dfaa to f56b16f Compare August 25, 2016 10:54
@jsquyres
Copy link
Member Author

@bturrubiates Pushed new commits addressing your comments. Still waiting for Travis to clear CI.

@bturrubiates
Copy link

👍

@jsquyres jsquyres merged commit 9ae51a0 into open-mpi:master Aug 26, 2016
@jsquyres jsquyres deleted the pr/update-usnic-to-libfabric-v1.4 branch August 26, 2016 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants