https://bugzilla.redhat.com/show_bug.cgi?id=866495
Bug ID: 866495 QA Contact: extras-qa@fedoraproject.org Severity: medium Version: rawhide Priority: unspecified CC: notting@redhat.com, package-review@lists.fedoraproject.org Assignee: nobody@fedoraproject.org Summary: Review Request: vzctl - OpenVZ containers Regression: --- Story Points: --- Classification: Fedora OS: Linux Reporter: glommer@gmail.com Type: --- Documentation: --- Hardware: All Mount Type: --- Status: NEW Component: Package Review Product: Fedora
Spec URL: http://glommer.net/vzctl-fedora/vzctl.spec SRPM URL: http://glommer.net/vzctl-fedora/vzctl-4.0-1.fc17.src.rpm Description: This package will provide OpenVZ support for Fedora.
Although OpenVZ historically required a modified kernel - and still do, for full functionality - version 4.0 gained the ability to run it ontop of Upstream Linux with limited functionality - as much as the underlying kernel will allow.
Fedora users should be able to start a container with networking enabled, and ssh to it.
Fedora Account System Username: glauber
https://bugzilla.redhat.com/show_bug.cgi?id=866495
Glauber Costa glommer@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |177841 (FE-NEEDSPONSOR)
https://bugzilla.redhat.com/show_bug.cgi?id=866495
Peter Lemenkov lemenkov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |lemenkov@gmail.com
--- Comment #1 from Peter Lemenkov lemenkov@gmail.com --- I already sponsored Glauber so I'm going to do it again.
https://bugzilla.redhat.com/show_bug.cgi?id=866495
Peter Lemenkov lemenkov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On|177841 (FE-NEEDSPONSOR) |
--- Comment #2 from Peter Lemenkov lemenkov@gmail.com --- Lifting FE-NEEDSPONSOR - I've just sponsored Glauber Costa.
https://bugzilla.redhat.com/show_bug.cgi?id=866495
Michael Scherer misc@zarb.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |misc@zarb.org Flags| |fedora-review?
--- Comment #3 from Michael Scherer misc@zarb.org --- Hi, a few remark : - no need for BuildRoot
- no need for rm -rf $RPM_BUILD_ROOT in %install ( done by default )
- this is also done by default %defattr(-,root,root)
- adding %attr(755,root,root) to all directory is useless, since that's the default setting
- this is likely incorrect, 777 is quite dangerous in fact %attr(777, root, root) /etc/vz/conf
- why is there such requires : Requires: /sbin/chkconfig
- Requires: libxml2 this is likly autodetected
- License: GPL this is incorrect, you need to explicit the version of the GPL
- the initrd script should be replaced by a systemd file
- why does it create a directory in / ? I think this would be blocker since we have tried to reduce cruft there, and not increase it.
- %description is quite misleading, this doesn't permit to manage linux container, but openvz container.
- I think there is also some grammar/typo fixes to add : This utility allows system administator to control Linux containers,
https://bugzilla.redhat.com/show_bug.cgi?id=866495
Michael Scherer misc@zarb.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |misc@zarb.org
https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #4 from Glauber Costa glommer@gmail.com --- Hello Michael, thank you very much.
Some questions: 1 ) Why would libxml2 be autodetected?
2 ) Note that I am not installing any initrd script. OpenVZ installs it because it is needed when running it fully-featured ontop of non-upstream kernels. It will dump everything in its "make install", but it is only to be seen in my package in the %exclude directive. Isn't this alright ? Or do you mean something else?
3) About the directory in /, I believe you mean /vz, right? This is unfortunately hard-coded into a lot of places in OpenVZ, but I fully understand the objection. I'll work to patch it and replace it. Meanwhile, it would be good if we could keep the review going, aside from that point.
I'll post a new .spec shortly once I understand better the questions I posted in 1) and 2).
https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #5 from Glauber Costa glommer@gmail.com --- BTW, I believe I can get rid of the libxml dependency as well. Strictly speaking, it is only used when a subpackage that I am not packaging here is used.
I never tested, though. I will, and if everything goes right, I can just remove that dependency.
https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #6 from Michael Scherer misc@zarb.org --- 1) because rpm detect the dynamic library used by various binary, using ldd. So no need to explicitly add this one.
2) I think it may be better to remove in %install than adding lots of thing in %files ( in fact, this make it easier to review ), but that's not blocking
3) yes, i speak of /vz. But I can continue looking for others issues.
I guess one way to have this pushed upstream would be to have it non hardcoded ( ie, using a prefix ) and then you will only have 1 path to change.
https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #7 from Glauber Costa glommer@gmail.com --- Thanks Again, Michael,
I am already looking at using a prefix for that in upstream OpenVZ.
I'll publish a new version with your other comments merged shortly
https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #8 from Glauber Costa glommer@gmail.com --- Hi,
I've pushed new versions of the files to their old location:
Spec URL: http://glommer.net/vzctl-fedora/vzctl.spec SRPM URL: http://glommer.net/vzctl-fedora/vzctl-4.0-1.fc17.src.rpm
I left the changelog and versions alone, because this is not released yet. Please tell me if it was supposed to have been bumped.
https://bugzilla.redhat.com/show_bug.cgi?id=866495
Ralf Corsepius rc040203@freenet.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |rc040203@freenet.de
--- Comment #9 from Ralf Corsepius rc040203@freenet.de --- (In reply to comment #8)
I left the changelog and versions alone, because this is not released yet. Please tell me if it was supposed to have been bumped.
You are supposed to add changelog entries for each change you make and to bump the release tag each time you update your package.
https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #10 from Glauber Costa glommer@gmail.com --- I believed this would only apply to packages already released, not in review stage. Sorry.
I've updated the changelog now:
Spec URL: http://glommer.net/vzctl-fedora/vzctl.spec SRPM URL: http://glommer.net/vzctl-fedora/vzctl-4.0-2.fc17.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #11 from Ralf Corsepius rc040203@freenet.de --- (In reply to comment #10)
I believed this would only apply to packages already released, not in review stage. Sorry.
No, this also applies to packages under review, because this tremendiously helps reviewers, as it e.g. helps reviewers to destinguish the versions.
https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #12 from Glauber Costa glommer@gmail.com --- Hi.
OpenVZ just released a new version that makes the /vz path configurable, as requested by Fedora. I changed it to /var/lib/vz, following what other packages like libvirt does for their images.
The updated spec and srpm can be both found at:
http://glommer.net/vzctl-fedora/v3/
Thanks.
https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #13 from Glauber Costa glommer@gmail.com --- Hello, any further comments here ?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
Peter Lemenkov lemenkov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|misc@zarb.org |lemenkov@gmail.com
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
Peter Lemenkov lemenkov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #14 from Peter Lemenkov lemenkov@gmail.com --- I'll review it.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
Peter Lemenkov lemenkov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? | Flags| |fedora-review+
--- Comment #15 from Peter Lemenkov lemenkov@gmail.com --- REVIEW:
Legend: + = PASSED, - = FAILED, 0 = Not Applicable
+ rpmlint is NOT silent. Apart from incorrect-fsf-address messages which are not that critical (however I advise you to update licensing headers) here are few others:
work ~/Desktop: rpmlint vzctl-* | grep -v incorrect-fsf-address vzctl-core.x86_64: W: conffile-without-noreplace-flag /etc/sysconfig/vz-scripts/ve-vswap-1g.conf-sample vzctl-core.x86_64: W: conffile-without-noreplace-flag /etc/sysconfig/vz-scripts/ve-vswap-1024m.conf-sample vzctl-core.x86_64: W: conffile-without-noreplace-flag /etc/sysconfig/vz-scripts/ve-vswap-4g.conf-sample vzctl-core.x86_64: W: conffile-without-noreplace-flag /etc/sysconfig/vz-scripts/ve-basic.conf-sample vzctl-core.x86_64: W: conffile-without-noreplace-flag /etc/sysconfig/vz-scripts/ve-vswap-512m.conf-sample vzctl-core.x86_64: W: conffile-without-noreplace-flag /etc/sysconfig/vz-scripts/ve-light.conf-sample vzctl-core.x86_64: W: conffile-without-noreplace-flag /etc/sysconfig/vz-scripts/ve-unlimited.conf-sample vzctl-core.x86_64: W: conffile-without-noreplace-flag /etc/sysconfig/vz-scripts/0.conf vzctl-core.x86_64: W: conffile-without-noreplace-flag /etc/sysconfig/vz-scripts/ve-vswap-256m.conf-sample vzctl-core.x86_64: W: conffile-without-noreplace-flag /etc/sysconfig/vz vzctl-core.x86_64: W: conffile-without-noreplace-flag /etc/sysconfig/vz-scripts/ve-vswap-2g.conf-sample vzctl-core.x86_64: W: non-conffile-in-etc /etc/vz/dists/distribution.conf-template
^^^ see below.
vzctl-core.x86_64: E: non-standard-dir-perm /var/lib/vz/private 0700L vzctl-core.x86_64: E: non-standard-dir-perm /var/lib/vz/root 0700L
^^^ I suspect that's a requrement for VZ.
vzctl-core.x86_64: W: non-conffile-in-etc /etc/bash_completion.d/vzctl.sh
^^^ see below.
vzctl-core.x86_64: E: incoherent-logrotate-file /etc/logrotate.d/vzctl
^^^ That's ok. It just warns you that package name doesn't match logrotate's file name.
vzctl-core.x86_64: W: non-conffile-in-etc /etc/logrotate.d/vzctl vzctl-core.x86_64: W: non-conffile-in-etc /etc/vz/dists/default
^^^ see below.
3 packages and 0 specfiles checked; 161 errors, 15 warnings. work ~/Desktop:
Regarding non-conffile-in-etc messages. There are two possible scenarios of installing files which are intended to be edited by user. We either can install them as is and mark them as conffiles (in this case RPM won't override them if they already exists) or we can install them as *.template / *.example / *.sample / etc, and require user to create them explicitly using provided ones as an examples.
You apparently choose the second option. And that's why rpmlint complaints a lot.
+ The package is named according to the Package Naming Guidelines. + The spec file name matches the base package %{name}, in the format %{name}.spec. + The package meets the Packaging Guidelines. + The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
- The License field in the package spec file MUST match the actual license. Correct tag is GPLv2+.
+ The file, containing the text of the license(s) for the package, is included in %doc. + The spec file is written in American English. + The spec file for the package is legible. + The sources used to build the package, match the upstream source, as provided in the spec URL.
sulaco ~/rpmbuild/SOURCES: sha256sum vzctl-4.1.tar.bz2* a77a9b4db34259ca9ded9d0e869dc8d0a65b2534530a57c79fb284b9745a2f7d vzctl-4.1.tar.bz2 a77a9b4db34259ca9ded9d0e869dc8d0a65b2534530a57c79fb284b9745a2f7d vzctl-4.1.tar.bz2.1 sulaco ~/rpmbuild/SOURCES:
+ The package successfully compiles and builds into binary rpms on at least one primary architecture.
* http://koji.fedoraproject.org/koji/taskinfo?taskID=4690929
+ All build dependencies are listed in BuildRequires. 0 No need to handle locales. + The package stores shared library files in some of the dynamic linker's default paths, and it calls ldconfig in %post and %postun. + The package does NOT bundle copies of system libraries. 0 The package is not designed to be relocatable.
- The package MUST own all directories that it creates. You must claim ownership on the following directories:
* %{_vzdir}/template/ * /var/lib/vzctl/
+ The package does not list a file more than once in the spec file's %files listings. + Permissions on files are set properly. 0 The package DOESN'T have a %clean section, so it won't build cleanly on systems with old rpm (EL-4 and EL-5, not sure about EL-6). But this package requires a very modern kernels so it won't work on older kernels anyway. + The package contains code, or permissible content. 0 No extremely large documentation files. + Anything, the package includes as %doc, does not affect the runtime of the application. 0 No C/C++ header files. 0 No static libraries. 0 No pkgconfig(.pc) files. 0 The package doesn't contain library files without a suffix (e.g. libfoo.so) in some of the dynamic linker's default paths. 0 No devel sub-package. + The package does NOT contain any .la libtool archives. 0 Not a GUI application. + The package does not own files or directories already owned by other packages. 0 At the beginning of %install, the package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) so it won't build cleanly on systems with old rpm (EL-4 and EL-5, not sure about EL-6). The same as with %clean section - it's irrelevant for this case. + All filenames in rpm packages are valid UTF-8.
So, please, before package uploading and building
* correct License tag * claim ownership on two additional libraries
Apart of that I don't see any other issues so this package is
APPROVED.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #16 from Glauber Costa glommer@gmail.com --- Before I posted this, rpmlint was already silent for me. It still is, even after I upgrade it to F19's, grabbed from Koji.
I am not sure why this is the case =(
In any case: The license is changed to gplv2+, and I claimed ownership over those two extra directories.
I also tried to address the other problems by moving some of the files from standard %{files} sections - but I can't verify the end rpmlint result.
In any case, I've put the updated files at:
http://glommer.net/vzctl-fedora/v4/
Thanks.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #17 from Peter Lemenkov lemenkov@gmail.com --- You have a go already. You should continue from this:
* https://fedoraproject.org/wiki/Package_SCM_admin_requests
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
Glauber Costa glommer@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #18 from Glauber Costa glommer@gmail.com --- New Package SCM Request ======================= Package Name: vzctl Short Description: utility that allows system administrators to control OpenVZ containers Owners: glauber Branches: f17 f18
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #19 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
Glauber Costa glommer@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |NEXTRELEASE Last Closed| |2012-11-16 03:37:41
--- Comment #20 from Glauber Costa glommer@gmail.com --- Closing as NEXTRELEASE, according to step 12 in https://fedoraproject.org/wiki/New_package_process_for_existing_contributors
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #21 from Fedora Update System updates@fedoraproject.org --- vzctl-4.1-2.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/vzctl-4.1-2.fc17
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #22 from Fedora Update System updates@fedoraproject.org --- vzctl-4.1-2.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/vzctl-4.1-2.fc18
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
Dan Horák dan@danny.cz changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |dan@danny.cz
--- Comment #23 from Dan Horák dan@danny.cz --- What architectures are supported by openvz.org or vzctl? I can see some #ifdefs in include/vzsyscalls.h but do they mean vzctl really works there?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #24 from Glauber Costa glommer@gmail.com --- vzsyscalls.h is the entry point for the OpenVZ kernel, that we don't use in Fedora. (We do keep the package in its full, it would be a lot more trouble to strip it).
As for the upstream kernel, any architecture that has full cgroup + namespace support should work. So although I will be the first to admit that this is only tested in x86 and x86_64, there should not be any fundamental problems in running it in other architectures. We would only need to test it, and in case it blows, fix it.
In the near future, I will introduce container migration through the CRIU project. (http://criu.org). That specific functionality will be available for x86_64 only. But it is also the goal of the criu project to have it supported in multiple architectures. So this is a temporary condition.
Thanks.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #25 from Dan Horák dan@danny.cz --- Then it would be nice if vzctl could build also on arches without the OpenVZ kernel, right now it fails on s390(x) with (http://s390.koji.fedoraproject.org/koji/taskinfo?taskID=857522)
... In file included from cpu.c:31:0: ../../include/vzsyscalls.h:66:2: error: #error "no syscall for this arch" cpu.c: In function 'fairsched_chwt': cpu.c:37:16: error: '__NR_fairsched_chwt' undeclared (first use in this function) cpu.c:37:16: note: each undeclared identifier is reported only once for each function it appears in cpu.c: In function 'fairsched_rate': cpu.c:47:16: error: '__NR_fairsched_rate' undeclared (first use in this function) cpu.c: In function 'fairsched_vcpus': cpu.c:57:16: error: '__NR_fairsched_vcpus' undeclared (first use in this function) make[2]: *** [cpu.lo] Error 1
and same error should occur on ARM and PPC64. Nothing critical, I've excluded vzctl in s390 build system now, the other secondary arch maintainers will do the same, but nice to have in the future I think.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #26 from Glauber Costa glommer@gmail.com --- Ok, I am working in a patch to it right now.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #27 from Dan Horák dan@danny.cz --- (In reply to comment #26)
Ok, I am working in a patch to it right now.
Thanks, please ping us on #fedora-{arm,ppc,s390x} if you'll need access to the HW (no public s390x AFAIK).
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #28 from Fedora Update System updates@fedoraproject.org --- vzctl-4.1-2.fc18 has been pushed to the Fedora 18 stable repository.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=866495
--- Comment #29 from Fedora Update System updates@fedoraproject.org --- vzctl-4.1-2.fc17 has been pushed to the Fedora 17 stable repository.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla