6:35 a.m.
This is a fix for:
https://bugzilla.redhat.com/show_bug.cgi?id=642930
These patches remove all use of the Perl inspection API (although
leaving the code, since it is used by virt-v2v).
The tools have been modified so they use the core inspection API.
Virt inspector has been basically rewritten to remove excess cruft and
to only use the core API (see the final commit for more details).
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
6:36 a.m.
New subject: [PATCH 1/4] Sys::Guestfs::Lib: deprecate inspection functions
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
7:29 a.m.
New subject: [PATCH 1/4] Sys::Guestfs::Lib: deprecate inspection functions
On 28/10/10 12:36, Richard W.M. Jones wrote:
> From b3394208522cd993788aa642e0082e6e5296fe02 Mon Sep 17 00:00:00
2001
From: Richard W.M. Jones<rjones(a)redhat.com>
Date: Wed, 27 Oct 2010 15:47:45 +0100
Subject: [PATCH 1/4] Sys::Guestfs::Lib: deprecate inspection functions (RHBZ#642930).
Deprecate the guest inspection functions in this module, remove
documentation, and point users at the core API functions instead.
However we will keep the code here since it is used by virt-v2v
and virt-inspector.
---
perl/lib/Sys/Guestfs/Lib.pm | 419 +++----------------------------------------
1 files changed, 29 insertions(+), 390 deletions(-)
I'm not convinced about this. By all means mark the functions as
deprecated, but I wouldn't remove the documentation until they're
actually removed.
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
7:50 a.m.
New subject: [PATCH 1/4] Sys::Guestfs::Lib: deprecate inspection functions
On Thu, Oct 28, 2010 at 01:29:14PM +0100, Matthew Booth wrote:
On 28/10/10 12:36, Richard W.M. Jones wrote:
>>From b3394208522cd993788aa642e0082e6e5296fe02 Mon Sep 17 00:00:00 2001
>From: Richard W.M. Jones<rjones(a)redhat.com>
>Date: Wed, 27 Oct 2010 15:47:45 +0100
>Subject: [PATCH 1/4] Sys::Guestfs::Lib: deprecate inspection functions
(RHBZ#642930).
>
>Deprecate the guest inspection functions in this module, remove
>documentation, and point users at the core API functions instead.
>
>However we will keep the code here since it is used by virt-v2v
>and virt-inspector.
>---
> perl/lib/Sys/Guestfs/Lib.pm | 419 +++----------------------------------------
> 1 files changed, 29 insertions(+), 390 deletions(-)
I'm not convinced about this. By all means mark the functions as
deprecated, but I wouldn't remove the documentation until they're
actually removed.
Trouble is, if we leave the documentation there people might use it ...
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
7:53 a.m.
New subject: [PATCH 1/4] Sys::Guestfs::Lib: deprecate inspection functions
On 28/10/10 13:50, Richard W.M. Jones wrote:
On Thu, Oct 28, 2010 at 01:29:14PM +0100, Matthew Booth wrote:
> On 28/10/10 12:36, Richard W.M. Jones wrote:
>> > From b3394208522cd993788aa642e0082e6e5296fe02 Mon Sep 17 00:00:00 2001
>> From: Richard W.M. Jones<rjones(a)redhat.com>
>> Date: Wed, 27 Oct 2010 15:47:45 +0100
>> Subject: [PATCH 1/4] Sys::Guestfs::Lib: deprecate inspection functions
(RHBZ#642930).
>>
>> Deprecate the guest inspection functions in this module, remove
>> documentation, and point users at the core API functions instead.
>>
>> However we will keep the code here since it is used by virt-v2v
>> and virt-inspector.
>> ---
>> perl/lib/Sys/Guestfs/Lib.pm | 419 +++----------------------------------------
>> 1 files changed, 29 insertions(+), 390 deletions(-)
>
> I'm not convinced about this. By all means mark the functions as
> deprecated, but I wouldn't remove the documentation until they're
> actually removed.
Trouble is, if we leave the documentation there people might use it ...
Well, they'll be getting what they deserve when it goes away :) The
trouble with removing the documentation for functions which are still in
use is that it makes it harder to understand existing code.
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
7:57 a.m.
New subject: [PATCH 1/4] Sys::Guestfs::Lib: deprecate inspection functions
On Thu, Oct 28, 2010 at 01:53:35PM +0100, Matthew Booth wrote:
On 28/10/10 13:50, Richard W.M. Jones wrote:
>On Thu, Oct 28, 2010 at 01:29:14PM +0100, Matthew Booth wrote:
>>On 28/10/10 12:36, Richard W.M. Jones wrote:
>>>> From b3394208522cd993788aa642e0082e6e5296fe02 Mon Sep 17 00:00:00 2001
>>>From: Richard W.M. Jones<rjones(a)redhat.com>
>>>Date: Wed, 27 Oct 2010 15:47:45 +0100
>>>Subject: [PATCH 1/4] Sys::Guestfs::Lib: deprecate inspection functions
(RHBZ#642930).
>>>
>>>Deprecate the guest inspection functions in this module, remove
>>>documentation, and point users at the core API functions instead.
>>>
>>>However we will keep the code here since it is used by virt-v2v
>>>and virt-inspector.
>>>---
>>> perl/lib/Sys/Guestfs/Lib.pm | 419
+++----------------------------------------
>>> 1 files changed, 29 insertions(+), 390 deletions(-)
>>
>>I'm not convinced about this. By all means mark the functions as
>>deprecated, but I wouldn't remove the documentation until they're
>>actually removed.
>
>Trouble is, if we leave the documentation there people might use it ...
Well, they'll be getting what they deserve when it goes away :) The
trouble with removing the documentation for functions which are
still in use is that it makes it harder to understand existing code.
Maybe encourages people to rewrite the code :-)
We have to keep the functions here because they're part of RHEL 6, and
I'll probably import the old virt-inspector code into 6.1 so that it
doesn't change dramatically.
But could these functions (and documentation, if you like) be copied
wholesale into virt-v2v? They are essentially only used by virt-v2v
now, and it's easier if this code is simply a part of virt-v2v.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
8 a.m.
New subject: [PATCH 1/4] Sys::Guestfs::Lib: deprecate inspection functions
On 28/10/10 13:57, Richard W.M. Jones wrote:
On Thu, Oct 28, 2010 at 01:53:35PM +0100, Matthew Booth wrote:
> On 28/10/10 13:50, Richard W.M. Jones wrote:
>> On Thu, Oct 28, 2010 at 01:29:14PM +0100, Matthew Booth wrote:
>>> On 28/10/10 12:36, Richard W.M. Jones wrote:
>>>>> From b3394208522cd993788aa642e0082e6e5296fe02 Mon Sep 17 00:00:00
2001
>>>> From: Richard W.M. Jones<rjones(a)redhat.com>
>>>> Date: Wed, 27 Oct 2010 15:47:45 +0100
>>>> Subject: [PATCH 1/4] Sys::Guestfs::Lib: deprecate inspection functions
(RHBZ#642930).
>>>>
>>>> Deprecate the guest inspection functions in this module, remove
>>>> documentation, and point users at the core API functions instead.
>>>>
>>>> However we will keep the code here since it is used by virt-v2v
>>>> and virt-inspector.
>>>> ---
>>>> perl/lib/Sys/Guestfs/Lib.pm | 419
+++----------------------------------------
>>>> 1 files changed, 29 insertions(+), 390 deletions(-)
>>>
>>> I'm not convinced about this. By all means mark the functions as
>>> deprecated, but I wouldn't remove the documentation until they're
>>> actually removed.
>>
>> Trouble is, if we leave the documentation there people might use it ...
>
> Well, they'll be getting what they deserve when it goes away :) The
> trouble with removing the documentation for functions which are
> still in use is that it makes it harder to understand existing code.
Maybe encourages people to rewrite the code :-)
We have to keep the functions here because they're part of RHEL 6, and
I'll probably import the old virt-inspector code into 6.1 so that it
doesn't change dramatically.
But could these functions (and documentation, if you like) be copied
wholesale into virt-v2v? They are essentially only used by virt-v2v
now, and it's easier if this code is simply a part of virt-v2v.
I'll just remove them from virt-v2v rather than do this.
I'd mark them deprecated and leave them in place, with documentation,
for at least a release cycle.
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
8:07 a.m.
New subject: [PATCH 1/4] Sys::Guestfs::Lib: deprecate inspection functions
On Thu, Oct 28, 2010 at 02:00:03PM +0100, Matthew Booth wrote:
I'll just remove them from virt-v2v rather than do this.
But virt-v2v still uses these functions? (Well, I know it does, at
least, it does in the code that is in the git repo).
So virt-v2v can't just remove the use of these functions, without a
fair bit of rewriting. Hence the suggestion to copy the functions
into virt-v2v, allowing you to rewrite or refactor code at your own
pace.
Rich.
PS. they would still be in Sys::Guestfs::Lib for the foreseeable future.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
6:37 a.m.
New subject: [PATCH 2/4] New API: inspect-get-windows-systemroot to get systemroot.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://et.redhat.com/~rjones/libguestfs/
See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html
7:43 a.m.
New subject: [PATCH 2/4] New API: inspect-get-windows-systemroot to get systemroot.
On 28/10/10 12:37, Richard W.M. Jones wrote:
> From 2b38ae9db560523c1b417e5888295cb1016ee930 Mon Sep 17 00:00:00
2001
From: Richard W.M. Jones<rjones(a)redhat.com>
Date: Wed, 27 Oct 2010 16:47:33 +0100
Subject: [PATCH 2/4] New API: inspect-get-windows-systemroot to get systemroot.
We are already using heuristics in the C inspection code to
determine the Windows %SYSTEMROOT% directory. This change
just exposes this information through the API.
---
generator/generator_actions.ml | 16 ++++++++++++
src/guestfs-internal.h | 1 +
src/inspect.c | 53 ++++++++++++++++++++++-----------------
3 files changed, 47 insertions(+), 23 deletions(-)
ACK.
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
6:37 a.m.
New subject: [PATCH 3/4] tools: Use C API for inspection
--
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
7:52 a.m.
New subject: [PATCH 3/4] tools: Use C API for inspection
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(a)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.
It wouldn't necessarily need to form part of a published API
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
7:54 a.m.
New subject: [PATCH 3/4] tools: Use C API for inspection
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(a)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
8:05 a.m.
New subject: [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(a)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
8:09 a.m.
New subject: [PATCH 3/4] tools: Use C API for inspection
On Thu, Oct 28, 2010 at 02:05:22PM +0100, Matthew Booth wrote:
There are 2 parameters:
inspect_and_mount(readonly, allow_multiroot);
Unfortunately that won't work for virt-inspector where it'd have
to be something along the lines of:
inspect_and_mount ($readonly, $allow_multiroot,
$callback_for_each_mounted_root);
This is messy.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
8:10 a.m.
New subject: [PATCH 3/4] tools: Use C API for inspection
On 28/10/10 14:09, Richard W.M. Jones wrote:
On Thu, Oct 28, 2010 at 02:05:22PM +0100, Matthew Booth wrote:
> There are 2 parameters:
>
> inspect_and_mount(readonly, allow_multiroot);
Unfortunately that won't work for virt-inspector where it'd have
to be something along the lines of:
inspect_and_mount ($readonly, $allow_multiroot,
$callback_for_each_mounted_root);
This is messy.
Fair enough. Everything other than virt-inspector can use a single
high-level function, though.
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
6:37 a.m.
New subject: [PATCH 4/4] inspector: Rewrite virt-inspector
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
8:55 a.m.
New subject: [PATCH 4/4] inspector: Rewrite virt-inspector
I'm not entirely sure removing all the output methods except XML with no
warning is an entirely good idea. This is an interface, after all, and
it's being removed with no warning. Instead, I would recommend updating
all the output methods except XML to display (to STDERR) a warning that
this function is deprecated and will be removed in the next stable release.
That said, I have a couple of code comments below,
Matt
On 28/10/10 12:37, Richard W.M. Jones wrote:
diff --git a/inspector/virt-inspector b/inspector/virt-inspector
index 36fbfa9..3bd9069 100755
--- a/inspector/virt-inspector
+++ b/inspector/virt-inspector
+sub output_applications
{
- foreach my $os (keys %$oses) {
- foreach my $kernel (@{$oses->{$os}->{kernels}}) {
- my $is_xen = $kernel->{version} =~ m/xen/;
- if ($is_xen) {
- print "xen_domU_kernel=yes\n";
- return;
- }
+ local $_;
+ my $root = shift;
+ my $distro = shift;
+
+ # Based on the distro, take a guess at the package format
+ # and package management.
+ my ($package_format, $package_management);
+ if ($distro eq "debian") {
+ $package_format = "dpkg";
+ $package_management = "apt";
+ }
+ elsif ($distro eq "fedora" || $distro =~/redhat/ || $distro =~/rhel/) {
+ $package_format = "rpm";
+ $package_management = "yum";
+ }
This would identify RHEL 3 and RHEL 4 as using yum, which they don't. I
suggest looking for /usr/bin/yum and /usr/bin/up2date instead.
+ # else unknown.
+
+ $xml->dataElement (package_format => $package_format)
+ if defined $package_format;
+ $xml->dataElement (package_management => $package_management)
+ if defined $package_management;
+
+ # Do we know how to get a list of applications?
+ if (defined $package_format) {
+ if ($package_format eq "rpm") {
+ output_applications_rpm ($root);
}
+ # else no we don't.
}
- print "xen_domU_kernel=no\n";
}
-=item xen_pv_drivers=(yes|no)
-
-Answer C<yes> if the guest has Xen paravirtualized drivers installed
-(usually the kernel itself will be fully virtualized, but the PV
-drivers have been installed by the administrator for performance
-reasons).
-
-=cut
-
-sub output_query_xen_pv_drivers
+sub output_applications_rpm
{
- foreach my $os (keys %$oses) {
- foreach my $kernel (@{$oses->{$os}->{kernels}}) {
- foreach my $module (@{$kernel->{modules}}) {
- if ($module =~ m/xen-/) {
- print "xen_pv_drivers=yes\n";
- return;
- }
- }
+ local $_;
+ my $root = shift;
+
+ # Previous virt-inspector ran the 'rpm' program from the guest.
+ # This is insecure, and unnecessary because we can get the same
+ # information directly from the RPM database.
+
+ my @applications;
+
+ eval {
+ my ($fh, $filename) = tempfile (UNLINK => 1);
+ my $fddev = "/dev/fd/" . fileno ($fh);
+ $g->download ("/var/lib/rpm/Name", $fddev);
+ close $fh or die "close: $!";
+
+ # Read the database with the Berkeley DB dump tool.
+ my $cmd = "db_dump -p '$filename'";
+ open PIPE, "$cmd |" or die "close: $!";
The code should gracefully handle the case that db_dump returns an
error, because the DB is corrupt.
+ while (<PIPE>) {
+ chomp;
+ last if /^HEADER=END$/;
}
- }
- print "xen_pv_drivers=no\n";
-}
-
-=item virtio_drivers=(yes|no)
+ while (<PIPE>) {
+ chomp;
+ last if /^DATA=END$/;
-Answer C<yes> if the guest has virtio paravirtualized drivers
-installed. Virtio drivers are commonly used to improve the
-performance of KVM.
-
-=cut
-
-sub output_query_virtio_drivers
-{
- foreach my $os (keys %$oses) {
- foreach my $kernel (@{$oses->{$os}->{kernels}}) {
- foreach my $module (@{$kernel->{modules}}) {
- if ($module =~ m/virtio_/) {
- print "virtio_drivers=yes\n";
- return;
- }
+ # First character on each data line is a space.
+ if (length $_> 0&& substr ($_, 0, 1) eq ' ') {
Weird spacing.
+ $_ = substr ($_, 1);
}
- }
- }
- print "virtio_drivers=no\n";
-}
-
-=item userspace_arch=(x86_64|...)
-
-Print the architecture of userspace.
-
-NB. For multi-boot VMs this can print several lines.
-
-=cut
-
-sub output_query_userspace_arch
-{
- my %arches;
-
- foreach my $os (keys %$oses) {
- $arches{$oses->{$os}->{arch}} = 1 if exists $oses->{$os}->{arch};
- }
-
- foreach (sort keys %arches) {
- print "userspace_arch=$_\n";
- }
-}
-
-=item kernel_arch=(x86_64|...)
-
-Print the architecture of the kernel.
-
-NB. For multi-boot VMs this can print several lines.
-
-=cut
+ # Name should never contain non-printable chars.
+ die "name contains non-printable chars" if /\\/;
+ push @applications, $_;
This will fail to inspect a guest with a corrupt rpm database. The XML
library will take care of escaping this properly, so you should just
pass through whatever is received. The only potential exception could be
a maximum length on the string.
-sub output_query_kernel_arch
-{
- my %arches;
-
- foreach my $os (keys %$oses) {
- foreach my $kernel (@{$oses->{$os}->{kernels}}) {
- $arches{$kernel->{arch}} = 1 if exists $kernel->{arch};
+ $_ =<PIPE>; # discard value
}
- }
-
- foreach (sort keys %arches) {
- print "kernel_arch=$_\n";
+ close PIPE or die "close: $!";
+ };
+ if (!$@&& @applications> 0) {
Weird spacing.
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
9:31 a.m.
New subject: [PATCH 4/4] inspector: Rewrite virt-inspector
On Thu, Oct 28, 2010 at 02:55:16PM +0100, Matthew Booth wrote:
I'm not entirely sure removing all the output methods except XML
with no warning is an entirely good idea. This is an interface,
after all, and it's being removed with no warning. Instead, I would
recommend updating all the output methods except XML to display (to
STDERR) a warning that this function is deprecated and will be
removed in the next stable release.
This is definitely a new program, not particularly related to the old
one. I was thinking about calling it 'virt-inspector2' at one point.
(I should say for readers that we don't guarantee the inputs and
outputs of these programs, only the C API).
This would identify RHEL 3 and RHEL 4 as using yum, which they
don't. I suggest looking for /usr/bin/yum and /usr/bin/up2date
instead.
Good point, will fix.
>+ eval {
>+ my ($fh, $filename) = tempfile (UNLINK => 1);
>+ my $fddev = "/dev/fd/" . fileno ($fh);
>+ $g->download ("/var/lib/rpm/Name", $fddev);
>+ close $fh or die "close: $!";
>+
>+ # Read the database with the Berkeley DB dump tool.
>+ my $cmd = "db_dump -p '$filename'";
>+ open PIPE, "$cmd |" or die "close: $!";
The code should gracefully handle the case that db_dump returns an
error, because the DB is corrupt.
Well it does sort of ... If the DB is corrupt or cannot be downloaded
then the eval { } around the whole thing will mean that no
<applications> section is produced.
>+ # First character on each data line is a space.
>+ if (length $_> 0&& substr ($_, 0, 1) eq ' ') {
Weird spacing.
Indeed, but not in my original :-)
>+ # Name should never contain non-printable chars.
>+ die "name contains non-printable chars" if /\\/;
>+ push @applications, $_;
This will fail to inspect a guest with a corrupt rpm database. The
XML library will take care of escaping this properly, so you should
just pass through whatever is received. The only potential exception
could be a maximum length on the string.
No, because db_dump does its own escaping.
RPM names with non-printable characters should die here. The die is
caught by the surrounding eval { } and no <applications> section is
produced, but apart from that virt-inspector continues running.
>+ if (!$@&& @applications> 0) {
Weird spacing.
Yeah, don't know what is going on, because that is not in the original
patch I posted either.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
4953
days inactive
4953
days old
18 comments
2 participants
participants (2)
-
Matthew Booth -
Richard W.M. Jones