[Libguestfs] [PATCH] Initial drop of virt-v2v

[Richard W.M. Jones]

On Fri, Jul 24, 2009 at 10:34:21PM +0100, Matthew Booth wrote:
> diff --git a/perl/lib/Sys/Guestfs/GuestOS/RedHat.pm b/perl/lib/Sys/Guestfs/GuestOS/RedHat.pm
> +=head1 SYNOPSIS
> +
> + use Sys::Guestfs::GuestOS;
> +
> + $guestos = Sys::Guestfs::GuestOS->get_instance($os, $distro, $version)
> +
> +=head1 DESCRIPTION
> +
> +Sys::Guestfs::GuestOS provides a mechanism for querying and manipulating a
> +specific guest operating system.
[...]

This documentation is just duplicated from the parent class.

> +sub enable_driver

Is this an internal function?  Test::Pod::Coverage will complain
unless internal function names are prefixed by underscore.  (It
enforces POD documentation on all external functions).

> +        # XXX: The following save should not be required, but is
> +        # If this save is omitted, by the time save is called just before
> +        # mkinitrd, these changes will have been lost.
> +        $g->aug_save();

We ought to ask David Lutterkort what's going on here, because I'm
quite sure that Augeas isn't supposed to behave like this.

> +sub disable_driver
[...]
> +        $g->aug_rm($augeas);

Missing a path parameter.

> +# We can't rely on the index in the augeas path because it will change if
> +# something has been inserted or removed before it.
> +# Look for the alias again in the same file which contained it on the first
> +# pass.

I'm sure that Augeas must provide a mechanism to name nodes.  Ask
Augeas upstream about it?

> +sub add_kernel
> +{
> +    my $self = shift;
> +    my $kernel_arch = "i386"; # XXX: Need to get this from inspection!

Yup - it's on the TODO list, and does need to be fixed.

> +sub remap_block_devices
> +{
> +    my $self = shift;
> +    my %map = @_;
> +
> +    my $g = $self->{g};
> +
> +    # Iterate over fstab. Any entries with a spec in the the map, replace them
> +    # with their mapped values
> +    eval {
> +        foreach my $spec ($g->aug_match('/etc/fstab/*/spec')) {
> +            my $device = $g->aug_get($spec);
> +            if(exists($map{$device})) {
> +                $g->aug_set($spec, $map{$device});
> +            }
> +        }
> +    };
> +
> +    # Propagate augeas failure
> +    die($@) if($@);
> +}

I'll ask a nasty question at this point: What happens if we are using
libguestfs with an IDE-based appliance (ie. devices named /dev/sd*),
but our target system will have virtio drivers (devices named
/dev/vd*)?

We should probably use a neutral name instead of actual device names
in the internal structures ("HD(a,1)"), then map those on the way out.
Hopefully it won't matter too much since all modern distros use UUIDs
or labels, but beware of RHEL 3 guests.

[I checked all the other modules here but didn't have any particular
comments about them].

> diff --git a/v2v/virt-v2v.pl b/v2v/virt-v2v.pl
[...]
> +# Create a squashfs filesystem containing all files given on the command line

I think you were right about ISO vs squashfs ...  If you do use
squashfs, then note that RHEL 5 cannot mount it unless you use a
special mount_vfs call.  See:

http://git.et.redhat.com/?p=libguestfs.git;a=commitdiff;h=9af502eff08017941a58ad676d0cbb867f83a341

	---

OK there's not as much duplicate code as I feared.  In fact, your code
builds nicely on top of the inspection done in Lib.pm.  How about we
leave Lib.pm alone?

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://et.redhat.com/~rjones/libguestfs/
See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html