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

[Richard W.M. Jones]

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.

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).

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.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top