[Bug 745510] Review Request: vdsm - Virtual Desktop Server Manager

Wed Nov 16 11:29:05 UTC 2011

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=745510

--- Comment #16 from Richard W.M. Jones <rjones at redhat.com> 2011-11-16 06:29:03 EST ---
Thanks for correcting all of the problems that I found before.

The Source0 file appears to be generated from git.  This is fine,
but it would be better to have a comment stating how to regenerate
this file.  I thought this was required by the review guidelines,
but I cannot find anything that says that now; therefore this is
not a review blocker.

Here is the rest of my package review:

- rpmlint output

vdsm.src: W: invalid-url Source0: vdsm-4.9.1.tar.gz
vdsm.x86_64: E: explicit-lib-dependency cyrus-sasl-lib

Not quite sure what rpmlint is on about here.  The dependency seems OK
to me.

vdsm.x86_64: W: incoherent-version-in-changelog 4.9.1-0 
['4.9.1-0.git31.039976c.fc16', '4.9.1-0.git31.039976c']

This should be fixed.

vdsm.x86_64: E: non-readable /etc/sudoers.d/50_vdsm 0440L
vdsm.x86_64: W: non-conffile-in-etc /etc/sudoers.d/50_vdsm

Bug in rpmlint probably.

vdsm.x86_64: E: non-executable-script /usr/share/vdsm/configNetwork.py 0644L
/usr/bin/python

Are the permissions on this script correct?

vdsm.x86_64: E: non-readable /etc/pki/vdsm/keys/libvirt_password 0600L
vdsm.x86_64: E: script-without-shebang /etc/cron.d/vdsm-libvirt-logrotate
vdsm.x86_64: E: executable-crontab-file /etc/cron.d/vdsm-libvirt-logrotate
vdsm.x86_64: E: non-standard-dir-perm /var/lib/libvirt/qemu/channels 0775L

This should probably be 0755 unless you want a group to write to
this directory.

vdsm.x86_64: E: non-executable-script /usr/share/vdsm/supervdsmServer.py 0644L
/usr/bin/python

As above.

vdsm.x86_64: W: non-ghost-in-var-run /var/run/vdsm
vdsm.x86_64: W: non-conffile-in-etc /etc/udev/rules.d/12-vdsm-lvm.rules

I believe this is a bug: udev rules files that are installed from
RPMs ought to go somewhere under /lib/udev/...

vdsm.x86_64: W: non-ghost-in-var-run /var/run/vdsm/pools
vdsm.x86_64: W: service-default-enabled /etc/rc.d/init.d/vdsmd
vdsm.x86_64: E: incoherent-subsys /etc/rc.d/init.d/vdsmd libvirt-guests

Just so you know, all Fedora packages MUST now include systemd
configuration.  SysV-init is not required and can be removed.

vdsm.x86_64: W: service-default-enabled /etc/rc.d/init.d/vdsmd
vdsm-hook-vhostmd.noarch: E: non-readable /etc/sudoers.d/50_vdsm_hook_vhostmd
0440L
vdsm-hook-vhostmd.noarch: W: non-conffile-in-etc
/etc/sudoers.d/50_vdsm_hook_vhostmd
vdsm-debug-plugin.noarch: W: no-documentation
vdsm-hook-faqemu.noarch: W: spelling-error Summary(en_US) qemu -> emu, q emu
vdsm-hook-faqemu.noarch: W: no-manual-page-for-binary vdsm-faqemu
vdsm-cli.noarch: E: non-executable-script /usr/share/vdsm/vdsClient.py 0644L
/usr/bin/python
vdsm-cli.noarch: W: non-conffile-in-etc /etc/bash_completion.d/vdsClient
vdsm-bootstrap.noarch: W: spelling-error %description -l en_US bootstap ->
bootstrap, boots tap, boots-tap
vdsm-bootstrap.noarch: E: non-executable-script
/usr/share/vdsm-bootstrap/vds_bootstrap_complete.py 0644L /usr/bin/python
vdsm-bootstrap.noarch: E: non-executable-script
/usr/share/vdsm-bootstrap/vds_bootstrap.py 0644L /usr/bin/python
vdsm-reg.noarch: E: non-executable-script
/usr/lib/python2.7/site-packages/ovirt_config_setup/rhevm.py 0644L
/usr/bin/python
vdsm-reg.noarch: W: non-conffile-in-etc /etc/ovirt-commandline.d/vdsm-reg
vdsm-reg.noarch: W: service-default-enabled /etc/init.d/vdsm-reg
vdsm-reg.noarch: W: no-reload-entry /etc/init.d/vdsm-reg

Please run rpmlint yourself and look at these errors and warnings.
Some can be ignored, but some look more serious.

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
? package should satisfy packaging guidelines

Fix the rpmlint problems.

+ license meets guidelines and is acceptable to Fedora
+ license matches the actual package license
+ %doc includes license file
+ spec file written in American English
+ spec file is legible
+ upstream sources match sources in the srpm
+ package successfully builds on at least one architecture
  (built on x86-64)
n/a ExcludeArch bugs filed
+ BuildRequires list all build dependencies
  http://koji.fedoraproject.org/koji/taskinfo?taskID=3518406
n/a %find_lang instead of %{_datadir}/locale/*
n/a binary RPM with shared library files must call ldconfig in %post and
%postun
+ does not use Prefix: /usr
+ package owns all directories it creates
+ no duplicate files in %files
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package
n/a header files should be in -devel
n/a static libraries should be in -static
n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
n/a libfoo.so must go in -devel
n/a -devel must require the fully versioned base
n/a packages should not contain libtool .la files
n/a packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ filenames must be valid UTF-8
+ use %global instead of %define

Optional:

n/a if there is no license file, packager should query upstream
n/a translations of description and summary for non-English languages, if
available
+ reviewer should build the package in mock
  http://koji.fedoraproject.org/koji/taskinfo?taskID=3518406
+ the package should build into binary RPMs on all supported architectures
n/a review should test the package functions as described
+ scriptlets should be sane
n/a pkgconfig files should go in -devel
n/a shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or
/usr/sbin

Please take a look at the problems above.

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