[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH 1/7] Push $desc creation into Sys::VirtConvert::Converter->convert



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]