Bug 563318 - Review Request: ceph - User space components of the CEPH file system
Summary: Review Request: ceph - User space components of the CEPH file system
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Jonathan Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-02-09 20:04 UTC by Josef Bacik
Modified: 2013-02-27 14:22 UTC (History)
12 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2011-04-25 16:05:17 UTC
Type: ---
Embargoed:
jonathan: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Josef Bacik 2010-02-09 20:04:54 UTC
Ceph is a distributed network file system designed to provide excellent
performance, reliability, and scalability.

SRPM: http://people.redhat.com/jwhiter/ceph-0.18-1.fc12.src.rpm
Spec: http://people.redhat.com/jwhiter/ceph.spec

I've mockbuilt it and run rpmlint against everything it creates, I get the following now

[root@localhost SPECS]# rpmlint /root/rpmbuild/RPMS/x86_64/ceph*.rpm
ceph.x86_64: W: name-repeated-in-summary C CEPH
ceph.x86_64: W: spelling-error %description -l en_US scalability -> availability, sociability, inviolability
ceph.x86_64: W: shared-lib-calls-exit /usr/lib64/librados.so.1.0.0 exit.5
ceph.x86_64: W: shared-lib-calls-exit /usr/lib64/librados.so.1.0.0 _exit.5
ceph.x86_64: W: shared-lib-calls-exit /usr/lib64/libceph.so.1.0.0 exit.5
ceph.x86_64: W: shared-lib-calls-exit /usr/lib64/libceph.so.1.0.0 _exit.5
4 packages and 0 specfiles checked; 0 errors, 6 warnings.

the exit() in shared libs can be ignored, they are special C++ classes that are supposed to exit() when an action happens.  The other two warnings are dumb.

Comment 1 Jason Tibbitts 2010-02-09 20:54:21 UTC
Scripts parse package reviews, so please either use the template for submitting them or try to stick to the usual format.

Comment 2 Jonathan Dieter 2010-03-26 19:38:00 UTC
Do you mind putting out an update for 0.19?  I'd feel better about reviewing this if it has a path for upgrading the filesystem without having to reformat.

Comment 3 Jonathan Dieter 2010-04-20 09:39:07 UTC
Josef, are you still interested in packaging this?

Comment 4 Josef Bacik 2010-04-20 14:13:14 UTC
Yeah sorry, got 1000 things going on atm.  I will update this today for you, thanks.

Comment 5 Josef Bacik 2010-04-20 20:44:40 UTC
Argh I'm having a problem building this atm, rpmbuild keeps driving the box into swap.  As soon as I figure out whats going on I'll post the new spec and srpm up here.

Comment 6 Josef Bacik 2010-04-21 16:55:40 UTC
Well that was a freaking ordeal.  I've updated ceph, its now at


SRPM: http://people.redhat.com/jwhiter/ceph-0.19.1-1.fc12.src.rpm
Spec: http://people.redhat.com/jwhiter/ceph.spec

Comment 7 Jonathan Dieter 2010-04-21 17:21:49 UTC
I'm sorry about the pain.  I've got a crazy week ahead here, but I'll try to get on this over the weekend.

Comment 8 Fabian Deutsch 2010-04-23 21:14:49 UTC
Just to give some input:
I successfully built ceph using koji, but I was not able to build it on my local machine:
make[2]: *** [rgw_main.o] Error 1

rpmlint just notes some small things:
name-repeated-in-summary
spelling-error
shared-lib-calls-exit
no-manual-page-for-binary

I did not yet run the daemon.

Comment 9 Jonathan Dieter 2010-04-25 18:25:55 UTC
Key:
X   - Good
-   - Not so good
N/A - Not applicable

