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

rbd-ggate: tool to map images on FreeBSD via GEOM Gate #15339

Merged
merged 2 commits into from Aug 8, 2017

Conversation

Projects
None yet
5 participants
@trociny
Copy link
Contributor

commented May 27, 2017

rbd-ggate spawns a process responsible for the creation of ggate
device and forwarding I/O requests between the GEOM Gate kernel
subsystem and RADOS.

On FreeBSD it provides functionality similar to rbd-nbd on Linux.

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

@trociny

This comment has been minimized.

Copy link
Contributor Author

commented May 27, 2017

@wjwithagen You might want to give it a try?

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented May 27, 2017

@trociny
Super.... Still cactching up after vacation.
But once there I will certainly give it some testruns.

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented May 28, 2017

@trociny
Do you have any plans to make it into a separate port?
So FreeBSD user can load the package without having to load a full Ceph installation?

@trociny

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2017

@wjwithagen I hope this will be merged to ceph repository. Then later sure it will be possible to build a separate package. Note, it will still depend on rados/rbd client libraries, which would need to be packaged too.

I don't have plans to work on the port. At least right now. Unfortunately upstream build breaks very frequently and it is still time consuming to keep local tree up to date, though it has become much easier recently after your reports and PRs. Your effort is highly appreciated. I suppose when your ceph-devel port becomes mature enough you might want to split it into separate packages anyway, then rbd-ggate could be packaged separately too.

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2017

@trociny
Hi Mykola,

All the other ceph stuff seems stable enough to actually start testing this.
First marker passed, I can easily patch it onto 12.1.0, once building is complete I'll see if your qa script actually passes.

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2017

@trociny
Build, and I can create a /dev/ggate0 device.
Gpart and dd can work on it.

newfs on a gparted disk generates cg 0: bad magic, so I need to ask on fs@freebsd.org whats up there.
the device can be used as device for ZFS. But after a while I/O stals, also a point for further investigation.
Could be that the underlying ceph store is to slow, because osd.0 complains about:
sync_entry timed out after 600 seconds

But I think this works in back form, and could be committed.

@trociny

This comment has been minimized.

Copy link
Contributor Author

commented Jul 5, 2017

@wjwithagen Thank you for testing. I will try to reproduce the issues to see if it is rbd-ggate related.

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2017

