Bug 521319 - Review Request: virt-v2v - Convert a virtual machine to run on KVM
Review Request: virt-v2v - Convert a virtual machine to run on KVM
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Richard W.M. Jones
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-09-04 14:26 EDT by Matthew Booth
Modified: 2013-10-19 10:42 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-06-08 09:06:49 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rjones: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Matthew Booth 2009-09-04 14:26:52 EDT
Spec URL: http://people.redhat.com/mbooth/virt-v2v/virt-v2v.spec
SRPM URL: http://people.redhat.com/mbooth/virt-v2v/virt-v2v-0.1.0-1.fc11.src.rpm
Description:
virt-v2v is a tool for converting virtual machines to use the KVM hypervisor.
It modifies both the virtual machine image and its associated libvirt metadata.
virt-v2v will also configure a guest to use VirtIO drivers if possible.

virt-v2v also includes the virt-snapshot tool, which will modify a guest to use
snapshot storage, and later commit the snapshot back to the original storage if
desired.
----
This is my first Fedora package and I am seeking a sponsor.
I am the primary developer of this application.
The package currently only builds against dist-f12.
I'm targeting EPEL-5, but there are outstanding dependency issues.
Comment 1 Richard W.M. Jones 2009-09-07 05:23:14 EDT
Taking for review.
Comment 2 Richard W.M. Jones 2009-09-07 11:02:27 EDT
It builds, but you need to add a libguestfs version
dep.  The current dep:

BuildRequires:  perl(Sys::Guestfs::Lib) >= 1.0.68

doesn't work.  It seems that perl() deps are unversioned
and therefore rpm will happily accept any version.
To make it fail properly on my old version of libguestfs
I had to add:

BuildRequires:  libguestfs >= 1:1.0.68
 ...
Requires:       libguestfs >= 1:1.0.68

Note also the epoch.

---

After making the above change (and installing the
requisite version of libguestfs), I was able to build
it locally.
Comment 3 Richard W.M. Jones 2009-09-07 11:08:47 EDT
Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1660335

rpmlint is silent.
Comment 4 Richard W.M. Jones 2009-09-07 11:20:15 EDT
+ rpmlint output
+ package name satisfies the packaging naming guidelines

Reporter doesn't think the Perl libraries are independently
useful, so they don't need to go in a separate package
and we don't need to follow the Perl naming guidelines
for that too closely.

+ specfile name matches the package base name
+ package should satisfy packaging guidelines

(See above about Perl packaging guidelines, although the
package is broadly correct even for them).

+ 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

http://koji.fedoraproject.org/koji/taskinfo?taskID=1660335

n/a ExcludeArch bugs filed
+ BuildRequires list all build dependencies

Koji build proves this.

+ %find_lang instead of %{_datadir}/locale/*

Although commented out at the moment, however this is
correct for this package.

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
+ %defattr line
+ %clean contains rm -rf $RPM_BUILD_ROOT
+ consistent use of macros
+ 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
+ %install must start with rm -rf %{buildroot} etc.
+ filenames must be valid UTF-8

Optional:

+ 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
n/a the package should build into binary RPMs on all supported architectures
+ review should test the package functions as described
n/a scriptlets should be sane
n/a pkgconfig files should go in -devel
+ shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or /usr/sbin

========== APPROVED by rjones ==========

The only thing to do is to modify the spec file as in
comment 2.
Comment 5 Ralf Corsepius 2009-09-07 11:35:22 EDT
(In reply to comment #2)
> It builds, but you need to add a libguestfs version
> dep.  The current dep:
> 
> BuildRequires:  perl(Sys::Guestfs::Lib) >= 1.0.68
> 
> doesn't work.  It seems that perl() deps are unversioned
Untrue. perl(..) are referring to a perl-module's version, not to the rpm's or the package's versions.

If a perl-module doesn't carry a version (from Perl's perspective), this perl module's version's rpm-representation, i.e. the perl(..) dep is unversioned.
Comment 6 Richard W.M. Jones 2009-09-07 11:55:20 EDT
(In reply to comment #5)
> (In reply to comment #2)
> > It builds, but you need to add a libguestfs version
> > dep.  The current dep:
> > 
> > BuildRequires:  perl(Sys::Guestfs::Lib) >= 1.0.68
> > 
> > doesn't work.  It seems that perl() deps are unversioned
> Untrue. perl(..) are referring to a perl-module's version, not to the rpm's or
> the package's versions.
> 
> If a perl-module doesn't carry a version (from Perl's perspective), this perl
> module's version's rpm-representation, i.e. the perl(..) dep is unversioned.  

OK, understood.  I opened a libguestfs bug to fix this:

https://bugzilla.redhat.com/show_bug.cgi?id=521674
Comment 7 Matthew Booth 2009-09-08 04:36:26 EDT
New Package CVS Request
=======================
Package Name: virt-v2v
Short Description: Convert a virtual machine to run on KVM
Owners: mdbooth 
Branches: F-11 F-12 EL-5
InitialCC:
Comment 8 Kevin Fenzi 2009-09-09 12:19:33 EDT
Matthew: I don't see you in the packager group. Is this your first package?
Comment 9 Matthew Booth 2009-09-09 12:30:45 EDT
Yes, it is.
Comment 10 Richard W.M. Jones 2009-09-09 12:50:41 EDT
So Matt needs sponsorship, is that right?
Comment 11 Matthew Booth 2009-09-10 06:00:53 EDT
I'm now a member of the packager group. Rich is my sponsor.
Comment 12 Richard W.M. Jones 2009-09-11 07:52:47 EDT
(In reply to comment #8)
> Matthew: I don't see you in the packager group. Is this your first package?  

Kevin, is there anything else which Matt or I need to
do in order to get this package added to CVS?  I *think*
I've followed all the relevant rules from the wiki...
Comment 13 Jason Tibbitts 2009-09-11 16:20:26 EDT
These are done by hand and occasionally it takes a bit for a human to find the time to run through the queue.

CVS done.
Comment 14 Mark McLoughlin 2009-09-21 10:21:40 EDT
Can we get virtmaint added to cvs commit and bugzilla cc ? Thanks

Package Change Request
======================
Package Name: virt-v2v
InitialCC: virtmaint
Comment 15 Matthew Booth 2009-09-21 18:26:38 EDT
Mark,

I'm new to this. Do I have to do anything to make this happen? I don't see any flags set on the BZ, so I guess I do. Any idea what?
Comment 16 Kevin Fenzi 2009-09-21 22:17:28 EDT
For cvs requests, you need to set the fedora-cvs flag to ? 

Since I was already CC'ed here, I went ahead and processed the request. ;) 
cvs done.
Comment 17 Richard W.M. Jones 2010-06-08 09:06:49 EDT
I'm pretty sure this bug should be closed now since
virt-v2v has been in Fedora for quite a long time.

Note You need to log in before you can comment on or make changes to this bug.