[Libguestfs] [PATCH 3/4] tools: Use C API for inspection

[Matthew Booth]

On 28/10/10 13:54, Richard W.M. Jones wrote:
> On Thu, Oct 28, 2010 at 01:52:24PM +0100, Matthew Booth wrote:
>> On 28/10/10 12:37, Richard W.M. Jones wrote:
>>> > From 6e19d032d0d28e62f38223b6cc05dc36d9352add Mon Sep 17 00:00:00 2001
>>> From: Richard W.M. Jones<rjones at redhat.com>
>>> Date: Wed, 27 Oct 2010 16:06:11 +0100
>>> Subject: [PATCH 3/4] tools: Use C API for inspection (RHBZ#642930).
>>>
>>> Update the following tools to use the C API for inspection:
>>>
>>>   - virt-cat
>>>   - virt-edit
>>>   - virt-ls
>>>   - virt-tar
>>>   - virt-win-reg
>>>
>>> None of the tools in the tools/ directory now use the deprecated
>>> Perl inspection APIs.
>>> ---
>>>   tools/virt-cat     |   37 +++++++++++++++++--------------------
>>>   tools/virt-edit    |   35 ++++++++++++++++-------------------
>>>   tools/virt-ls      |   39 ++++++++++++++++++---------------------
>>>   tools/virt-tar     |   38 ++++++++++++++++++--------------------
>>>   tools/virt-win-reg |   39 ++++++++++++++++++---------------------
>>>   5 files changed, 87 insertions(+), 101 deletions(-)
>>>
>>> diff --git a/tools/virt-cat b/tools/virt-cat
>>> index 66806a1..546e85c 100755
>>> --- a/tools/virt-cat
>>> +++ b/tools/virt-cat
>>> @@ -157,22 +156,20 @@ if ($uri) {
>>>
>>>   $g->launch ();
>>>
>>> -# List of possible filesystems.
>>> -my @partitions = get_partitions ($g);
>>> -
>>> -# Now query each one to build up a picture of what's in it.
>>> -my %fses =
>>> -    inspect_all_partitions ($g, \@partitions,
>>> -      use_windows_registry =>   0);
>>> -
>>> -my $oses = inspect_operating_systems ($g, \%fses);
>>> -
>>> -my @roots = keys %$oses;
>>> -die __"multiboot operating systems are not supported by virt-cat" if @roots>   1;
>>> -my $root_dev = $roots[0];
>>> -
>>> -my $os = $oses->{$root_dev};
>>> -mount_operating_system ($g, $os);
>>> +my @roots = $g->inspect_os ();
>>> +if (@roots == 0) {
>>> +    die __x("{prog}: No operating system could be detected inside this disk image.\n\nThis may be because the file is not a disk image, or is not a virtual machine\nimage, or because the OS type is not understood by libguestfs.\n\nIf you feel this is an error, please file a bug report including as much\ninformation about the disk image as possible.\n",
>>> +            prog =>   basename ($0));
>>> +}
>>> +if (@roots>   1) {
>>> +    die __x("{prog}: multiboot operating systems are not supported.\n",
>>> +            prog =>   basename ($0))
>>> +}
>>> +my %fses = $g->inspect_get_mountpoints ($roots[0]);
>>> +my @fses = sort { length $a<=>   length $b } keys %fses;
>>> +foreach (@fses) {
>>> +    $g->mount_ro ($fses{$_}, $_);
>>> +}
>>>
>>>   # Allow this to fail in case eg. the file does not exist.
>>>   # NB:https://bugzilla.redhat.com/show_bug.cgi?id=501888
>>
>> This is pretty long for boiler plate code which will be used in all
>> the virt tools, including virt-v2v. This should live in a high-level
>> library function which:
>>
>> 1. Inspects a guest.
>> 2. Mounts all its storage, optionally read-only.
>
> That's how I did it first time, but:
>
>> It wouldn't necessarily need to form part of a published API
>
> this is the problem.  If it's in Sys::Guestfs::Lib it's pretty much
> a published API.

I'd make it a published API, then.

> Also the boilerplate isn't quite the same each time, particularly
> w.r.t whether we allow multiple roots (virt-inspector) or require
> writable mounts (eg. virt-edit).

There are 2 parameters:

inspect_and_mount(readonly, allow_multiroot);

> I don't think there's a good answer here.
>
> At least the error messages are the same so they only have to be
> translated once.

As long as you ensure they all remain identical in 6(?) different places.

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