@tchaikov @trociny
How are we going to very this before committing?
The test script that Mykola wrote works at my end. Including the newfs.
(I've received patches to fix the newfs bailout, since it seems a problem in libufs, so that is on the FreeBSD side.)

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2017

@trociny
How are you testing this? Against a Linux Cluster?

@trociny

This comment has been minimized.

Copy link
Contributor Author

commented Jul 5, 2017

I am building and testing this on a FreeBSD host, using vstart.sh cluster.

@@ -0,0 +1,79 @@
:orphan:

This comment has been minimized.

Copy link
@tchaikov

tchaikov Jul 5, 2017

Contributor

@trociny you might want to include this file in doc/man/8/CMakeLists.txt.

This comment has been minimized.

Copy link
@trociny

trociny Jul 5, 2017

Author Contributor

@tchaikov thanks, updated

@tchaikov

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2017

@wjwithagen i don't have enough expertise in either FreeBSD or rbd, so cannot tell how to verify this. guess as long as @dillaman , @trociny and you are happy, we are good.

@dillaman

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2017

@tchaikov we really need CI for FreeBSD before I would feel comfortable including unique features.

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2017

@dillaman
Not a really unjustified requirement.
Question is just how to get there....

The current model works like I run about 4 build/test runs of master per day.
And when they fail, I usually go out and deduce the error, fix or ask to fix.
But that is only running the Jenkins tests.

I havent gotten Teuthology 'because I got stuck at what libvirt requires, the setup fails at the FreeBSD mix of QEMU and libvirt.
Otherwise I could perhaps also run some of the CI tests that you'd require for this modules to be enough QA

Testing this module does not work in a Jail, but needs a raw processor: either hardware of a full VM.

The other solution would be not to integrate in the Ceph tree, but keep it a separate FreeBSD port.
But then there is no testing at all.

So I'd rather have it in the tree, and then perhaps call FreeBSD a second tier provider.
How is this done with the solaris/AIX code bits that I have across?

@trociny trociny force-pushed the trociny:wip-rbd-ggate branch from b3d88da to d81a0c1 Jul 5, 2017

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2017

@trociny
Ah cool, some more people testdriving the port.

You might want to check pr #15906 where I'm trying to build a cluster on one server, but with jails.
This gets as close to a full cluster as I can on home-hardware.

@dillaman

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2017

@wjwithagen My biggest concern is actually about teuthology testing -- if we can get a semi-consistent subset of rbd tests against FreeBSD, it would make me comfortable. Without such testing, the software can be expected to decay.

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2017

@dillaman
I share your concern. I'm using FreeBSD since its birth (Linux 0.24 was way to unstable at that time)
And I've seen parts of code slowly rot away because nobody cared. (X25, ATM, old hardware)
So yes, it needs to be regularly tested.

First tests are the Jenkins build tests that are in the tree. You can see the latest results at:
http://cephdev.digiware.nl:8180/jenkins/job/ceph-master/lastBuild/console

And I can discuss with @trociny to see what more tests could/should be run in that sequence.
And we just add those tests thru CMake for FreeBSD.
So it will not be as intense as some of the theutology tests, but it will prevent the code rotting away.

@dillaman

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2017

@wjwithagen Thanks -- sounds good to me.

@trociny trociny force-pushed the trociny:wip-rbd-ggate branch from d81a0c1 to 778672c Jul 8, 2017

@wjwithagen wjwithagen self-requested a review Jul 13, 2017

@wjwithagen
Copy link
Contributor

left a comment

Passes pjdfs test

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Jul 13, 2017

@dillaman
Hi Jason,
After a last glitch I think we should commit this.
Either me of Mykola is going to add a test so that it'll be tested on eacht FreeBSD testrun.

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Jul 13, 2017

Running pjdfstest:

All tests successful.
Files=220, Tests=12216, 833 wallclock secs ( 2.29 usr  0.37 sys + 31.81 cusr 72.62 csys = 107.09 CPU)
Result: PASS

Still needs one patch to be loaded

@trociny trociny force-pushed the trociny:wip-rbd-ggate branch 2 times, most recently from f07e85e to 79e7102 Jul 13, 2017

@wjwithagen wjwithagen requested a review from dillaman Jul 19, 2017

_sudo dd if=${DATA} of=${DEV} bs=1M
_sudo sync
expect_false timeout 10 \
rbd bench ${IMAGE} --io-type write --io-size=1024 --io-total=1024

This comment has been minimized.

Copy link
@wjwithagen

wjwithagen Jul 19, 2017

Contributor

@trociny
Would need -p ${POOL}

rbd bench ${IMAGE} --io-type write --io-size=1024 --io-total=1024
_sudo rbd-ggate unmap ${DEV}
DEV=
rbd bench ${IMAGE} --io-type write --io-size=1024 --io-total=1024

This comment has been minimized.

Copy link
@wjwithagen

wjwithagen Jul 19, 2017

Contributor

@trociny
Same here

@wjwithagen wjwithagen added this to the luminous milestone Jul 28, 2017

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2017

@trociny
Mykola could you take a look a the 2 comments I have on the GEOM code.

@dillaman

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2017

@wjwithagen Mykola is on vacation for another week or so I believe.

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Jul 29, 2017

@dillaman
Right, sort to be expected around this time of year.... 8)

DEV=`_sudo rbd-ggate map --read-only ${POOL}/${IMAGE}`
_sudo rbd-ggate list | grep "^${DEV}$"
access=$(geom gate list ggate0 | awk '$1 == "access:" {print $2}')
test "${access}" = "read-only"

This comment has been minimized.

Copy link
@wjwithagen