[ - ] MUST: rpmlint must be run on every package

      Aside from non-existent spelling mistakes, the only thing that
      comes up is:
      ceph.x86_64: W: shared-lib-calls-exit
                      /usr/lib64/librados.so.1.0.0 exit.5
      ceph.x86_64: W: shared-lib-calls-exit
                      /usr/lib64/librados.so.1.0.0 _exit.5
      ceph.x86_64: W: shared-lib-calls-exit
                      /usr/lib64/libceph.so.1.0.0 exit.5
      ceph.x86_64: W: shared-lib-calls-exit
                      /usr/lib64/libceph.so.1.0.0 _exit.5

      If we could try to track this down, it would be nice, as this
      really isn't recommended.

[ X ] MUST: The package must be named according to the Package Naming 
      Guidelines
[ X ] MUST: The spec file name must match the base package %{name}
[ X ] MUST: The package must meet the Packaging Guidelines
[ X ] MUST: The package must be licensed with a Fedora approved license
      and meet the Licensing Guidelines

      LGPLv2

[ X ] MUST: The License field in the package spec file must match the 
      actual license
[ - ] MUST: If (and only if) the source package includes the text of the 
      license(s) in its own file, then that file, containing the text of 
      the license(s) for the package must be included in %doc

      I see that you included COPYING only in the -devel package.  I
      think it should be included in *every* package, based on the above
      text (though it is a bit ambiguous).

[ X ] MUST: The spec file must be written in American English.
[ X ] MUST: The spec file for the package MUST be legible.
[ X ] MUST: The sources used to build the package must match the upstream 
      source, as provided in the spec URL.

      sha256sum:
      20e311c97e761017cdbbad13ab06bc6694338871832579bd51203173535ed796

[ - ] MUST: The package MUST successfully compile and build into binary 
      rpms on at least one primary architecture

      When I try to build this package in Fedora 13 x86_64, I get the following:
      RPM build errors:
          Installed (but unpackaged) file(s) found:
          /usr/lib64/libhadoopcephfs.so
          /usr/lib64/libhadoopcephfs.so.1
          /usr/lib64/libhadoopcephfs.so.1.0.0

[N/A] MUST: If the package does not successfully compile, build or work on 
      an architecture, then those architectures should be listed in the 
      spec in ExcludeArch.
[ X ] MUST: All build dependencies must be listed in BuildRequires.
[N/A] MUST: The spec file MUST handle locales properly.
[ X ] MUST: Every binary RPM package (or subpackage) which stores shared 
      library files (not just symlinks) in any of the dynamic linker's 
      default paths, must call ldconfig in %post and %postun.
[N/A] MUST: If the package is designed to be relocatable, the packager must 
      state this fact in the request for review.
[ - ] MUST: A package must own all directories that it creates. If it does 
      not create a directory that it uses, then it should require a package 
      which does create that directory.

      You create %{_libdir}/ceph, but don't own it.

[ X ] MUST: A package must not contain any duplicate files in the %files 
      listing.
[ X ] MUST: Permissions on files must be set properly. Executables should 
      be set with executable permissions, for example. Every %files section 
      must include a %defattr(...) line.
[ X ] MUST: Each package must have a %clean section, which contains rm -rf
         %{buildroot} (or $RPM_BUILD_ROOT).
[ X ] MUST: Each package must consistently use macros.
[ X ] MUST: The package must contain code, or permissable content.
[N/A] MUST: Large documentation files must go in a -doc subpackage.
[ X ] MUST: If a package includes something as %doc, it must not affect the 
      runtime of the application.
[ X ] MUST: Header files must be in a -devel package.
[N/A] MUST: Static libraries must be in a -static package.
[N/A] MUST: Packages containing pkgconfig(.pc) files must 'Requires: 
      pkgconfig'.
[ X ] MUST: If a package contains library files with a suffix (e.g. 
      libfoo.so.1.1), then library files that end in .so (without suffix) 
      must go in a -devel package.
[ X ] MUST: In the vast majority of cases, devel packages must require the 
      base package using a fully versioned dependency: Requires: %{name} =
      %{version}-%{release}
[ X ] MUST: Packages must NOT contain any .la libtool archives, these must 
      be removed in the spec if they are built.
