cmake: add dependency from ceph_smalliobenchrbd to cls libraries

[ivancich]

Add osd dependency to cls_rbd library when WITH_RBD specified. Otherwise, for example, "make vstart smalliobenchrbd" will not build necessary cls_rbd shared library.

[dillaman]

Perhaps all the cls libraries should be added as dependencies to vstart

[ivancich]

That would certainly fix my proximate issue. But shouldn't libcls_rbd.so be built when WITH_RBD specified? Or to put it the other way, is there an occasion when we wouldn't want it built when WITH_RBD is specified? Who can we bring in to this discussion to help figure this out?

[dillaman]

@ivancich Feel free to bring this up on ceph-devel if you don't like my answer. But for some background, I think it's odd to put a cls dependency directly against ceph-osd (since it really isn't a ceph-osd dependency -- the OSD will run just fine w/o any cls methods).

If we want to be technical, RBD requires cls_rbd, cls_journal, and cls_lock -- but this is more a test case dependency, so perhaps add the cls dependencies to just smalliobenchrbd.

[ivancich]

OK, I've changed it so ceph_smalliobenchrbd depends on the three libcls* files.

I'd still like to better understand why my first fix wasn't as acceptable, if you wouldn't mind helping me understand your perspective. I understand that OSD will run fine without libcls_rbd, but only until an rbd call is made in which case it will try to load the shared library and fail when it doesn't exist. So the runtime dependency exits when using rbd, and my assumption is that WITH_RBD signals the intention to use rbd. And a runtime dependency should be a build dependency.

[dillaman]

@ivancich Using that reasoning, all the cls_XYZ classes should be built when using building against the vstart target (with perhaps some WITH_XYZ wrappers). IMHO, the trouble with putting that dependency down in ceph-osd was that (a) it was partially fixing something and (b) it's not apparent that the ceph-osd CMakefile should be updated with new cls dependencies when new classes are created. The purpose of the 'vstart' dummy target was to cover building just enough of the system so that it runs -- that is why I suggested 'vstart' is a better place.

[ivancich]

So would you prefer it be a a) vstart dependency, b) ceph_smalliobenchrbd dependency, or c) both?

[dillaman]

@ivancich I think your current change solves the immediate issue and matches existing unit test CMakeList.txt cls_XYZ dependencies. Therefore, the current commit ltgm.

[cbodley]

@dillaman for vstart, i added our cls dependencies to the radosgw target (d3d0a11). if the rbd tool is part of vstart, you might add cls_{rbd,journal,lock} as dependencies to that