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

Matthew Booth mbooth at redhat.com
Tue Apr 26 16:49:07 UTC 2011


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




More information about the Libguestfs mailing list