[N/A] MUST: Packages containing GUI applications must include a
      %{name}.desktop file.
[ X ] MUST: Packages must not own files or directories already owned by 
      other packages.
[ X ] MUST: At the beginning of %install, each package MUST run rm -rf
      %{buildroot} (or $RPM_BUILD_ROOT).
[ X ] MUST: All filenames in rpm packages must be valid UTF-8.


[ ? ] SHOULD query upstream to include it.
[N/A] SHOULD: The description and summary sections in the package spec file
      should contain translations for supported Non-English languages, if
      available.
[ - ] SHOULD: The reviewer should test that the package builds in mock.

      I've been unable to test that it builds in mock, but according to the
      previous comment, it appears that it doesn't.

[ ? ] SHOULD: The package should compile and build into binary rpms on all
      supported architectures.
[ ? ] SHOULD: The reviewer should test that the package functions as
      described.

      I've tested the most of the binaries don't segfault, but not much
      more.

[ X ] SHOULD: If scriptlets are used, those scriptlets must be sane.
[ X ] SHOULD: Usually, subpackages other than devel should require the base
      package using a fully versioned dependency.
[N/A] SHOULD: The placement of pkgconfig(.pc) files depends on their
      usecase, and this is usually for development purposes, so should be
      placed in a -devel pkg.
[N/A] SHOULD: If the package has file dependencies outside of /etc, /bin,
      /sbin, /usr/bin, or /usr/sbin consider requiring the package which
      provides the file instead of the file itself.
[ X ] SHOULD: your package should contain man pages for binaries/scripts.

Apart from this, the description for the fuse client is a bit too brief

Comment 10 Jonathan Dieter 2010-04-25 18:31:37 UTC
A spec file which includes COPYING in every package, includes libhadoopcephfs.so*, owns %{_libdir}/ceph and fixes the description for the fuse client is available at:

http://www.lesbg.com/jdieter/ceph.spec

