Bug 220185
Summary: | Review Request: kvm - Kernel Based Virtual Machine | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Jeremy Katz <katzj> |
Component: | Package Review | Assignee: | David Cantrell <dcantrell> |
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | avi, panemade |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-01-05 19:09:24 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Jeremy Katz
2006-12-19 16:11:14 UTC
I love packages with kernel word. Anyway i tried to build this package in mock build environment but build failed with make[1]: Entering directory `/builddir/build/BUILD/kvm-7/qemu' gcc34 -DQEMU_TOOL -I /builddir/build/BUILD/kvm-7/qemu/../user -Wall -O2 -g -fno-strict-aliasing -I. -g -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -o qemu-img qemu-img.c block.c block-cow.c block-qcow.c aes.c block-vmdk.c block-cloop.c block-dmg.c block-bochs.c block-vpc.c block-vvfat.c -lz -luuid block-vmdk.c:25:23: uuid/uuid.h: No such file or directory block-vmdk.c: In function `vmdk_snapshot_create': block-vmdk.c:194: error: `uuid_t' undeclared (first use in this function) block-vmdk.c:194: error: (Each undeclared identifier is reported only once block-vmdk.c:194: error: for each function it appears in.) block-vmdk.c:194: error: syntax error before "name" block-vmdk.c:211: warning: implicit declaration of function `uuid_generate' block-vmdk.c:211: error: `name' undeclared (first use in this function) block-vmdk.c:212: warning: implicit declaration of function `uuid_unparse' block-vmdk.c: In function `vmdk_open': block-vmdk.c:420: warning: passing arg 1 of `dirname' discards qualifiers from pointer target type make[1]: *** [qemu-img] Error 1 make[1]: Leaving directory `/builddir/build/BUILD/kvm-7/qemu' make: *** [qemu] Error 2 error: Bad exit status from /var/tmp/rpm-tmp.21186 (%build) Looks to me you need to add e2fsprogs-devel as BuildRequires. Indeed -- that'll teach me to trust based on inspection for deps. Build actually tested in mock now (after setting up my new workstation with a sane config) and it builds fine on x86_64 in mock with that buildreq. Updated spec file in the same location, and http://people.redhat.com/~katzj/kvm/kvm-7-2.src.rpm is there I will review this. mock build for i386 successfull but rpmlint is not silent You SHOULD: - Add disttag {?dist} to release tag - Add direct download link of any mirror of sf.net say http://jaist.dl.sourceforge.net/sourceforge/kvm/kvm-7.tar.gz - Add COPYRIGHT file under %doc - You may add test directory under %doc Kindly check against rpmlint command both RPM and SRPM and resubmit package. (In reply to comment #3) > - Add disttag {?dist} to release tag Disttag ist still optional > - Add direct download link of any mirror of sf.net > say http://jaist.dl.sourceforge.net/sourceforge/kvm/kvm-7.tar.gz Using download.sourceforge.net is prefered: http://download.sourceforge.net/sourceforge/kvm/kvm-7.tar.gz That will redirect to a mirror. (In reply to comment #4) > (In reply to comment #3) > > - Add disttag {?dist} to release tag > > Disttag ist still optional Ohh sorry, i thought asking reporter for adding disttag is essential. Will take care from next time for not to ask anyone to add disttag. > > > - Add direct download link of any mirror of sf.net > > say http://jaist.dl.sourceforge.net/sourceforge/kvm/kvm-7.tar.gz > > Using download.sourceforge.net is prefered: > http://download.sourceforge.net/sourceforge/kvm/kvm-7.tar.gz > That will redirect to a mirror. Ohh sorry, actually i was asked same for my own v4l2-tool package review, so similarly i asked here. Information given on kvm.sf.net said "KVM (for Kernel-based Virtual Machine) is a full virtualization solution for Linux on x86 hardware. It consists of a loadable kernel module (kvm.ko) and a userspace component." IMHO, Either you need to submit KMOD package for kvm.ko for kernel versions lesser than 2.6.20 if this module supports existing FC6/devel kernels or this package needs dependency to be set for kernel's > 2.6.20 Kindly ignore my comment for disttag and Source URL and use http://download.sourceforge.net/sourceforge/kvm/kvm-7.tar.gz as Source URL Also static libraries need FESCO approval. So I request you to ask FESCO for permission to include .a files ot drop -devel package. (In reply to comment #3) > mock build for i386 successfull but rpmlint is not silent rpmlint errors are bogus -- E: kvm configure-without-libdir-spec The configure script isn't a standard autoconf configure and so doesn't take a libdir argument W: kvm no-documentation Upstream doesn't really include any documentation in the tarball > You SHOULD: > - Add disttag {?dist} to release tag disttag isn't required. > - Add direct download link of any mirror of sf.net > say http://jaist.dl.sourceforge.net/sourceforge/kvm/kvm-7.tar.gz Fixed, but with the link from the other comment > - Add COPYRIGHT file under %doc There's not an explicit COPYRIGHT for the top-level and adding more copies of the GPL to the distro doesn't help anyone. > - You may add test directory under %doc This doesn't make any sense at all; tests aren't documentation. (In reply to comment #6) > IMHO, Either you need to submit KMOD package for kvm.ko for kernel versions > lesser than 2.6.20 if this module supports existing FC6/devel kernels or this > package needs dependency to be set for kernel's > 2.6.20 The package is only going to be submitted for devel (where rawhide has 2.6.20-rc). And a requires on a "new" kernel isn't the answer as a user can trivially build the module if they really don't want to bump versions. Also, the requires doesn't mean that the user is actually running said kernel. The more right answer is that as tools get built up, they need to check and make sure the kernel provides the functionality, much like tools for xen need to ensure that they're running on a Xen dom0. > Also static libraries need FESCO approval. So I request you to ask FESCO for > permission to include .a files ot drop -devel package. I don't think that's actually been fully passed yet, but I don't care enough to keep the library around for now. Nuking -devel. http://people.redhat.com/~katzj/kvm/kvm-7-3.src.rpm and http://people.redhat.com/~katzj/kvm/kvm.spec updated Suggest calling the binary 'kvm' instead of qemu-kvm, and adding a softlink to the qemu manual page. Alternatively, merge it to the qemu package. That's perhaps a little premature at this moment. Mock build failed. (In reply to comment #8) > Suggest calling the binary 'kvm' instead of qemu-kvm, and adding a softlink to > the qemu manual page. Not calling the binary kvm due to potential confusion with the little kvm wrapper script in the tarball; although I'm somewhat interested in getting the script a little bit more suitable so that I can just include it. But I'll do that via kvm-devel > Alternatively, merge it to the qemu package. That's perhaps a little > premature at this moment. Yeah, seems a little early still. Once everything is upstream, I'll be more than glad to have qemu obsolete this (although if there are some useful tools around using qemu+kvm, then maybe this just becomes the tools package :-) (In reply to comment #9) > Mock build failed. Apparently, I can't type/save files when I'm actually done with them... http://people.redhat.com/~katzj/kvm/kvm-7-4.src.rpm and http://people.redhat.com/~katzj/kvm/kvm.spec updated. For real this time! This now builds and looks good, I would approve it. Ok, approving, sorry Parag but we want this in soon. Thanks Parag and Jesse. Built for extras-development Jesse and Jeremy, Sorry i could not get time to approve this package. I don't mind Jesse that you approved this package :) |