[Bug 527488] Review Request: drbd - drbd tools

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=527488





--- Comment #53 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp>  2009-10-20 12:46:58 EDT ---
For -10:

* Source tarball
  - in your srpm differs from what I could download from the URL
    written as Source in your spec:
------------------------------------------------------------
426483 2009-10-06 22:26 drbd-8.3.4.tar.gz
450560 2009-10-20 03:03 drbd-8.3.4-10.src/drbd-8.3.4.tar.gz
------------------------------------------------------------

* -utils <-> -udev dependency
  - utils subpackage contains %post scriptlets:
------------------------------------------------------------
%post utils
chkconfig --add drbd
%if %{without udev}
for i in `seq 0 15` ; do
    test -b /dev/drbd$i || mknod -m 0660 /dev/drbd$i b 147 $i;
done
%endif #without udev
------------------------------------------------------------
    I wonder what happens if udev option is enabled (which
    is the default of this srpm), however sysadmin only installs
    -utils subpackage but does not install -udev subpackage (which
    is possible, according to this spec file).
    In this case udev related script is not installed, and
    /dev/drbd* devices are not created either. Is this expected?

    And, if udev option is enabled (which is the default), does
    this mean that -utils subpackage requires -udev subpackage
    at %post (i.e. -utils subpackage should have
    "Requires(post): %{name}-udev" )?

* %prep <-> %build
  - Forgot to mention that we usually write %configure in
    %build, not in %prep.

* %files
-------------------------------------------------------------
%defattr(-,root,root,-)
%dir %{_var}/lib/%{name}
%config(noreplace) %{_sysconfdir}/drbd.conf

%defattr(-,root,root,-)
%{_mandir}/man8/drbd.8.*
-------------------------------------------------------------
  - The second %defattr is not needed.

* drbd service
-------------------------------------------------------------
[root at localhost Binary]# service drbd status

                --== This is a new installation of DRBD ==--
Please take part in the global DRBD usage count at http://usage.drbd.org.

The counter works anonymously. It creates a random number to identify
your machine and sends that random number, along with 
DRBD's version number, to usage.drbd.org.

The benefits for you are:
 * In response to your submission, the server (usage.drbd.org) will tell you
   how many users before you have installed this version (8.3.4).
 * With a high counter LINBIT has a strong motivation to
   continue funding DRBD's development.
...
...
-------------------------------------------------------------
  - What is this? "service status" should just return the status
    of the service and should not try to execute some other installation
    process.

    ! For reference:
     
https://fedoraproject.org/wiki/Packaging/SysVInitScript#Exit_Codes_for_the_Status_Action
      This is just a reference, not following the above exit status
      is not a blocker for the current review, however anyway "service ...
status"
      should just return the status.

    Similarly, "service drbd start" should just fail abnormally if some needed
    initialization has not been completed (i.e. "start" command should just
    try to start daemon), for example.

* %changelog
  - Adding %{?dist} at the end of EVR in %changelog is not needed.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.