[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 #55 from LINBIT <partner at linbit.com>  2009-10-20 14:13:27 EDT ---
(In reply to comment #53)
> 
> * -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?

Since you are evidently referring to a udev-enabled target system, then yes it
is, and it's completely harmless. The /dev/drbdX device nodes would simply be
created by udev on demand. It is merely the convenience symlinks
(/dev/drbd/by-res/<resourcename>) that would not be created.


>     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" )?

No, for the same reason as stated above. I repeat, our udev integration scripts
are not essential, in any way whatsoever, for DRBD's functionality. They are
merely provided for the user's convenience.

This is actually a typical situation where it would be extremely handy to have
universal Suggests or Recommends tags for the packages, as discussed in the
recent RPM summit (if I'm not misinformed).


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

I got the idea from the cluster-glue package that has just very recently passed
package review and has been accepted into Fedora, which uses the exact approach
of invoking %configure from %prep. Putting %configure into %prep seems more
logical to me than %build.


> * %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.

OK, will be fixed.


> * 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.

Hm. That usage-count thing should really be silenced for service status. Need
to talk to Phil and Lars about that.


>     ! 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.

Point taken.


>     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.  

I see. Will be fixed.

-- 
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.