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

time: Make ceph_time clocks work under BSD #7340

Merged
merged 1 commit into from Jan 29, 2016
Merged

Conversation

adamemerson
Copy link
Contributor

Signed-off-by: Adam C. Emerson aemerson@redhat.com

Linux's naming things _COARSE is non-standard. Other systems name them other
non-standard things. BSD names them _FAST.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
@tchaikov tchaikov mentioned this pull request Jan 29, 2016
liewegas added a commit that referenced this pull request Jan 29, 2016
common: ake ceph_time clocks work under BSD

Reviewed-by: Erwan Velu <erwan@redhat.com>
@liewegas liewegas merged commit 4cf7af5 into master Jan 29, 2016
@liewegas liewegas deleted the wip-portable-time branch January 29, 2016 13:25
@trociny
Copy link
Contributor

trociny commented Jan 31, 2016

It still does not build on FreeBSD due to CLOCK_REALTIME_COARSE being used in ceph_time.cc:

common/ceph_time.cc:52:21: error: use of undeclared identifier 'CLOCK_REALTIME_COARSE'
clock_gettime(CLOCK_REALTIME_COARSE, &ts);

In #7170 I used a little different approach: fa05220

@wjwithagen
Copy link
Contributor

On 31-1-2016 09:28, Mykola Golub wrote:

It still does not build on FreeBSD due to CLOCK_REALTIME_COARSE being
used in ceph_time.cc:

common/ceph_time.cc:52:21: error: use of undeclared identifier
'CLOCK_REALTIME_COARSE'
clock_gettime(CLOCK_REALTIME_COARSE, &ts);

In #7170 #7170 I used a little
different approach: fa05220
fa05220

Hi Mykola,

I sort of prefer the approach where it is made more explicit, and put at
the place where it is actually used.

If you prefer you way of porting, then perhaps src/include/compat.h
would be a more applicable place?

--WjW

@trociny
Copy link
Contributor

trociny commented Jan 31, 2016

I am fine with any approach. I just was pointing to yet another possible, which still is not perfect as it might be confusing. I suppose, a better solution would be to define in ceph_time.h a wrapper function ceph_clock_gettime() and constants CEPH_CLOCK_REALTIME..., so the changes are localized. But again I am fine with any fix.

@adamemerson
Copy link
Contributor Author

I have a fix in adamemerson@5477424

If it works for people I'll send another pull request.

@adamemerson
Copy link
Contributor Author

Wrong commit. I'll just link to the branch

https://github.com/adamemerson/ceph/commits/wip-bsd-time-second-try

@wjwithagen
Copy link
Contributor

On 1-2-2016 16:47, Adam C. Emerson wrote:

Wrong commit. I'll just link to the branch

https://github.com/adamemerson/ceph/commits/wip-bsd-time-second-try


Reply to this email directly or view it on GitHub
#7340 (comment).

In my work last night I added the "std" workaround to this blob of code
as well. Removing clock_gettime calls here will very likely compile for
BSD without trouble. And it also mitigate the danger of having lots of
places where the conditonal clocktype selection is #ifdef-ed.

--WjW

@adamemerson
Copy link
Contributor Author

All right, I now have pull request #7466 submitted that should take care of this.

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