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@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