Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Travis validation #13

Closed
wants to merge 0 commits into from
Closed

Conversation

nmorey
Copy link
Contributor

@nmorey nmorey commented Apr 18, 2019

@hnrose I cannot enable -Werror for clang due to this error:
osm_vendor_ibumad.c: In function ‘osm_vendor_open_port’:
osm_vendor_ibumad.c:745:56: error: passing argument 2 of ‘umad_get_ca_portguids’ from incompatible pointer type [-Werror=incompatible-pointer-types]
if ((r = umad_get_ca_portguids(p_vend->ca_names[ca], portguids,

You should be able to enable it once you fix it

@nmorey nmorey force-pushed the dev/master/travis branch 3 times, most recently from 27aee5c to d988f62 Compare April 18, 2019 08:22
@nmorey
Copy link
Contributor Author

nmorey commented Apr 18, 2019

@hnrose Do you know if osmtest can be run over ibsim ?
It feels a bit wrong to validate ibsim with opensm and opensm with ibsim but I don't see any other way to do this in travis. And the software is probably mature enough for it not to be an issue

@hnrose
Copy link
Contributor

hnrose commented Apr 18, 2019

[nmorey wrote:]
I cannot enable -Werror for clang due to this error:
osm_vendor_ibumad.c: In function ‘osm_vendor_open_port’:
osm_vendor_ibumad.c:745:56: error: passing argument 2 of ‘umad_get_ca_portguids’ from incompatible pointer type [-Werror=incompatible-pointer-types]
if ((r = umad_get_ca_portguids(p_vend->ca_names[ca], portguids,

Is it correct to assume that you are using libibumad in rdma-core and it's compiles OK without -Werror=incompatible-pointer-types ?

@hnrose
Copy link
Contributor

hnrose commented Apr 18, 2019

[nmorey wrote:]
Do you know if osmtest can be run over ibsim ?
It feels a bit wrong to validate ibsim with opensm and opensm with ibsim but I don't see any other way to do this in travis. And the software is probably mature enough for it not to be an issue

osmtest can't be run on ibsim because osmtest uses RMPP which is mostly not supported by the simulator except for single packet RMPP which is insufficient for osmtest. This has been a long time shortcoming of ibsim.

I agree it's a little weird to have the circular dependency to test ibsim. ibsim could also be exercised with some of the InfiniBand-diags but that is probably a similar dependency. More of InfiniBand-diags could run if opensm were running on the simulator. I need to think about this more.

@nmorey
Copy link
Contributor Author

nmorey commented Apr 18, 2019

@hnrose Yes it compiles fine without -Werror. For some reason clang is being a real pain in the *** and cannot figure out that the fields are actually aligned.

If we can't use ibsim, we do not have to create a circular dependency anyway. Not sure how we can do useful testing in Travis though.

This PR still make sense to verify that new patches do not break compilation at least

@hnrose
Copy link
Contributor

hnrose commented Apr 18, 2019

I'll run clang with -Werror and fix the issue but won't be able to get to this until mid next week.

@hnrose
Copy link
Contributor

hnrose commented Apr 18, 2019

[nmorey wrote:]
If we can't use ibsim, we do not have to create a circular dependency anyway. Not sure how we can do useful testing in Travis though.

This PR still make sense to verify that new patches do not break compilation at least

Yes checking that everything compiles is better than nothing ;-)

I will think more about testing even if it is primitive. It will at least put the structure in place to build on.

Thanks for your amazing efforts on this over the last few days!

@hnrose
Copy link
Contributor

hnrose commented Apr 21, 2019

Manually merged.

Created PR #14 (under test) to attempt to fix clang complaint so -Werror can be used

@hnrose
Copy link
Contributor

hnrose commented Apr 21, 2019

@nmorey Is libibumad from rdma-core being picked up ?

602osm_vendor_ibumad.c:746:56: error: incompatible pointer types passing '__be64 *'
603 (aka 'unsigned long long *') to parameter of type 'uint64_t *'
604 (aka 'unsigned long *') [-Werror,-Wincompatible-pointer-types]
605 ...if ((r = umad_get_ca_portguids(p_vend->ca_names[ca], &portguids[0],
606 ^~~~~~~~~~~~~
607/usr/include/infiniband/umad.h:166:59: note: passing argument to parameter
608 'portguids' here
609int umad_get_ca_portguids(const char *ca_name, uint64_t * portguids, int max);
610 ^

This declaration of umad_get_ca_portguids looks to be from umad.h prior to rdma-core.

I also see clang complaints of packed member alignment issue with p_mad->trans_id

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants