[Libguestfs] [PATCH 1/2] Fix remapping of block devices
Matthew Booth
mbooth at redhat.com
Fri Feb 19 16:42:56 UTC 2010
We were assuming that guests wouldn't use multiple interfaces, or if they did,
drive letters wouldn't clash between the interfaces. An ESX guest with a CDROM
device breaks this because hard disks will be presented a SCSI, starting at sda,
and CDROM devices are presented as IDE, starting at hda. This is in contrast to
QEMU, which starts at hdc by default.
Firstly, this change causes disks and CDROM devices to be treated differently.
CDROM devices are changed to use IDE, regardless of what interface they were
using before. This also limits the maximum number of CDROM devices in a guest to
4. CDROM devices after 4 will be dropped, and a warning emitted.
Secondly, it is smarter about renaming IDE and SCSI devices, which may be merged
in the guest configuration.
Thirdly, it renames devices consistently based on the order they are listed in
the domain XML.
---
lib/Sys/VirtV2V/Connection.pm | 40 ++++++++---
lib/Sys/VirtV2V/Converter.pm | 86 +++++++++++++++++++-----
lib/Sys/VirtV2V/Converter/Linux.pm | 41 ++---------
lib/Sys/VirtV2V/GuestOS/RedHat.pm | 132 ++++++++++++++++++++++++++++++------
v2v/virt-v2v.pl | 9 ++-
5 files changed, 222 insertions(+), 86 deletions(-)
diff --git a/lib/Sys/VirtV2V/Connection.pm b/lib/Sys/VirtV2V/Connection.pm
index 501772b..46bd020 100644
--- a/lib/Sys/VirtV2V/Connection.pm
+++ b/lib/Sys/VirtV2V/Connection.pm
@@ -38,7 +38,8 @@ Sys::VirtV2V::Connection - Obtain domain metadata
$conn = Sys::VirtV2V::Connection::LibVirt->new($uri, $name, $pool);
$dom = $conn->get_dom();
- @storage = $conn->get_local_storage();
+ $storage = $conn->get_storage_paths();
+ $devices = $conn->get_storage_devices();
=head1 DESCRIPTION
@@ -53,18 +54,33 @@ directly. Use one of the subclasses:
=over
-=item get_local_storage
+=item get_storage_paths
-Return a list of the domain's storage devices. The returned list contains local
-paths.
+Return an arrayref of local paths to the guest's storage devices. This list is
+guaranteed to be in the same order as the list returned by get_storage_devices.
=cut
-sub get_local_storage
+sub get_storage_paths
{
my $self = shift;
- return @{$self->{storage}};
+ return $self->{paths};
+}
+
+=item get_storage_devices
+
+Return an arrayref of libvirt device names for the guest's storage prior to
+conversion. This list is guaranteed to be in the same order as the list returned
+by get_storage_paths.
+
+=cut
+
+sub get_storage_devices
+{
+ my $self = shift;
+
+ return $self->{devices};
}
=item get_dom()
@@ -94,8 +110,10 @@ sub _storage_iterate
my $dom = $self->get_dom();
- # Create a hash of guest devices to their paths
- my @storage;
+ # An list of local paths to guest storage
+ my @paths;
+ # A list of libvirt target device names
+ my @devices;
foreach my $disk ($dom->findnodes('/domain/devices/disk')) {
my ($source_e) = $disk->findnodes('source');
@@ -167,11 +185,13 @@ sub _storage_iterate
}
}
- push(@storage, $path);
+ push(@paths, $path);
+ push(@devices, $target->getNodeValue());
}
}
- $self->{storage} = \@storage;
+ $self->{paths} = \@paths;
+ $self->{devices} = \@devices;
}
=back
diff --git a/lib/Sys/VirtV2V/Converter.pm b/lib/Sys/VirtV2V/Converter.pm
index cba6db8..a306103 100644
--- a/lib/Sys/VirtV2V/Converter.pm
+++ b/lib/Sys/VirtV2V/Converter.pm
@@ -20,6 +20,8 @@ package Sys::VirtV2V::Converter;
use strict;
use warnings;
+use Carp;
+
use Module::Pluggable sub_name => 'modules',
search_path => ['Sys::VirtV2V::Converter'],
require => 1;
@@ -40,7 +42,7 @@ Sys::VirtV2V::Converter - Convert a guest to run on KVM
use Sys::VirtV2V::Converter;
my $guestos = Sys::VirtV2V::GuestOS->new($g, $os, $dom, $config);
- Sys::VirtV2V::Converter->convert($vmm, $guestos, $config, $dom, $os);
+ Sys::VirtV2V::Converter->convert($vmm, $guestos, $config, $dom, $os, $devices);
=head1 DESCRIPTION
@@ -122,6 +124,11 @@ An XML::DOM object resulting from parsing the guests's libvirt domain XML.
The OS description returned by Sys::Guestfs::Lib.
+=item devices
+
+An arrayref of libvirt storage device names, in the order they will be presented
+to the guest.
+
=back
=cut
@@ -130,19 +137,20 @@ sub convert
{
my $class = shift;
- my ($vmm, $guestos, $config, $dom, $desc) = @_;
+ my ($vmm, $guestos, $config, $dom, $desc, $devices) = @_;
carp("convert called without vmm argument") unless defined($vmm);
carp("convert called without guestos argument") unless defined($guestos);
carp("convert called without config argument") unless defined($config);
carp("convert called without dom argument") unless defined($dom);
carp("convert called without desc argument") unless defined($desc);
+ carp("convert called without devices argument") unless defined($devices);
my $guestcaps;
# Find a module which can convert the guest and run it
foreach my $module ($class->modules()) {
if($module->can_handle($desc)) {
- $guestcaps = $module->convert($vmm, $guestos, $dom, $desc);
+ $guestcaps = $module->convert($vmm, $guestos, $desc, $devices);
last;
}
}
@@ -154,7 +162,7 @@ sub convert
_map_networks($dom, $config);
# Convert the metadata
- _convert_metadata($vmm, $dom, $desc, $guestcaps);
+ _convert_metadata($vmm, $dom, $desc, $devices, $guestcaps);
my ($name) = $dom->findnodes('/domain/name/text()');
$name = $name->getNodeValue();
@@ -170,7 +178,7 @@ sub convert
sub _convert_metadata
{
- my ($vmm, $dom, $desc, $guestcaps) = @_;
+ my ($vmm, $dom, $desc, $devices, $guestcaps) = @_;
my $arch = $guestcaps->{arch};
my $virtio = $guestcaps->{virtio};
@@ -191,8 +199,11 @@ sub _convert_metadata
# Remove any configuration related to a PV kernel bootloader
_unconfigure_bootloaders($dom);
- # Configure network and block drivers
- _configure_drivers($dom, $virtio);
+ # Update storage devices and drivers
+ _configure_storage($dom, $devices, $virtio);
+
+ # Configure network drivers
+ _configure_network($dom, $virtio);
# Ensure guest has a standard set of default devices
_configure_default_devices($dom, $default_dom);
@@ -383,24 +394,65 @@ sub _unconfigure_bootloaders
}
}
-sub _configure_drivers
+sub _configure_storage
{
- my ($dom, $virtio) = @_;
+ my ($dom, $devices, $virtio) = @_;
- # Convert disks
- # N.B. <disk> is required to have a <target> element
+ my $prefix = $virtio ? 'vd' : 'sd';
- # Convert alternate bus specifications
- foreach my $bus ($dom->findnodes('/domain/devices/disk/target/@bus')) {
- $bus->setNodeValue($virtio ? 'virtio' : 'scsi');
+ my $suffix = 'a';
+ foreach my $device (@$devices) {
+ my ($target) = $dom->findnodes("/domain/devices/disk[\@device='disk']/".
+ "target[\@dev='$device']");
+
+ die(user_message(__x("Previously detected drive {drive} is no longer ".
+ "present in domain XML: {xml}",
+ drive => $device,
+ xml => $dom->toString())))
+ unless (defined($target));
+
+ $target->setAttribute('bus', $virtio ? 'virtio' : 'scsi');
+ $target->setAttribute('dev', $prefix.$suffix);
+ $suffix++; # Perl magic means 'z'++ == 'aa'
}
- # Add an explicit bus specification to targets without one
+ # Convert the first 4 CDROM drives to IDE, and remove the rest
+ $suffix = 'a';
+ my $i = 0;
+ my @removed = ();
foreach my $target
- ($dom->findnodes('/domain/devices/disk/target[not(@bus)]'))
+ ($dom->findnodes("/domain/devices/disk[\@device='cdrom']/target"))
{
- $target->setAttribute('bus', $virtio ? 'virtio' : 'scsi');
+ if ($i < 4) {
+ $target->setAttribute('bus', 'ide');
+ $target->setAttribute('dev', "hd$suffix");
+ $suffix++;
+ } else {
+ push(@removed, $target->getAttribute('dev'));
+
+ my $disk = $target->getParentNode();
+ $disk->getParentNode()->removeChild($disk);
+ }
+ $i++;
+ }
+
+ if (@removed > 0) {
+ print user_message(__x("WARNING: Only 4 CDROM drives are supported. ".
+ "The following CDROM drives have been removed: ".
+ "{list}",
+ list => join(' ', @removed)));
+ }
+
+ # As we just changed and unified all their underlying controllers, device
+ # addresses are no longer relevant
+ foreach my $address ($dom->findnodes('/domain/devices/disk/address')) {
+ $address->getParentNode()->removeChild($address);
}
+}
+
+sub _configure_network
+{
+ my ($dom, $virtio) = @_;
# Convert network adapters
# N.B. <interface> is not required to have a <model> element, but <model>
diff --git a/lib/Sys/VirtV2V/Converter/Linux.pm b/lib/Sys/VirtV2V/Converter/Linux.pm
index 1186430..c10c963 100644
--- a/lib/Sys/VirtV2V/Converter/Linux.pm
+++ b/lib/Sys/VirtV2V/Converter/Linux.pm
@@ -83,14 +83,15 @@ A Sys::Virt handle to the target libvirt.
An initialised Sys::VirtV2V::GuestOS for manipulating the guest OS>.
-=item dom
-
-A parsed XML::DOM of the guest's libvirt domain XML prior to conversion.
-
=item desc
A description of the guest OS as returned by Sys::Guestfs::Lib.
+=item devices
+
+An arrayref of libvirt storage device names, in the order they will be presented
+to the guest.
+
=back
=cut
@@ -99,11 +100,11 @@ sub convert
{
my $class = shift;
- my ($vmm, $guestos, $dom, $desc) = @_;
+ my ($vmm, $guestos, $desc, $devices) = @_;
carp("convert called without vmm argument") unless defined($vmm);
carp("convert called without guestos argument") unless defined($guestos);
- carp("convert called without dom argument") unless defined($dom);
carp("convert called without desc argument") unless defined($desc);
+ carp("convert called without devices argument") unless defined($devices);
# Un-configure HV specific attributes which don't require a direct
# replacement
@@ -117,7 +118,7 @@ sub convert
# Configure the rest of the system
_configure_display_driver($guestos, $virtio);
- _remap_block_devices($guestos, $dom, $desc, $virtio);
+ $guestos->remap_block_devices($devices, $virtio);
_configure_kernel_modules($guestos, $desc, $virtio);
_configure_boot($guestos, $kernel, $virtio);
@@ -130,32 +131,6 @@ sub convert
return \%guestcaps;
}
-sub _remap_block_devices
-{
- my ($guestos, $dom, $desc, $virtio) = @_;
-
- my %map = ();
-
- # Prefix is vd for virtio or sd for scsi
- my $prefix = $virtio ? 'vd' : 'sd';
-
- # Look for devices specified in the device metadata
- foreach my $dev ($dom->findnodes('/domain/devices/disk/target/@dev')) {
- if($dev->getNodeValue() =~ m{^(sd|hd|xvd)([a-z]+)\d*$}) {
- $map{"$1$2"} = "$prefix$2";
-
- # A guest might present an IDE disk as SCSI
- if($1 eq 'hd') {
- $map{"sd$2"} = "$prefix$2";
- }
-
- $dev->setNodeValue("$prefix$2");
- }
- }
-
- $guestos->remap_block_devices(\%map);
-}
-
sub _configure_kernel_modules
{
my ($guestos, $desc, $virtio) = @_;
diff --git a/lib/Sys/VirtV2V/GuestOS/RedHat.pm b/lib/Sys/VirtV2V/GuestOS/RedHat.pm
index 9149cfc..6adf8b8 100644
--- a/lib/Sys/VirtV2V/GuestOS/RedHat.pm
+++ b/lib/Sys/VirtV2V/GuestOS/RedHat.pm
@@ -944,7 +944,7 @@ sub _ensure_transfer_mounted
$g->mount_ro($transfer, $self->{transfer_mount});
}
-=item remap_block_devices(map)
+=item remap_block_devices(devices, virtio)
See BACKEND INTERFACE in L<Sys::VirtV2V::GuestOS> for details.
@@ -953,32 +953,120 @@ See BACKEND INTERFACE in L<Sys::VirtV2V::GuestOS> for details.
sub remap_block_devices
{
my $self = shift;
- my ($map) = @_;
+ my ($devices, $virtio) = @_;
- my $g = $self->{g};
+ my $g = $self->{g};
+ my $desc = $self->{desc};
- # 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('/files/etc/fstab/*/spec')) {
- my $device = $g->aug_get($spec);
-
- next unless($device =~ m{^/dev/((?:sd|hd|xvd)(?:[a-z]+))(\d*)});
-
- if(defined($map->{$1})) {
- my $target = '/dev/'.$map->{$1};
- $target .= $2 if(defined($2));
- $g->aug_set($spec, $target);
- } else {
- print STDERR user_message(__x("No mapping found for block ".
- "device {device}",
- device => $device));
+ # $devices contains an order list of devices, as named by the host. Because
+ # libvirt uses a similar naming scheme to Linux, these will mostly be the
+ # same names as used by the guest. However, if the guest is using libata,
+ # IDE drives could be renamed.
+
+ # Look for IDE and SCSI devices in fstab for the guest
+ my %guestif;
+ foreach my $spec ($g->aug_match('/files/etc/fstab/*/spec')) {
+ my $device = $g->aug_get($spec);
+
+ next unless($device =~ m{^/dev/(sd|hd)([a-z]+)});
+ $guestif{$1} ||= {};
+ $guestif{$1}->{$1.$2} = 1;
+ }
+
+ # If fstab contains references to sdX, these could refer to IDE or SCSI
+ # devices. Need to look at the domain config for clues.
+ if (exists($guestif{sd})) {
+ # Look for IDE and SCSI devices from the domain definition
+ my %domainif;
+ foreach my $device (@$devices) {
+ foreach my $type ('hd', 'sd') {
+ if ($device =~ m{^$type([a-z]+)}) {
+ $domainif{$type} ||= {};
+ $domainif{$type}->{$device} = 1;
+ }
}
}
- };
- # Propagate augeas failure
- die($@) if($@);
+ # If domain defines both IDE and SCSI drives, and fstab contains
+ # references on sdX, but not hdX, we don't know if the guest is using
+ # libata or not. This means that we don't know if sdX in fstab refers
+ # to hdX or sdX in the domain. Warn and assume that libata device
+ # renaming is not in use.
+ if (exists($domainif{hd}) && exists($domainif{sd}) &&
+ !exists($guestif{hd}))
+ {
+ print STDERR user_message(__"WARNING: Unable to determine whether ".
+ "sdX devices in /etc/fstab refer to ".
+ "IDE or SCSI devices. Assuming they ".
+ "refer to SCSI devices. /etc/fstab ".
+ "may be incorrect after conversion if ".
+ "guest uses libata.");
+ }
+
+ # If we've got only IDE devices in the domain, and only sdX devices in
+ # fstab, the guest is renaming them.
+ elsif (exists($domainif{hd}) && !exists($domainif{sd}) &&
+ !exists($guestif{hd}))
+ {
+ my %map;
+
+ my $letter = 'a';
+ foreach my $old (sort { _drivecmp('hd', $a, $b) }
+ keys(%{$domainif{hd}}))
+ {
+ $map{$old} = "sd$letter";
+ $letter++;
+ }
+
+ map { $_ = $map{$_} } @$devices;
+ }
+
+ # Otherwise we leave it alone
+ }
+
+ # We now assume that $devices contains an ordered list of device names, as
+ # used by the guest. Create a map of old guest device names to new guest
+ # device names.
+ my %map;
+
+ # Everything will be converted to either vdX or sdX
+ my $prefix = $virtio ? 'vd' : 'sd';
+ my $letter = 'a';
+ foreach my $device (@$devices) {
+ $map{$device} = $prefix.$letter;
+ $letter++;
+ }
+
+ # Go through fstab again, updating bare device references
+ foreach my $spec ($g->aug_match('/files/etc/fstab/*/spec')) {
+ my $device = $g->aug_get($spec);
+
+ next unless($device =~ m{^/dev/([a-z]+)(\d*)});
+ my $name = $1;
+ my $part = $2;
+
+ next unless(exists($map{$name}));
+
+ $g->aug_set($spec, "/dev/".$map{$name}.$part);
+ }
+ $g->aug_save();
+}
+
+sub _drivecmp
+{
+ my ($prefix, $a, $b) = @_;
+
+ map {
+ $_ =~ /^$prefix([a-z]+)/ or die("drive $_ doesn't have prefix $prefix");
+ $_ = $1;
+ } ($a, $b);
+
+ return -1 if (length($a) < length($b));
+ return 1 if (length($a) > length($b));
+
+ return -1 if ($a lt $b);
+ return 0 if ($a eq $b);
+ return 1;
}
=item prepare_bootable(version [, module, module, ...])
diff --git a/v2v/virt-v2v.pl b/v2v/virt-v2v.pl
index eb5ba1f..7b389e4 100755
--- a/v2v/virt-v2v.pl
+++ b/v2v/virt-v2v.pl
@@ -262,13 +262,13 @@ my $dom = $conn->get_dom();
exit(1) unless(defined($dom));
# Get a list of the guest's transfered storage devices
-my @storage = $conn->get_local_storage();
+my $storage = $conn->get_storage_paths();
# Create the transfer iso if required
my $transferiso = get_transfer_iso($config, $config_file);
# Open a libguestfs handle on the guest's storage devices
-my $g = get_guestfs_handle(\@storage, $transferiso);
+my $g = get_guestfs_handle($storage, $transferiso);
$SIG{'INT'} = \&close_guest_handle;
$SIG{'QUIT'} = \&close_guest_handle;
@@ -280,7 +280,8 @@ my $os = inspect_guest($g);
my $guestos = Sys::VirtV2V::GuestOS->new($g, $os, $dom, $config);
# Modify the guest and its metadata for the target hypervisor
-Sys::VirtV2V::Converter->convert($vmm, $guestos, $config, $dom, $os);
+Sys::VirtV2V::Converter->convert($vmm, $guestos, $config, $dom, $os,
+ $conn->get_storage_devices());
$vmm->define_domain($dom->toString());
@@ -393,7 +394,7 @@ sub get_guestfs_handle
# Enable autosync to defend against data corruption on unclean shutdown
$g->set_autosync(1);
- $g->launch ();
+ $g->launch();
return $g;
}
--
1.6.6
More information about the Libguestfs
mailing list