[PATCH 1/7] Push $desc creation into Sys::VirtConvert::Converter->convert
This change is part of an ongoing effort to remove use of $desc and inspect the OS directly as required during conversion. --- lib/Sys/VirtConvert/Connection/LibVirtTarget.pm | 4 +- lib/Sys/VirtConvert/Connection/RHEVTarget.pm | 41 +++++++++--------- lib/Sys/VirtConvert/Converter.pm | 35 ++++++++++++---- lib/Sys/VirtConvert/Converter/RedHat.pm | 45 +++++++++++-------- lib/Sys/VirtConvert/Converter/Windows.pm | 10 +++- p2v-server/virt-p2v-server.pl | 52 ++++------------------- v2v/virt-v2v.pl | 49 ++++------------------ 7 files changed, 99 insertions(+), 137 deletions(-) diff --git a/lib/Sys/VirtConvert/Connection/LibVirtTarget.pm b/lib/Sys/VirtConvert/Connection/LibVirtTarget.pm index a74f9df..b77ce1d 100644 --- a/lib/Sys/VirtConvert/Connection/LibVirtTarget.pm +++ b/lib/Sys/VirtConvert/Connection/LibVirtTarget.pm @@ -269,7 +269,7 @@ sub guest_exists return 1; } -=item create_guest(desc, meta, config, guestcaps, output_name) +=item create_guest(g, root, meta, config, guestcaps, output_name) Create the guest in the target @@ -278,7 +278,7 @@ Create the guest in the target sub create_guest { my $self = shift; - my ($desc, $meta, $config, $guestcaps, $output_name) = @_; + my (undef, undef, $meta, $config, $guestcaps, $output_name) = @_; my $vmm = $self->{vmm}; diff --git a/lib/Sys/VirtConvert/Connection/RHEVTarget.pm b/lib/Sys/VirtConvert/Connection/RHEVTarget.pm index 4ebdcb9..5f0fe47 100644 --- a/lib/Sys/VirtConvert/Connection/RHEVTarget.pm +++ b/lib/Sys/VirtConvert/Connection/RHEVTarget.pm @@ -605,7 +605,7 @@ sub guest_exists return 0; } -=item create_guest(desc, meta, config, guestcaps, output_name) +=item create_guest(g, root, meta, config, guestcaps, output_name) Create the guest in the target @@ -614,7 +614,7 @@ Create the guest in the target sub create_guest { my $self = shift; - my ($desc, $meta, $config, $guestcaps, $output_name) = @_; + my ($g, $root, $meta, $config, $guestcaps, $output_name) = @_; # Get the number of virtual cpus my $ncpus = $meta->{cpus}; @@ -627,7 +627,7 @@ sub create_guest my $vmuuid = rhev_util::get_uuid(); - my $ostype = _get_os_type($desc); + my $ostype = _get_os_type($g, $root); my $ovf = new XML::DOM::Parser->parse(<<EOF); <ovf:Envelope @@ -798,23 +798,24 @@ EOF # one of the above values in case we're wrong. sub _get_os_type { - my ($desc) = @_; + my ($g, $root) = @_; + + my $arch = $g->inspect_get_arch($root); my $arch_suffix = ''; - if ($desc->{arch} eq 'x86_64') { + if ($arch eq 'x86_64') { $arch_suffix = 'x64'; - } elsif ($desc->{arch} ne 'i386') { - logmsg WARN, __x('Unsupported architecture: {arch}', - arch => $desc->{arch}); + } elsif ($arch ne 'i386') { + logmsg WARN, __x('Unsupported architecture: {arch}', arch => $arch); return undef; } my $type; - $type = _get_os_type_linux($desc, $arch_suffix) - if ($desc->{os} eq 'linux'); - $type = _get_os_type_windows($desc, $arch_suffix) - if ($desc->{os} eq 'windows'); + $type = _get_os_type_linux($g, $root, $arch_suffix) + if ($g->inspect_get_type($root) eq 'linux'); + $type = _get_os_type_windows($g, $root, $arch_suffix) + if ($g->inspect_get_type($root) eq 'windows'); return 'Unassigned' if (!defined($type)); return $type; @@ -822,11 +823,11 @@ sub _get_os_type sub _get_os_type_windows { - my ($desc, $arch_suffix) = @_; + my ($g, $root, $arch_suffix) = @_; - my $major = $desc->{major_version}; - my $minor = $desc->{minor_version}; - my $product = $desc->{product_name}; + my $major = $g->inspect_get_major_version($root); + my $minor = $g->inspect_get_minor_version($root); + my $product = $g->inspect_get_product_name($root); if ($major == 5) { if ($minor == 1 || @@ -847,7 +848,7 @@ sub _get_os_type_windows } if ($major == 6 && $minor == 1) { - if ($desc->{product_variant} eq 'Client') { + if ($g->inspect_get_product_variant($root) eq 'Client') { return "Windows7".$arch_suffix; } @@ -861,10 +862,10 @@ sub _get_os_type_windows sub _get_os_type_linux { - my ($desc, $arch_suffix) = @_; + my ($g, $root, $arch_suffix) = @_; - my $distro = $desc->{distro}; - my $major = $desc->{major_version}; + my $distro = $g->inspect_get_distro($root); + my $major = $g->inspect_get_major_version($root); # XXX: RHEV 2.2 doesn't support a RHEL 6 target, however RHEV 2.3+ will. # For the moment, we set RHEL 6 to be 'OtherLinux', however we will need to diff --git a/lib/Sys/VirtConvert/Converter.pm b/lib/Sys/VirtConvert/Converter.pm index cfee3da..28c57d5 100644 --- a/lib/Sys/VirtConvert/Converter.pm +++ b/lib/Sys/VirtConvert/Converter.pm @@ -40,7 +40,7 @@ Sys::VirtConvert::Converter - Convert a guest to run on KVM use Sys::VirtConvert::Converter; - Sys::VirtConvert::Converter->convert($g, $config, $desc, $meta); + Sys::VirtConvert::Converter->convert($g, $config, $root, $meta); =head1 DESCRIPTION @@ -51,7 +51,7 @@ guest OS, and uses it to convert the guest to run on KVM. =over -=item Sys::VirtConvert::Converter->convert(g, config, desc, meta) +=item Sys::VirtConvert::Converter->convert(g, config, root, meta) Instantiate an appropriate backend and call convert on it. @@ -65,9 +65,9 @@ A libguestfs handle to the target. An initialised Sys::VirtConvert::Config object. -=item desc +=item root -The OS description (see virt-v2v.pl:inspect_guest). +The root device of the os to be converted. =item meta @@ -81,18 +81,37 @@ sub convert { my $class = shift; - my ($g, $config, $desc, $meta) = @_; + my ($g, $config, $root, $meta) = @_; croak("convert called without g argument") unless defined($g); croak("convert called without config argument") unless defined($config); - croak("convert called without desc argument") unless defined($desc); + croak("convert called without root argument") unless defined($root); croak("convert called without meta argument") unless defined($meta); my $guestcaps; + # Mount up the disks. + my %fses = $g->inspect_get_mountpoints ($root); + my @fses = sort { length $a <=> length $b } keys %fses; + foreach (@fses) { + eval { $g->mount_options ("", $fses{$_}, $_) }; + print __x("{e} (ignored)\n", e => $@) if $@; + } + + # Construct the "$desc" hashref which contains the main features + # found by inspection. + my %desc; + + $desc{os} = $g->inspect_get_type($root); + $desc{distro} = $g->inspect_get_distro($root); + $desc{product_name} = $g->inspect_get_product_name($root); + $desc{major_version} = $g->inspect_get_major_version($root); + $desc{minor_version} = $g->inspect_get_minor_version($root); + $desc{arch} = $g->inspect_get_arch($root); + # Find a module which can convert the guest and run it foreach my $module ($class->modules()) { - if($module->can_handle($desc)) { - $guestcaps = $module->convert($g, $config, $desc, $meta); + if($module->can_handle(\%desc)) { + $guestcaps = $module->convert($g, $root, $config, \%desc, $meta); last; } } diff --git a/lib/Sys/VirtConvert/Converter/RedHat.pm b/lib/Sys/VirtConvert/Converter/RedHat.pm index cd7bde1..8a6ff6b 100644 --- a/lib/Sys/VirtConvert/Converter/RedHat.pm +++ b/lib/Sys/VirtConvert/Converter/RedHat.pm @@ -68,7 +68,7 @@ sub can_handle $desc->{distro} =~ /^(rhel|fedora)$/); } -=item Sys::VirtConvert::Converter::RedHat->convert(g, config, meta, desc) +=item Sys::VirtConvert::Converter::RedHat->convert(g, root, config, meta, desc) Convert a Red Hat based guest. Assume that can_handle has previously returned 1. @@ -78,13 +78,17 @@ Convert a Red Hat based guest. Assume that can_handle has previously returned 1. An initialised Sys::Guestfs handle +=item root + +The root device of this operating system. + =item config An initialised Sys::VirtConvert::Config =item desc -A description of the guest OS (see virt-v2v.pl:inspect_guest). +A description of the guest OS (see Sys::VirtConvert::Converter->convert()). =item meta @@ -98,8 +102,9 @@ sub convert { my $class = shift; - my ($g, $config, $desc, $meta) = @_; + my ($g, $root, $config, $desc, $meta) = @_; croak("convert called without g argument") unless defined($g); + croak("convert called without root argument") unless defined($root); croak("convert called without config argument") unless defined($config); croak("convert called without desc argument") unless defined($desc); croak("convert called without meta argument") unless defined($meta); @@ -112,7 +117,7 @@ sub convert # Un-configure HV specific attributes which don't require a direct # replacement - _unconfigure_hv($g, $desc); + _unconfigure_hv($g, $root, $desc); # Try to install the virtio capability my $virtio = _install_capability('virtio', $g, $config, $meta, $desc); @@ -125,7 +130,7 @@ sub convert _configure_display_driver($g); _remap_block_devices($meta, $virtio, $g, $desc); _configure_kernel_modules($g, $desc, $virtio, $modpath); - _configure_boot($kernel, $virtio, $g, $desc); + _configure_boot($kernel, $virtio, $g, $root, $desc); my %guestcaps; @@ -850,18 +855,18 @@ sub _configure_kernel sub _configure_boot { - my ($kernel, $virtio, $g, $desc) = @_; + my ($kernel, $virtio, $g, $root, $desc) = @_; if($virtio) { # The order of modules here is deliberately the same as the order # specified in the postinstall script of kmod-virtio in RHEL3. The # reason is that the probing order determines the major number of vdX # block devices. If we change it, RHEL 3 KVM guests won't boot. - _prepare_bootable($g, $desc, $kernel, "virtio", "virtio_ring", - "virtio_blk", "virtio_net", - "virtio_pci"); + _prepare_bootable($g, $root, $desc, $kernel, "virtio", "virtio_ring", + "virtio_blk", "virtio_net", + "virtio_pci"); } else { - _prepare_bootable($g, $desc, $kernel, "sym53c8xx"); + _prepare_bootable($g, $root, $desc, $kernel, "sym53c8xx"); } } @@ -934,21 +939,23 @@ sub _get_application_owner sub _unconfigure_hv { - my ($g, $desc) = @_; + my ($g, $root, $desc) = @_; - _unconfigure_xen($g, $desc); - _unconfigure_vmware($g, $desc); + my @apps = $g->inspect_list_applications($root); + + _unconfigure_xen($g, $desc, \@apps); + _unconfigure_vmware($g, $desc, \@apps); } # Unconfigure Xen specific guest modifications sub _unconfigure_xen { - my ($g, $desc) = @_; + my ($g, $desc, $apps) = @_; my $found_kmod = 0; # Look for kmod-xenpv-*, which can be found on RHEL 3 machines - foreach my $app (@{$desc->{apps}}) { + foreach my $app (@$apps) { my $name = $app->{app_name}; if($name =~ /^kmod-xenpv(-.*)?$/) { @@ -1005,10 +1012,10 @@ sub _unconfigure_xen # Unconfigure VMware specific guest modifications sub _unconfigure_vmware { - my ($g, $desc) = @_; + my ($g, $desc, $apps) = @_; # Uninstall VMwareTools - foreach my $app (@{$desc->{apps}}) { + foreach my $app (@$apps) { my $name = $app->{app_name}; if ($name eq "VMwareTools") { @@ -2161,7 +2168,7 @@ sub _drivecmp sub _prepare_bootable { - my ($g, $desc, $version, @modules) = @_; + my ($g, $root, $desc, $version, @modules) = @_; # Find the grub entry for the given kernel my $initrd; @@ -2255,7 +2262,7 @@ sub _prepare_bootable # internal variable in mkinitrd, and is therefore extremely nasty # and applicable only to a particular version of mkinitrd. if ($desc->{distro} eq 'rhel' && $desc->{major_version} eq '4') { - push(@env, 'root_lvm=1') if ($g->is_lv($desc->{root_device})); + push(@env, 'root_lvm=1') if ($g->is_lv($root)); } $g->sh(join(' ', @env).' /sbin/mkinitrd '.join(' ', @module_args). diff --git a/lib/Sys/VirtConvert/Converter/Windows.pm b/lib/Sys/VirtConvert/Converter/Windows.pm index b4eae74..321f797 100644 --- a/lib/Sys/VirtConvert/Converter/Windows.pm +++ b/lib/Sys/VirtConvert/Converter/Windows.pm @@ -80,7 +80,7 @@ sub can_handle return ($desc->{os} eq 'windows'); } -=item Sys::VirtConvert::Converter::Windows->convert(g, config, desc, meta) +=item Sys::VirtConvert::Converter::Windows->convert(g, root, config, desc, meta) (Pre-)convert a Windows guest. Assume that can_handle has previously returned 1. @@ -91,13 +91,17 @@ returned 1. A libguestfs handle to the target. +=item root + +The root device of this operating system. + =item config An initialised Sys::VirtConvert::Config object. =item desc -A description of the guest OS (see virt-v2v.pl:inspect_guest). +A description of the guest OS (see Sys::VirtConvert::Converter->convert). =item meta @@ -111,7 +115,7 @@ sub convert { my $class = shift; - my ($g, $config, $desc, undef) = @_; + my ($g, undef, $config, $desc, undef) = @_; croak("convert called without g argument") unless defined($g); croak("convert called without config argument") unless defined($config); croak("convert called without desc argument") unless defined($desc); diff --git a/p2v-server/virt-p2v-server.pl b/p2v-server/virt-p2v-server.pl index 5b1cb68..c193e1c 100755 --- a/p2v-server/virt-p2v-server.pl +++ b/p2v-server/virt-p2v-server.pl @@ -306,8 +306,6 @@ sub convert my @localpaths = map { $_->{local_path} } @{$meta->{disks}}; my $g; - my $desc; - my $guestcaps; eval { $g = new Sys::VirtConvert::GuestfsHandle( \@localpaths, @@ -315,10 +313,11 @@ sub convert $target->isa('Sys::VirtConvert::Connection::RHEVTarget') ); - $desc = inspect_guest($g); - $guestcaps = - Sys::VirtConvert::Converter->convert($g, $config, $desc, $meta); - $target->create_guest($desc, $meta, $config, $guestcaps, $meta->{name}); + my $root = inspect_guest($g); + my $guestcaps = + Sys::VirtConvert::Converter->convert($g, $config, $root, $meta); + $target->create_guest($g, $root, $meta, $config, $guestcaps, + $meta->{name}); if($guestcaps->{block} eq 'virtio' && $guestcaps->{net} eq 'virtio') { logmsg NOTICE, __x('{name} configured with virtio drivers.', @@ -373,14 +372,13 @@ END { } # Perform guest inspection using the libguestfs core inspection API. -# Returns a hashref ("$desc") which contains the main features from -# inspection. +# Returns the root device of the os to be converted. sub inspect_guest { my $g = shift; # Get list of roots, sorted - my @roots = $g->inspect_os (); + my @roots = $g->inspect_os(); @roots = sort @roots; # Only work on single-root operating systems. @@ -390,41 +388,7 @@ sub inspect_guest v2vdie __('Multiboot operating systems are not supported.') if @roots > 1; - my $root_dev = $roots[0]; - - # Mount up the disks. - my %fses = $g->inspect_get_mountpoints ($root_dev); - my @fses = sort { length $a <=> length $b } keys %fses; - foreach (@fses) { - eval { $g->mount_options ("", $fses{$_}, $_) }; - print __x("{e} (ignored)\n", e => $@) if $@; - } - - # Construct the "$desc" hashref which contains the main features - # found by inspection. - my %desc; - - $desc{root_device} = $root_dev; - - $desc{os} = $g->inspect_get_type ($root_dev); - $desc{distro} = $g->inspect_get_distro ($root_dev); - $desc{product_name} = $g->inspect_get_product_name ($root_dev); - $desc{product_variant} = $g->inspect_get_product_variant ($root_dev); - $desc{major_version} = $g->inspect_get_major_version ($root_dev); - $desc{minor_version} = $g->inspect_get_minor_version ($root_dev); - $desc{arch} = $g->inspect_get_arch ($root_dev); - - # Notes: - # (1) Filesystems have to be mounted for this to work. Do not - # move this code over the filesystem mounting code above. - # (2) For RPM-based distros, new libguestfs inspection code - # is only able to populate the 'app_name' field (old Perl code - # populated a lot more). Fortunately this is the only field - # that the code currently uses. - my @apps = $g->inspect_list_applications ($root_dev); - $desc{apps} = \@apps; - - return \%desc; + return $roots[0]; } sub p2v_receive diff --git a/v2v/virt-v2v.pl b/v2v/virt-v2v.pl index 6e73102..3d71afe 100755 --- a/v2v/virt-v2v.pl +++ b/v2v/virt-v2v.pl @@ -504,14 +504,14 @@ if (defined($transferiso)) { } my $guestcaps; -my $desc; +my $root; eval { # Inspect the guest - $desc = inspect_guest($g, $transferdev); + $root = inspect_guest($g, $transferdev); # Modify the guest and its metadata $guestcaps = - Sys::VirtConvert::Converter->convert($g, $config, $desc, $meta); + Sys::VirtConvert::Converter->convert($g, $config, $root, $meta); }; # If any of the above commands result in failure, we need to ensure that the @@ -523,9 +523,9 @@ if ($@) { die($err); } -$g->close(); +$target->create_guest($g, $root, $meta, $config, $guestcaps, $output_name); -$target->create_guest($desc, $meta, $config, $guestcaps, $output_name); +$g->close(); if($guestcaps->{block} eq 'virtio' && $guestcaps->{net} eq 'virtio') { logmsg NOTICE, __x('{name} configured with virtio drivers.', @@ -561,15 +561,14 @@ sub signal_exit } # Perform guest inspection using the libguestfs core inspection API. -# Returns a hashref ("$desc") which contains the main features from -# inspection. +# Returns the root device of the os to be converted. sub inspect_guest { my $g = shift; my $transferdev = shift; # Get list of roots, sorted. - my @roots = $g->inspect_os (); + my @roots = $g->inspect_os(); # Filter out the transfer device from the results of inspect_os # There's a libguestfs bug (fixed upstream) which meant the transfer ISO @@ -655,39 +654,7 @@ sub inspect_guest } } - # Mount up the disks. - my %fses = $g->inspect_get_mountpoints ($root_dev); - my @fses = sort { length $a <=> length $b } keys %fses; - foreach (@fses) { - eval { $g->mount_options ("", $fses{$_}, $_) }; - print __x("{e} (ignored)\n", e => $@) if $@; - } - - # Construct the "$desc" hashref which contains the main features - # found by inspection. - my %desc; - - $desc{root_device} = $root_dev; - - $desc{os} = $g->inspect_get_type ($root_dev); - $desc{distro} = $g->inspect_get_distro ($root_dev); - $desc{product_name} = $g->inspect_get_product_name ($root_dev); - $desc{product_variant} = $g->inspect_get_product_variant ($root_dev); - $desc{major_version} = $g->inspect_get_major_version ($root_dev); - $desc{minor_version} = $g->inspect_get_minor_version ($root_dev); - $desc{arch} = $g->inspect_get_arch ($root_dev); - - # Notes: - # (1) Filesystems have to be mounted for this to work. Do not - # move this code over the filesystem mounting code above. - # (2) For RPM-based distros, new libguestfs inspection code - # is only able to populate the 'app_name' field (old Perl code - # populated a lot more). Fortunately this is the only field - # that the code currently uses. - my @apps = $g->inspect_list_applications ($root_dev); - $desc{apps} = \@apps; - - return \%desc; + return $root_dev; } =head1 PREPARING TO CONVERT A GUEST -- 1.7.4.4
F14 doesn't have inspect_get_product_variant (F15 does). The only thing it's needed for is differentiating Win7 from Win2k8r2 when writing to RHEV, so work round it if it's not there. --- lib/Sys/VirtConvert/Connection/RHEVTarget.pm | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/lib/Sys/VirtConvert/Connection/RHEVTarget.pm b/lib/Sys/VirtConvert/Connection/RHEVTarget.pm index 5f0fe47..687f745 100644 --- a/lib/Sys/VirtConvert/Connection/RHEVTarget.pm +++ b/lib/Sys/VirtConvert/Connection/RHEVTarget.pm @@ -848,7 +848,11 @@ sub _get_os_type_windows } if ($major == 6 && $minor == 1) { - if ($g->inspect_get_product_variant($root) eq 'Client') { + # This API is new in libguestfs 1.10 + # If it's not present, we can't differentiate between Win7 and Win2k8r2 + if ($g->can('inspect_get_product_variant') && + $g->inspect_get_product_variant($root) eq 'Client') + { return "Windows7".$arch_suffix; } -- 1.7.4.4
On Tue, Apr 26, 2011 at 05:03:47PM +0100, Matthew Booth wrote:
F14 doesn't have inspect_get_product_variant (F15 does). The only thing it's needed for is differentiating Win7 from Win2k8r2 when writing to RHEV, so work round it if it's not there. --- lib/Sys/VirtConvert/Connection/RHEVTarget.pm | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/lib/Sys/VirtConvert/Connection/RHEVTarget.pm b/lib/Sys/VirtConvert/Connection/RHEVTarget.pm index 5f0fe47..687f745 100644 --- a/lib/Sys/VirtConvert/Connection/RHEVTarget.pm +++ b/lib/Sys/VirtConvert/Connection/RHEVTarget.pm @@ -848,7 +848,11 @@ sub _get_os_type_windows }
if ($major == 6 && $minor == 1) { - if ($g->inspect_get_product_variant($root) eq 'Client') { + # This API is new in libguestfs 1.10 + # If it's not present, we can't differentiate between Win7 and Win2k8r2 + if ($g->can('inspect_get_product_variant') &&
My tests indicated that you have to write: if (defined (Sys::Guestfs->can ("inspect_get_product_variant"))) ... However I don't know if 'defined' is really needed there or not. 'can' is documented this way: can(METHOD) "can" checks to see if its object has a method called "METHOD", if it does then a reference to the sub is returned, if it does not then "undef" is returned. It seems to hang on whether the sub reference could ever be evaluated to false (eg. it could be "0" or something like that). That seems unlikely to me right now ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
--- lib/Sys/VirtConvert/Converter/RedHat.pm | 97 +++++++++++++++---------------- 1 files changed, 47 insertions(+), 50 deletions(-) diff --git a/lib/Sys/VirtConvert/Converter/RedHat.pm b/lib/Sys/VirtConvert/Converter/RedHat.pm index 8a6ff6b..5704fd8 100644 --- a/lib/Sys/VirtConvert/Converter/RedHat.pm +++ b/lib/Sys/VirtConvert/Converter/RedHat.pm @@ -621,67 +621,64 @@ sub _init_kernels if($@) { warn __x("Grub entry {title} has no kernel", title => $config{title}); + next; } - # Check we've got a kernel entry - if(defined($grub_kernel)) { - my $path = "$grub$grub_kernel"; + my $path = "$grub$grub_kernel"; - # Reconstruct the kernel command line - my @args = (); - foreach my $arg ($g->aug_match("$bootable/kernel/*")) { - $arg =~ m{/kernel/([^/]*)$} - or die("Unexpected return from aug_match: $arg"); + # Reconstruct the kernel command line + my @args = (); + foreach my $arg ($g->aug_match("$bootable/kernel/*")) { + $arg =~ m{/kernel/([^/]*)$} + or die("Unexpected return from aug_match: $arg"); - my $name = $1; - my $value; - eval { $value = $g->aug_get($arg); }; + my $name = $1; + my $value; + eval { $value = $g->aug_get($arg); }; - if(defined($value)) { - push(@args, "$name=$value"); - } else { - push(@args, $name); - } - } - $config{cmdline} = join(' ', @args) if(scalar(@args) > 0); - - my $kernel; - if ($g->exists($path)) { - $kernel = _inspect_linux_kernel($g, $path); + if(defined($value)) { + push(@args, "$name=$value"); } else { - warn __x("grub refers to {path}, which doesn't exist\n", - path => $path); + push(@args, $name); } + } + $config{cmdline} = join(' ', @args) if(scalar(@args) > 0); - # Check the kernel was recognised - if(defined($kernel)) { - # Put this kernel on the top level kernel list - $desc->{kernels} ||= []; - push(@{$desc->{kernels}}, $kernel); - - $config{kernel} = $kernel; - - # Look for an initrd entry - my $initrd; - eval { - $initrd = $g->aug_get("$bootable/initrd"); - }; - - unless($@) { - $config{initrd} = - _inspect_initrd($g, $desc, "$grub$initrd", - $kernel->{version}); - } else { - warn __x("Grub entry {title} does not specify an ". - "initrd", title => $config{title}); - } - } + my $kernel; + if ($g->exists($path)) { + $kernel = _inspect_linux_kernel($g, $path); + } else { + warn __x("grub refers to {path}, which doesn't exist\n", + path => $path); + } + + # Check the kernel was recognised + next unless defined($kernel); + + # Put this kernel on the top level kernel list + $desc->{kernels} ||= []; + push(@{$desc->{kernels}}, $kernel); + + $config{kernel} = $kernel; + + # Look for an initrd entry + my $initrd; + eval { + $initrd = $g->aug_get("$bootable/initrd"); + }; + + unless($@) { + $config{initrd} = + _inspect_initrd($g, $desc, "$grub$initrd", + $kernel->{version}); + } else { + warn __x("Grub entry {title} does not specify an ". + "initrd", title => $config{title}); } push(@configs, \%config); } - # Create the top level boot entry my %boot; $boot{configs} = \@configs; -- 1.7.4.4
Assume we need to upgrade the kernel if we can't automatically detect anything about any installed kernel. --- lib/Sys/VirtConvert/Converter/RedHat.pm | 121 +++++++++++++++++-------------- 1 files changed, 66 insertions(+), 55 deletions(-) diff --git a/lib/Sys/VirtConvert/Converter/RedHat.pm b/lib/Sys/VirtConvert/Converter/RedHat.pm index 5704fd8..456110b 100644 --- a/lib/Sys/VirtConvert/Converter/RedHat.pm +++ b/lib/Sys/VirtConvert/Converter/RedHat.pm @@ -1113,66 +1113,80 @@ sub _install_capability # Kernels are special if ($name eq 'kernel') { - my ($kernel_pkg, $kernel_rpmver, $kernel_arch) = + my ($kernel_pkg, $kernel_arch, $kernel_rpmver) = _discover_kernel($desc); - my ($kernel_epoch, $kernel_ver, $kernel_release); - eval { - ($kernel_epoch, $kernel_ver, $kernel_release) = - _parse_evr($kernel_rpmver); - }; - if ($@) { - # Don't die here, just make best effort to do a version - # comparison by directly comparing the full strings - $kernel_epoch = undef; - $kernel_ver = $kernel_rpmver; - $kernel_release = undef; - - $min_epoch = undef; - $min_version = $props->{minversion}; - $min_release = undef; + # If we didn't establish a kernel version, assume we have to upgrade + # it. + if (!defined($kernel_rpmver)) { + $kernel = [$kernel_pkg, $kernel_arch]; } - # If the guest is using a Xen PV kernel, choose an appropriate - # normal kernel replacement - if ($kernel_pkg eq "kernel-xen" || $kernel_pkg eq "kernel-xenU") { - $kernel_pkg = - _get_replacement_kernel_name($kernel_arch, $desc, $meta); - - # Check if we've got already got an appropriate kernel - my ($installed) = - _get_installed("$kernel_pkg.$kernel_arch", $g); + else { + my ($kernel_epoch, $kernel_ver, $kernel_release); + eval { + ($kernel_epoch, $kernel_ver, $kernel_release) = + _parse_evr($kernel_rpmver); + }; + if ($@) { + # Don't die here, just make best effort to do a version + # comparison by directly comparing the full strings + $kernel_epoch = undef; + $kernel_ver = $kernel_rpmver; + $kernel_release = undef; + + $min_epoch = undef; + $min_version = $props->{minversion}; + $min_release = undef; + } - if (!defined($installed) || - _evr_cmp($installed->[0], $installed->[1], $installed->[2], - $min_epoch, $min_version, $min_release) < 0) + # If the guest is using a Xen PV kernel, choose an appropriate + # normal kernel replacement + if ($kernel_pkg eq "kernel-xen" || $kernel_pkg eq "kernel-xenU") { - # filter out xen/xenU from release field - if (defined($kernel_release) && - $kernel_release =~ /^(\S+?)(xen)?(U)?$/) - { - $kernel_release = $1; - } + $kernel_pkg = + _get_replacement_kernel_name($kernel_arch, $desc, + $meta); - # If the guest kernel is new enough, but PV, try to replace - # it with an equivalent version FV kernel - if (_evr_cmp($kernel_epoch, $kernel_ver, $kernel_release, - $min_epoch, $min_version, $min_release) >= 0) { - $kernel = [$kernel_pkg, $kernel_arch, - $kernel_epoch, $kernel_ver, $kernel_release]; - } + # Check if we've got already got an appropriate kernel + my ($inst) = + _get_installed("$kernel_pkg.$kernel_arch", $g); - # Otherwise, just grab the latest - else { - $kernel = [$kernel_pkg, $kernel_arch]; + if (!defined($inst) || + _evr_cmp($inst->[0], $inst->[1], $inst->[2], + $min_epoch, $min_version, $min_release) < 0) + { + # filter out xen/xenU from release field + if (defined($kernel_release) && + $kernel_release =~ /^(\S+?)(xen)?(U)?$/) + { + $kernel_release = $1; + } + + # If the guest kernel is new enough, but PV, try to + # replace it with an equivalent version FV kernel + if (_evr_cmp($kernel_epoch, $kernel_ver, + $kernel_release, + $min_epoch, $min_version, + $min_release) >= 0) + { + $kernel = [$kernel_pkg, $kernel_arch, + $kernel_epoch, $kernel_ver, + $kernel_release]; + } + + # Otherwise, just grab the latest + else { + $kernel = [$kernel_pkg, $kernel_arch]; + } } } - } - # If the kernel is too old, grab the latest replacement - elsif (_evr_cmp($kernel_epoch, $kernel_ver, $kernel_release, - $min_epoch, $min_version, $min_release) < 0) { - $kernel = [$kernel_pkg, $kernel_arch]; + # If the kernel is too old, grab the latest replacement + elsif (_evr_cmp($kernel_epoch, $kernel_ver, $kernel_release, + $min_epoch, $min_version, $min_release) < 0) { + $kernel = [$kernel_pkg, $kernel_arch]; + } } } @@ -1557,8 +1571,8 @@ sub _discover_kernel # Get a current bootable kernel, preferrably the default my $kernel_pkg; - my $kernel_ver; my $kernel_arch; + my $kernel_ver; foreach my $i (@configs) { my $config = $boot->{configs}->[$i]; @@ -1587,14 +1601,11 @@ sub _discover_kernel # directly detected $kernel_arch = $desc->{arch} if (!defined($kernel_arch)); - v2vdie __('Unable to determine a kernel architecture for this guest.') - unless defined($kernel_arch); - # We haven't supported anything other than i686 for the kernel on 32 bit for # a very long time. $kernel_arch = 'i686' if ('i386' eq $kernel_arch); - return ($kernel_pkg, $kernel_ver, $kernel_arch); + return ($kernel_pkg, $kernel_arch, $kernel_ver); } sub _get_replacement_kernel_name @@ -1665,7 +1676,7 @@ sub _install_good_kernel { my ($g, $config, $desc, $meta) = @_; - my ($kernel_pkg, $kernel_rpmver, $kernel_arch) = _discover_kernel($desc); + my ($kernel_pkg, $kernel_arch, undef) = _discover_kernel($desc); # If the guest is using a Xen PV kernel, choose an appropriate # normal kernel replacement -- 1.7.4.4
--- lib/Sys/VirtConvert/Converter/RedHat.pm | 23 ++++++++++++++++------- 1 files changed, 16 insertions(+), 7 deletions(-) diff --git a/lib/Sys/VirtConvert/Converter/RedHat.pm b/lib/Sys/VirtConvert/Converter/RedHat.pm index 456110b..82d52de 100644 --- a/lib/Sys/VirtConvert/Converter/RedHat.pm +++ b/lib/Sys/VirtConvert/Converter/RedHat.pm @@ -112,7 +112,7 @@ sub convert _init_selinux($g); _init_augeas($g); _init_modprobe_aliases($g, $desc); - _init_kernels($g, $desc); + _init_kernels($g, $root, $desc); my $modpath = _init_modpath($g); # Un-configure HV specific attributes which don't require a direct @@ -583,17 +583,26 @@ sub _list_kernels sub _init_kernels { - my ($g, $desc) = @_; + my ($g, $root, $desc) = @_; if ($desc->{os} eq "linux") { # Iterate over entries in grub.conf, populating $desc->{boot} # For every kernel we find, inspect it and add to $desc->{kernels} - # All known past and present Red Hat-based distros mount a - # boot partition on /boot. We may have to revisit this if - # this assumption changes in future. (Old Perl inspection - # code used to try to detect this setting). - my $grub = "/boot"; + # Find the path which needs to be prepended to paths in grub.conf to + # make them absolute + # Default to / (no prefix required) + my $grub = ""; + + # Look for the most specific mount point discovered + my %mounts = $g->inspect_get_mountpoints($root); + foreach my $path qw(/boot/grub /boot) { + if (exists($mounts{$path})) { + $grub = $path; + last; + } + } + my $grub_conf = "/etc/grub.conf"; my @boot_configs; -- 1.7.4.4
On Tue, Apr 26, 2011 at 05:03:50PM +0100, Matthew Booth wrote:
--- lib/Sys/VirtConvert/Converter/RedHat.pm | 23 ++++++++++++++++------- 1 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/lib/Sys/VirtConvert/Converter/RedHat.pm b/lib/Sys/VirtConvert/Converter/RedHat.pm index 456110b..82d52de 100644 --- a/lib/Sys/VirtConvert/Converter/RedHat.pm +++ b/lib/Sys/VirtConvert/Converter/RedHat.pm @@ -112,7 +112,7 @@ sub convert _init_selinux($g); _init_augeas($g); _init_modprobe_aliases($g, $desc); - _init_kernels($g, $desc); + _init_kernels($g, $root, $desc); my $modpath = _init_modpath($g);
# Un-configure HV specific attributes which don't require a direct @@ -583,17 +583,26 @@ sub _list_kernels
sub _init_kernels { - my ($g, $desc) = @_; + my ($g, $root, $desc) = @_;
if ($desc->{os} eq "linux") { # Iterate over entries in grub.conf, populating $desc->{boot} # For every kernel we find, inspect it and add to $desc->{kernels}
- # All known past and present Red Hat-based distros mount a - # boot partition on /boot. We may have to revisit this if - # this assumption changes in future. (Old Perl inspection - # code used to try to detect this setting). - my $grub = "/boot"; + # Find the path which needs to be prepended to paths in grub.conf to + # make them absolute + # Default to / (no prefix required) + my $grub = ""; + + # Look for the most specific mount point discovered + my %mounts = $g->inspect_get_mountpoints($root); + foreach my $path qw(/boot/grub /boot) { + if (exists($mounts{$path})) { + $grub = $path; + last; + } + } +
ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
--- lib/Sys/VirtConvert/Util.pm | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Sys/VirtConvert/Util.pm b/lib/Sys/VirtConvert/Util.pm index 94211a5..b74d750 100644 --- a/lib/Sys/VirtConvert/Util.pm +++ b/lib/Sys/VirtConvert/Util.pm @@ -121,8 +121,8 @@ sub augeas_error chomp($msg); - v2vdie $msg if length($msg) > 0; - v2vdie $err; + v2vdie($msg) if length($msg) > 0; + v2vdie($err); } -- 1.7.4.4
On Tue, Apr 26, 2011 at 05:03:51PM +0100, Matthew Booth wrote:
--- lib/Sys/VirtConvert/Util.pm | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/Sys/VirtConvert/Util.pm b/lib/Sys/VirtConvert/Util.pm index 94211a5..b74d750 100644 --- a/lib/Sys/VirtConvert/Util.pm +++ b/lib/Sys/VirtConvert/Util.pm @@ -121,8 +121,8 @@ sub augeas_error
chomp($msg);
- v2vdie $msg if length($msg) > 0; - v2vdie $err; + v2vdie($msg) if length($msg) > 0; + v2vdie($err); }
ACK. Bit surprised the without-parens version doesn't work though ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
On 26/04/11 17:55, Richard W.M. Jones wrote:
On Tue, Apr 26, 2011 at 05:03:51PM +0100, Matthew Booth wrote:
--- lib/Sys/VirtConvert/Util.pm | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/Sys/VirtConvert/Util.pm b/lib/Sys/VirtConvert/Util.pm index 94211a5..b74d750 100644 --- a/lib/Sys/VirtConvert/Util.pm +++ b/lib/Sys/VirtConvert/Util.pm @@ -121,8 +121,8 @@ sub augeas_error
chomp($msg);
- v2vdie $msg if length($msg)> 0; - v2vdie $err; + v2vdie($msg) if length($msg)> 0; + v2vdie($err); }
ACK.
Bit surprised the without-parens version doesn't work though ...
Me too. The output it produces, though, suggests it's trying to do $err->v2vdie(). Matt -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
--- lib/Sys/VirtConvert/Converter/RedHat.pm | 140 ++++++++++++++++-------------- 1 files changed, 75 insertions(+), 65 deletions(-) diff --git a/lib/Sys/VirtConvert/Converter/RedHat.pm b/lib/Sys/VirtConvert/Converter/RedHat.pm index 82d52de..4b8467a 100644 --- a/lib/Sys/VirtConvert/Converter/RedHat.pm +++ b/lib/Sys/VirtConvert/Converter/RedHat.pm @@ -109,10 +109,13 @@ sub convert croak("convert called without desc argument") unless defined($desc); croak("convert called without meta argument") unless defined($meta); + _init_grub($g, $root, $desc); + my $grub_conf = $desc->{boot}->{grub_conf}; + _init_selinux($g); - _init_augeas($g); + _init_augeas($g, $grub_conf); _init_modprobe_aliases($g, $desc); - _init_kernels($g, $root, $desc); + _init_kernels($g, $desc); my $modpath = _init_modpath($g); # Un-configure HV specific attributes which don't require a direct @@ -126,7 +129,7 @@ sub convert my $kernel = _configure_kernel($virtio, $g, $config, $desc, $meta); # Configure the rest of the system - _configure_console($g); + _configure_console($g, $grub_conf); _configure_display_driver($g); _remap_block_devices($meta, $virtio, $g, $desc); _configure_kernel_modules($g, $desc, $virtio, $modpath); @@ -154,18 +157,49 @@ sub _init_selinux $g->touch('/.autorelabel'); } +sub _init_grub +{ + my ($g, $root, $desc) = @_; + + # Find the path which needs to be prepended to paths in grub.conf to + # make them absolute + # Default to / (no prefix required) + my $grub = ""; + + # Look for the most specific mount point discovered + my %mounts = $g->inspect_get_mountpoints($root); + foreach my $path qw(/boot/grub /boot) { + if (exists($mounts{$path})) { + $grub = $path; + last; + } + } + + my $grub_conf; + foreach my $path qw(boot/grub/grub.conf boot/grub/menu.lst) { + if ($g->exists("/$path")) { + $grub_conf = $path; + last; + } + } + + $desc->{boot} ||= {}; + $desc->{boot}->{grub_fs} = $grub; + $desc->{boot}->{grub_conf} = $grub_conf; +} + sub _init_augeas { - my ($g) = @_; + my ($g, $grub_conf) = @_; # Initialise augeas eval { $g->aug_init("/", 1); - # Check if /boot/grub/menu.lst is included by the Grub lens + # Check grub_conf is included by the Grub lens my $found = 0; foreach my $incl ($g->aug_match("/augeas/load/Grub/incl")) { - if ($g->aug_get($incl) eq '/boot/grub/menu.lst') { + if ($g->aug_get($incl) eq $grub_conf) { $found = 1; last; } @@ -173,12 +207,11 @@ sub _init_augeas # If it wasn't there, add it unless ($found) { - $g->aug_set("/augeas/load/Grub/incl[last()+1]", - "/boot/grub/menu.lst"); - } + $g->aug_set("/augeas/load/Grub/incl[last()+1]", $grub_conf); - # Make augeas pick up the new configuration - $g->aug_load(); + # Make augeas pick up the new configuration + $g->aug_load(); + } }; # The augeas calls will die() on any error. @@ -437,7 +470,7 @@ sub _configure_kernel_modules # /dev/ttyS0 for RHEL 6 anyway. sub _configure_console { - my ($g) = @_; + my ($g, $grub_conf) = @_; # Look for gettys which use xvc0 or hvc0 # RHEL 6 doesn't use /etc/inittab, but this doesn't hurt @@ -462,7 +495,7 @@ sub _configure_console # Update any kernel console lines foreach my $augpath - ($g->aug_match("/files/boot/grub/menu.lst/title/kernel/console")) + ($g->aug_match("/files/$grub_conf/title/kernel/console")) { my $console = $g->aug_get($augpath); if ($console =~ /\b(x|h)vc0\b/) { @@ -524,10 +557,12 @@ sub _list_kernels { my ($g, $desc) = @_; + my $grub_conf = $desc->{boot}->{grub_conf}; + # Get the default kernel from grub if it's set my $default; eval { - $default = $g->aug_get('/files/boot/grub/menu.lst/default'); + $default = $g->aug_get("/files/$grub_conf/default"); }; # Doesn't matter if get fails @@ -537,10 +572,9 @@ sub _list_kernels # Look for a kernel, starting with the default my @paths; eval { - push(@paths, $g->aug_match("/files/boot/grub/menu.lst/". - "title[$default]/kernel")) + push(@paths, $g->aug_match("/files/$grub_conf/title[$default]/kernel")) if defined($default); - push(@paths, $g->aug_match('/files/boot/grub/menu.lst/title/kernel')); + push(@paths, $g->aug_match("/files/$grub_conf/title/kernel")); }; augeas_error($g, $@) if ($@); @@ -583,27 +617,14 @@ sub _list_kernels sub _init_kernels { - my ($g, $root, $desc) = @_; + my ($g, $desc) = @_; if ($desc->{os} eq "linux") { # Iterate over entries in grub.conf, populating $desc->{boot} # For every kernel we find, inspect it and add to $desc->{kernels} - # Find the path which needs to be prepended to paths in grub.conf to - # make them absolute - # Default to / (no prefix required) - my $grub = ""; - - # Look for the most specific mount point discovered - my %mounts = $g->inspect_get_mountpoints($root); - foreach my $path qw(/boot/grub /boot) { - if (exists($mounts{$path})) { - $grub = $path; - last; - } - } - - my $grub_conf = "/etc/grub.conf"; + my $grub = $desc->{boot}->{grub_fs}; + my $grub_conf = $desc->{boot}->{grub_conf}; my @boot_configs; @@ -689,16 +710,13 @@ sub _init_kernels } # Create the top level boot entry - my %boot; - $boot{configs} = \@configs; - $boot{grub_fs} = $grub; + $desc->{boot} ||= {}; + my $boot = $desc->{boot}; - # Add the default configuration - eval { - $boot{default} = $g->aug_get("/files/$grub_conf/default"); - }; + $boot->{configs} = \@configs; - $desc->{boot} = \%boot; + # Add the default configuration + eval { $boot->{default} = $g->aug_get("/files/$grub_conf/default") }; } } @@ -1739,14 +1757,13 @@ sub _check_grub { my ($version, $kernel, $g, $desc) = @_; - my $grubfs = $desc->{boot}->{grub_fs}; - my $prefix = $grubfs eq '/boot' ? '' : '/boot'; + my $grub_conf = $desc->{boot}->{grub_conf}; + my $grubfs = $desc->{boot}->{grub_fs}; + my $prefix = $grubfs eq '/boot' ? '' : '/boot'; # Nothing to do if there's already a grub entry return if eval { - foreach my $augpath - ($g->aug_match('/files/boot/grub/menu.lst/title/kernel')) - { + foreach my $augpath ($g->aug_match("/files/$grub_conf/title/kernel")) { return 1 if ($grubfs.$g->aug_get($augpath) eq $kernel); } @@ -1771,30 +1788,25 @@ sub _check_grub my $default; # Doesn't matter if there's no default - eval { - $default = $g->aug_get('/files/boot/grub/menu.lst/default'); - }; + eval { $default = $g->aug_get("/files/$grub_conf/default"); }; eval { if (defined($default)) { $g->aug_defvar('template', - '/files/boot/grub/menu.lst/title['.($default + 1).']'); + "/files/$grub_conf/title[".($default + 1).']'); } # If there's no default, take the first entry with a kernel else { - my ($match) = - $g->aug_match('/files/boot/grub/menu.lst/title/kernel'); - die("No template kernel found in grub.") unless(defined($match)); + my ($match) = $g->aug_match("/files/$grub_conf/title/kernel"); + die("No template kernel found in grub.") unless defined($match); $match =~ s/\/kernel$//; $g->aug_defvar('template', $match); } # Add a new title node at the end - $g->aug_defnode('new', - '/files/boot/grub/menu.lst/title[last()+1]', - $title); + $g->aug_defnode('new', "/files/$grub_conf/title[last()+1]", $title); # N.B. Don't change the order of root, kernel and initrd below, or the # guest will not boot. @@ -1827,9 +1839,7 @@ sub _check_grub my ($new) = $g->aug_match('$new'); $new =~ /\[(\d+)\]$/; - $g->aug_set('/files/boot/grub/menu.lst/default', - defined($1) ? $1 - 1 : 0); - + $g->aug_set("/files/$grub_conf/default", defined($1) ? $1 - 1 : 0); $g->aug_save(); }; augeas_error($g, $@) if ($@); @@ -2198,12 +2208,12 @@ sub _prepare_bootable $prefix = '/boot'; } - foreach my $kernel - ($g->aug_match('/files/boot/grub/menu.lst/title/kernel')) { + my $grub_conf = $desc->{boot}->{grub_conf}; + foreach my $kernel ($g->aug_match("/files/$grub_conf/title/kernel")) { if($g->aug_get($kernel) eq "$prefix/vmlinuz-$version") { # Ensure it's the default - $kernel =~ m{/files/boot/grub/menu.lst/title(?:\[(\d+)\])?/kernel} + $kernel =~ m{/files/$grub_conf/title(?:\[(\d+)\])?/kernel} or die($kernel); my $aug_index; @@ -2213,11 +2223,11 @@ sub _prepare_bootable $aug_index = 1; } - $g->aug_set('/files/boot/grub/menu.lst/default', - $aug_index - 1); + $g->aug_set("/files/$grub_conf/default", $aug_index - 1); # Get the initrd for this kernel - $initrd = $g->aug_get("/files/boot/grub/menu.lst/title[$aug_index]/initrd"); + $initrd = + $g->aug_get("/files/$grub_conf/title[$aug_index]/initrd"); $found = 1; last; -- 1.7.4.4
On Tue, Apr 26, 2011 at 05:03:46PM +0100, Matthew Booth wrote:
@@ -934,21 +939,23 @@ sub _get_application_owner
sub _unconfigure_hv { - my ($g, $desc) = @_; + my ($g, $root, $desc) = @_;
- _unconfigure_xen($g, $desc); - _unconfigure_vmware($g, $desc); + my @apps = $g->inspect_list_applications($root);
I can't tell without looking at the full context, but this might not be safe. inspect_list_applications will only work if the mountpoints are all mounted up. That's because this one call can read files out of the filesystem, where as all the inspect_get_* calls just return data that was cached in the handle (by inspect_os). Note the comment in the removed code below which says just about the same thing:
- my $root_dev = $roots[0]; - - # Mount up the disks. - my %fses = $g->inspect_get_mountpoints ($root_dev); - my @fses = sort { length $a <=> length $b } keys %fses; - foreach (@fses) { - eval { $g->mount_options ("", $fses{$_}, $_) }; - print __x("{e} (ignored)\n", e => $@) if $@; - } - - # Construct the "$desc" hashref which contains the main features - # found by inspection. - my %desc; - - $desc{root_device} = $root_dev; - - $desc{os} = $g->inspect_get_type ($root_dev); - $desc{distro} = $g->inspect_get_distro ($root_dev); - $desc{product_name} = $g->inspect_get_product_name ($root_dev); - $desc{product_variant} = $g->inspect_get_product_variant ($root_dev); - $desc{major_version} = $g->inspect_get_major_version ($root_dev); - $desc{minor_version} = $g->inspect_get_minor_version ($root_dev); - $desc{arch} = $g->inspect_get_arch ($root_dev); - - # Notes: - # (1) Filesystems have to be mounted for this to work. Do not - # move this code over the filesystem mounting code above. - # (2) For RPM-based distros, new libguestfs inspection code - # is only able to populate the 'app_name' field (old Perl code - # populated a lot more). Fortunately this is the only field - # that the code currently uses. - my @apps = $g->inspect_list_applications ($root_dev); - $desc{apps} = \@apps; - - return \%desc; + return $roots[0]; }
References: http://libguestfs.org/guestfs.3.html#inspection http://libguestfs.org/guestfs.3.html#guestfs_inspect_list_applications The rest seems all fine. I'd be a lot happier if our language/ compiler was strongly type-checking all of these changes though ... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
On 26/04/11 17:41, Richard W.M. Jones wrote:
On Tue, Apr 26, 2011 at 05:03:46PM +0100, Matthew Booth wrote:
@@ -934,21 +939,23 @@ sub _get_application_owner
sub _unconfigure_hv { - my ($g, $desc) = @_; + my ($g, $root, $desc) = @_;
- _unconfigure_xen($g, $desc); - _unconfigure_vmware($g, $desc); + my @apps = $g->inspect_list_applications($root);
I can't tell without looking at the full context, but this might not be safe. inspect_list_applications will only work if the mountpoints are all mounted up. That's because this one call can read files out of the filesystem, where as all the inspect_get_* calls just return data that was cached in the handle (by inspect_os).
This is fine. The above code is right in the middle of the conversion. No mount points are changed after they're mounted.
Note the comment in the removed code below which says just about the same thing:
- my $root_dev = $roots[0]; - - # Mount up the disks. - my %fses = $g->inspect_get_mountpoints ($root_dev); - my @fses = sort { length $a<=> length $b } keys %fses; - foreach (@fses) { - eval { $g->mount_options ("", $fses{$_}, $_) }; - print __x("{e} (ignored)\n", e => $@) if $@; - } - - # Construct the "$desc" hashref which contains the main features - # found by inspection. - my %desc; - - $desc{root_device} = $root_dev; - - $desc{os} = $g->inspect_get_type ($root_dev); - $desc{distro} = $g->inspect_get_distro ($root_dev); - $desc{product_name} = $g->inspect_get_product_name ($root_dev); - $desc{product_variant} = $g->inspect_get_product_variant ($root_dev); - $desc{major_version} = $g->inspect_get_major_version ($root_dev); - $desc{minor_version} = $g->inspect_get_minor_version ($root_dev); - $desc{arch} = $g->inspect_get_arch ($root_dev); - - # Notes: - # (1) Filesystems have to be mounted for this to work. Do not - # move this code over the filesystem mounting code above. - # (2) For RPM-based distros, new libguestfs inspection code - # is only able to populate the 'app_name' field (old Perl code - # populated a lot more). Fortunately this is the only field - # that the code currently uses. - my @apps = $g->inspect_list_applications ($root_dev); - $desc{apps} = \@apps; - - return \%desc; + return $roots[0]; }
References: http://libguestfs.org/guestfs.3.html#inspection http://libguestfs.org/guestfs.3.html#guestfs_inspect_list_applications
The rest seems all fine. I'd be a lot happier if our language/ compiler was strongly type-checking all of these changes though ...
You and me both :) Matt -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
participants (2)
-
Matthew Booth -
Richard W.M. Jones