[PATCH 0/4] Deprecate and remove use of the Perl inspection API
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
-- 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
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@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
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@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
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@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
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@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/
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@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
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
-- 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
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@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
-- 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
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. 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
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. 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
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
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
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
-- 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/
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
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/
participants (2)
-
Matthew Booth -
Richard W.M. Jones