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

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

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.

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]