FreeBSD related fixes #7170

trociny · 2016-01-10T17:40:34Z

No description provided.

tchaikov · 2016-01-29T06:36:52Z

@trociny lgtm apart from fa05220: although i prefer your approach, seems #7340 is likely to be merged first.

trociny · 2016-01-31T08:31:40Z

@tchaikov
It looks like #7340 does not totally resolve the issue (left the comment there). I will remove my patch from this PR after it is resolved.

tchaikov · 2016-01-31T09:16:21Z

@adamemerson ping?

tchaikov · 2016-02-08T14:45:33Z

@trociny since #7466 was merged. would you remove fa05220 from your commits?

trociny · 2016-02-08T18:46:52Z

@tchaikov thanks for the remainder. Rebased.

tchaikov · 2016-02-14T14:54:43Z

src/common/Makefile.am

@@ -86,11 +87,6 @@ libcommon_internal_la_SOURCES += \
 	common/pick_address.cc
 endif

-if LINUX


@trociny are you sure linux_versoin.c compiles on Solaris also? see #6416

@tchaikov I don't have a Solaris to check, but linux_versoin.c uses uname(). It looks like it is present on Solaris too and compatible.

@trociny agreed . see http://www.opensolarisforum.org/man/man2/uname.html @rohanmars could you help confirm this?

@trociny & @tchaikov linux_version.c looks like it compiles.

@rohanmars But it is also used by FileStore. Isn't FileStore built on Solaris?

tchaikov · 2016-02-16T16:51:12Z

lgtm unless @rohanmars thinks otherwise.

rohanmars · 2016-02-16T17:15:09Z

src/common/Makefile.am

@@ -103,10 +99,8 @@ libcommon_internal_la_SOURCES += \
 endif

 # used by RBD and FileStore
-if LINUX


@trociny & @tchaikov this will not compile on Solaris however, I had originally added a if WITH_RBD around that file.

@rohanmars Sorry, my previous message (it is also used by FileStore) actually was for this comment.

@trociny, no FileStore is not built for client side as I understand. Due to limitation on Makefile.am syntax, probably best to add another if FREE_BSD or something. Not sure why WITH_RBD guard was taken away.