Comment 11 Fabian Deutsch 2010-04-25 18:37:46 UTC
Re(In reply to comment #9)
> [ - ] SHOULD: The reviewer should test that the package builds in mock.
> 
>       I've been unable to test that it builds in mock, but according to the
>       previous comment, it appears that it doesn't.

I just did a plain rpmrebuild outside of mock.

Comment 12 Josef Bacik 2010-04-26 13:16:40 UTC
Hrm I built this thing on a F12 box and libhadoop wasn't built.  I know its supposed to be there, it's weird that it didn't show up.

The exit() in the shared libraries are expected, they are specifically "kill this client" functions for when things in the cluster go wrong, so they aren't going anywhere.

I interpreted the COPYING thing for just the -devel package, but it makes sense to have it in all packages too.

It seems your spec cleans up all of the issues you found (thank you!) I will rebuild in koji to make sure the thing will actually build somewhere other than my box.  I built it in MOCK and it worked for me, but my box is an odd one in general so hopefully koji will be a little bit more consistent.

Comment 13 Jonathan Dieter 2010-04-26 13:50:15 UTC
As I was playing around with this today, I noticed that the man pages for cauthtool and rados are in the ceph-fuse package, while the binaries are in the base package.  We probably want to move them to the base package as I'm assuming cauthtool and rados aren't just needed for the fuse client.

As for the exit() in shared libraries, your explanation makes sense, so we'll call that good.  FWIW, I did build the package on my system with the updated spec and haven't hit any glitches yet (though I haven't set up a ceph cluster yet).

I'm going to call this approved, with the proviso that the man pages get moved (or binaries the other way if they are specific to the fuse client).

Comment 14 Josef Bacik 2010-04-26 14:00:59 UTC
Argh yeah that man page thing was a screw up, i just saw i needed to add man pages and put them under the first %{_mandir} tag I saw.  Here are the updated spec and srpm

SRPM: http://people.redhat.com/jwhiter/ceph-0.19.1-3.fc12.src.rpm
Spec: http://people.redhat.com/jwhiter/ceph.spec

Thanks for the review!

Comment 15 Josef Bacik 2010-04-26 14:21:17 UTC
New Package CVS Request
=======================
Package Name: ceph
Short Description: User space components of the CEPH file system
Owners: josef boodle
Branches: F-12 F-13
InitialCC:

Comment 16 Jonathan Dieter 2010-04-26 14:31:03 UTC
No problem.  Have you thought about asking the kernel devs to backport the ceph kernel module into 2.6.33?  Seeing as it made it into 2.6.34, it should fit Fedora's requirements for adding modules to the kernel.  Anyhow, just a thought.

Comment 17 Josef Bacik 2010-04-26 18:43:54 UTC
Alrighty figured out the libhadoop weirdness.  Apparently if you have java installed it will build, but if you don't it won't.  Since I don't have java installed I wasn't getting libhadoop and you were.  As soon as I figure out which package is needed for it to work I'll add that as a BR.

As for backporting CEPH, I aggravate the kernel people enough by backporting btrfs stuff, so I'm going to leave things as they are, and then when we ship a 2.6.34 kernel it will all work fine, and in the meantime people can use sage's git tree to get the kernel client.  Plus, you can always use the fuse client.

Comment 18 Neil Horman 2010-04-28 01:12:40 UTC
Josef, I was setting up a ceph server here and had to figure this out.  shockingly, java requires some hacks in the spec file to package it properly.  You need this patch:

--- ceph.spec	2010-04-26 09:56:54.000000000 -0400
+++ ceph.spec.new	2010-04-27 21:03:37.116861651 -0400
@@ -10,7 +10,7 @@
 Patch0:        ceph-init-fix.patch
 BuildRequires: fuse-devel, libtool, libtool-ltdl-devel, boost-devel, 
 BuildRequires: libedit-devel, fuse-devel, git, perl, perl-devel, gdbm,
-BuildRequires: openssl-devel
+BuildRequires: openssl-devel, java-devel
 BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 Requires(post): chkconfig
 Requires(preun): chkconfig
@@ -43,7 +43,10 @@
 
 %build
 ./autogen.sh
-%configure
+for i in -I/usr/lib/jvm/java/include{,/linux}; do
+	java_inc="$java_inc $i"
+done
+%configure CPPFLAGS="$java_inc"
 make CFLAGS="$RPM_OPT_FLAGS"
 
 %install


I derived it from here:
http://osdir.com/ml/fedora-packaging/2009-05/msg00061.html

Comment 19 Josef Bacik 2010-04-28 13:20:03 UTC
Ahh perfect thanks Neil, thats a bit cleaner than what I had come up with.

Comment 20 Jason Tibbitts 2010-04-29 02:07:57 UTC
CVS done (by process-cvs-requests.py).

Comment 21 Fabian Deutsch 2010-05-06 11:44:27 UTC
@Josef , is the spec landing in CVS at some point? :)

Comment 22 Josef Bacik 2010-05-06 13:11:52 UTC
Yeah, I've had to fiddle with things to get it working nice for 0.20.  I plan to add it to CVS tomorrow (since my fedora box is at home and i'm at work atm).

Comment 23 Fabian Deutsch 2010-05-06 13:23:36 UTC
Ah okay, nice. Thanks.

Comment 24 Orion Poplawski 2012-03-22 17:28:12 UTC
I'd like to see this in EPEL6.  I can maintain it there if the current maintainers don't want to.  It builds and install fine.

http://koji.fedoraproject.org/koji/taskinfo?taskID=3923067

Comment 25 Josef Bacik 2013-02-26 16:11:51 UTC
Package Change Request
======================
Package Name: ceph
New Branches: el6
Owners: josef

Comment 26 Gwyn Ciesla 2013-02-27 14:22:01 UTC
Git done (by process-git-requests).


Note You need to log in before you can comment on or make changes to this bug.