wjwithagen Jul 29, 2017

Contributor

@trociny
This ggate0 needs to be ${DEV}.
I ran into some errors, due to ggate0 being used by another mount.

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2017

@dillaman
I understand from Sage that Liminous release is probably in a week.
So I guess we not merge before that, so no FreeBSD stuff in Luminous?

@dillaman

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2017

@wjwithagen it won't make 12.2.0, but it can be backported to a point release since it's self contained for the most part.

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2017

@dillaman
Hi, talked a bit with Sage yesterday, but he felt it was oke for 12.2.0 since the only 'invasive' part is
src/tools/rbd/action/Ggate.cc And the only impact that has, is that (only) on FreeBSD rbd gets extra ggate commands.
But it is up to you guys. I'm just trying to get things off my plate. And "official release", is just one of them.

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2017

@dillaman
Just as another though of what I could also do:

  • take 12.2.0
  • patch this PR on top of it
    (that is how I now do my master jenkins runs)
  • build the package as net/ceph.

It would still have version 12.2.0 Luminous as release version.
And I could tweak the hash into something like: abcdef7-patched

@liewegas

This comment has been minimized.

Copy link
Member

commented Aug 4, 2017

@dillaman I'm okay with merging this as is. We have pretty good momentum on the freebsd port and testing, but I suspect it will be a while before we have teuthology going.

@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2017

@liewegas @dillaman
Teuthology is expecting certain features from libvirt that FreeBSD has a hard time resolving;
KVM/Qemu is not a possible combi on FreeBSD, due to KVM.
Now one of the challenges would to see if the virtualiser on FreeBSD: bhyve would do.

First target is to get the net/ceph port out. Then I'll take a whack at teuthology.

@trociny trociny force-pushed the trociny:wip-rbd-ggate branch 2 times, most recently from 3389ead to 5bd5f0c Aug 6, 2017

@trociny

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2017

qa/workunits/rbd/rbd-ggate.sh updated to resolve @wjwithagen comments.

default:
c->release();
derr << pctx << ": invalid request command" << dendl;
return;

This comment has been minimized.

Copy link
@dillaman

dillaman Aug 8, 2017

Contributor

Nit: should this set m_stopping to true since the reader thread will exit?

This comment has been minimized.

Copy link
@trociny

trociny Aug 8, 2017

Author Contributor

@dillaman updated, thanks.

@@ -0,0 +1,375 @@
// -*- mode:C; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-

This comment has been minimized.

Copy link
@dillaman

dillaman Aug 8, 2017

Contributor

Was this sourced from a different location / project? I ask since it's an interesting separation between Driver and this plus the debug logging helper functions in "debug.h"

This comment has been minimized.

Copy link
@trociny

trociny Aug 8, 2017

Author Contributor

No. Although it is based on examples from FreeBSD source the code is mine. The reason I had to separate geom specific code to a C file is due to include geom/gate/g_gate.h, which further includes geom/geom.h. In the latest there is code like this:

LIST_ENTRY(g_class)     class;

which confuses c++ compiler.

Mykola Golub added some commits May 14, 2017

Mykola Golub
rbd-ggate: tool to map images on FreeBSD via GEOM Gate
rbd-ggate spawns a process responsible for the creation of ggate
device and forwarding I/O requests between the GEOM Gate kernel
subsystem and RADOS.

On FreeBSD it provides functionality similar to rbd-nbd on Linux.

Signed-off-by: Mykola Golub <mgolub@mirantis.com>
Mykola Golub
test: add wrapper to run rbd-ggate test on FreeBSD
Signed-off-by: Mykola Golub <mgolub@mirantis.com>

@trociny trociny force-pushed the trociny:wip-rbd-ggate branch from 5bd5f0c to 698c0ff Aug 8, 2017

@dillaman
Copy link
Contributor

left a comment

lgtm

@dillaman dillaman merged commit 96cde17 into ceph:master Aug 8, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details
@wjwithagen

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2017

@dillaman
Thanx for merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.