probably because it is used by FileStore and BlueStore. can we guard it with WITH_OSD and `!SOLARIS?

wjwithagen · 2016-02-16T21:44:00Z

@rohanmars @trociny @tchaikov
linux_version is only used to determine how caching is set on disks in linux.
I would recone that in both Solaris as in FreeBSD that is done completely different, IF it is even possible. So the solution is to not use linux_version unless we're using Linux, and thus also exclude the code in FileJournal.cc

tchaikov · 2016-02-17T03:18:26Z

linux_version is only used to determine how caching is set on disks in linux.

@wjwithagen you are right, get_linux_version() is used to check the features (extsize, write cache, dentry invalidation) by comparing the linux version number. but it does not hurt if we compile it and use it check result, as the number returned by get_linux_version() is 0 if it runs on FreeBSD or Solaris. at the caller side, we tend to be conservative at seeing 0, and hence disable feature being test. so we have less #if __linux__ in upper layer of caller. and hide the platform specific thing in the common/ directory.

if our goal is to port the ceph client, maybe we can guard "common/linux_version.c" with

if ENABLE_SERVER
libcommon_internal_la_SOURCES += \
    common/linux_version.c
else
if WITH_FUSE
libcommon_internal_la_SOURCES += \
    common/linux_version.c
else
endif
endif

wjwithagen · 2016-02-17T20:40:01Z

@tchaikov
I agree with your objective, but then as such I have contemplated to rename linux_version.cc to os_version.cc which allows for adding more versions without major changes to the compile infra.
And it can always be included.

Now the problem is when such a block of code tries to call OS specific functions.
Then we still need to ifdef that block of code at the point of using the get_OS_version, otherwise is does not compile.
But it removes the ifdefs in the makefile-infra

Looking at the code in ./src/os/filestore/FileJournal.cc there are errors/warnings printed that are not relevant on other platforms and those might confuse a admin. Even text that is highlighted in RED.
So those prints need only be done if get_linux_level <> 0

BTW: calls are at:
./src/os/filestore/FileJournal.cc: int ver = get_linux_version();
./src/os/filestore/XfsFileStoreBackend.cc: int ver = get_linux_version();
./src/ceph_fuse.cc: int ver = get_linux_version();

trociny · 2016-02-18T07:33:15Z

Ok, I tend to agree that linux_version.c should be included only on linux platform, and get_linux_version() (together with linux only relevant code around it) be ifdefed.

As for blkdev.cc I think it should be included for all platforms with os specific things ifdefed in blkdev.cc.

I will come with the patch later.

tchaikov · 2016-02-18T08:22:38Z

@wjwithagen agreed!

trociny · 2016-02-20T20:41:21Z

rebased after updating the patch for linux_version.c and blkdev.cc handling.

trociny · 2016-02-24T18:32:45Z

Rebased after updating stop.sh

tchaikov · 2016-03-09T07:12:48Z

@trociny could rebase the PR against master, and i think it's ready for needs-qa.

Signed-off-by: Mykola Golub <mgolub@mirantis.com>

- export CC and CXX, so the same compiler is used - use NPROC Signed-off-by: Mykola Golub <mgolub@mirantis.com>

On freebsd <malloc.h> has been replaced by <stdlib.h> Signed-off-by: Mykola Golub <mgolub@mirantis.com>

Signed-off-by: Mykola Golub <mgolub@mirantis.com>

(excluding linux specific features) Signed-off-by: Mykola Golub <mgolub@mirantis.com>

`$(CC) -dumpversion` works for clang too, as it supports gcc compatibility for -dumpversion option. Signed-off-by: Mykola Golub <mgolub@mirantis.com>

Signed-off-by: Mykola Golub <mgolub@mirantis.com>

trociny · 2016-03-09T07:53:11Z

rebased against the recent master

tchaikov · 2016-03-09T12:03:36Z

lgtm with qa run.

wjwithagen · 2016-03-09T13:05:32Z

@trociny @tchaikov Nice...
happy with the blockdev implementation
Once this is in it'll remove quite some of my commits in wip-wjw-freebsd-*

tchaikov · 2016-03-14T09:37:27Z

tested at http://pulpito.ceph.com/kchai-2016-03-11_19:18:43-rados-wip-kefu-testing---basic-smithi/ , failed tests are known issues and not related to the tested changes:

FreeBSD related fixes Reviewed-by: Kefu Chai <kchai@redhat.com>

wjwithagen · 2016-03-14T11:12:51Z

@tchaikov
Cool,thanx for this merge.
I'll update my pull-set after I've run some tests with Clang 3.7 to check why I'm having crashes that did not happen before the upgrade to 3.8. But I'm down with flue and lots of work that makes money.
So it'll be a day or 2.

trociny force-pushed the wip-freebsd-misc branch from 8f52a25 to ca35ec6 Compare January 10, 2016 18:56

tchaikov self-assigned this Jan 29, 2016

tchaikov added the build/ops label Jan 29, 2016

trociny mentioned this pull request Jan 31, 2016

time: Make ceph_time clocks work under BSD #7340

Merged

trociny force-pushed the wip-freebsd-misc branch from ca35ec6 to 8dcc002 Compare February 8, 2016 18:46

tchaikov reviewed Feb 14, 2016
View reviewed changes

trociny force-pushed the wip-freebsd-misc branch from 8dcc002 to 3a39bb1 Compare February 14, 2016 17:09

tchaikov added the needs-qa label Feb 16, 2016

rohanmars reviewed Feb 16, 2016
View reviewed changes

tchaikov removed the needs-qa label Feb 17, 2016

trociny force-pushed the wip-freebsd-misc branch from 3a39bb1 to 72993f4 Compare February 20, 2016 20:39

trociny force-pushed the wip-freebsd-misc branch from 72993f4 to 87ba670 Compare February 24, 2016 18:31

Mykola Golub added 4 commits March 9, 2016 07:50

freebsd: use sysctl instead of nproc

1ae5600

Signed-off-by: Mykola Golub <mgolub@mirantis.com>

rocksdb: more portable make invocation

48a1ba5

- export CC and CXX, so the same compiler is used - use NPROC Signed-off-by: Mykola Golub <mgolub@mirantis.com>

common: include malloc.h only on linux

a572226

On freebsd <malloc.h> has been replaced by <stdlib.h> Signed-off-by: Mykola Golub <mgolub@mirantis.com>

compat: fix build on non-linux

6ce4744

Signed-off-by: Mykola Golub <mgolub@mirantis.com>

Mykola Golub added 4 commits March 9, 2016 07:51

common: build with blkdev on all platforms

aa33837

Signed-off-by: Mykola Golub <mgolub@mirantis.com>

rbd: build rbd binary on all platforms

2be4029

(excluding linux specific features) Signed-off-by: Mykola Golub <mgolub@mirantis.com>

compat: don't call gcc explicitly

88c666d

`$(CC) -dumpversion` works for clang too, as it supports gcc compatibility for -dumpversion option. Signed-off-by: Mykola Golub <mgolub@mirantis.com>

stop.sh: make more portable

27f2d80

Signed-off-by: Mykola Golub <mgolub@mirantis.com>

trociny force-pushed the wip-freebsd-misc branch from 87ba670 to 27f2d80 Compare March 9, 2016 07:52

tchaikov added the needs-qa label Mar 9, 2016

tchaikov added the wip-kefu-testing label Mar 14, 2016

tchaikov added a commit that referenced this pull request Mar 14, 2016

Merge pull request #7170 from trociny/wip-freebsd-misc

0d6d340

FreeBSD related fixes Reviewed-by: Kefu Chai <kchai@redhat.com>

tchaikov merged commit 0d6d340 into ceph:master Mar 14, 2016

trociny deleted the wip-freebsd-misc branch March 14, 2016 10:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FreeBSD related fixes #7170

FreeBSD related fixes #7170

trociny commented Jan 10, 2016

tchaikov commented Jan 29, 2016

trociny commented Jan 31, 2016

tchaikov commented Jan 31, 2016

tchaikov commented Feb 8, 2016

trociny commented Feb 8, 2016

tchaikov Feb 14, 2016

trociny Feb 14, 2016

tchaikov Feb 15, 2016

rohanmars Feb 16, 2016

trociny Feb 16, 2016

tchaikov commented Feb 16, 2016

rohanmars Feb 16, 2016

trociny Feb 16, 2016

rohanmars Feb 16, 2016

tchaikov Feb 17, 2016

wjwithagen commented Feb 16, 2016

tchaikov commented Feb 17, 2016

wjwithagen commented Feb 17, 2016

trociny commented Feb 18, 2016

tchaikov commented Feb 18, 2016

trociny commented Feb 20, 2016

trociny commented Feb 24, 2016

tchaikov commented Mar 9, 2016

trociny commented Mar 9, 2016

tchaikov commented Mar 9, 2016

wjwithagen commented Mar 9, 2016

tchaikov commented Mar 14, 2016

wjwithagen commented Mar 14, 2016

FreeBSD related fixes #7170

FreeBSD related fixes #7170

Conversation

trociny commented Jan 10, 2016

tchaikov commented Jan 29, 2016

trociny commented Jan 31, 2016

tchaikov commented Jan 31, 2016

tchaikov commented Feb 8, 2016

trociny commented Feb 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tchaikov commented Feb 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wjwithagen commented Feb 16, 2016

tchaikov commented Feb 17, 2016

wjwithagen commented Feb 17, 2016

trociny commented Feb 18, 2016

tchaikov commented Feb 18, 2016

trociny commented Feb 20, 2016

trociny commented Feb 24, 2016

tchaikov commented Mar 9, 2016

trociny commented Mar 9, 2016

tchaikov commented Mar 9, 2016

wjwithagen commented Mar 9, 2016

tchaikov commented Mar 14, 2016

wjwithagen commented Mar 14, 2016