[Libguestfs] [PATCH 4/4] inspector: Rewrite virt-inspector

[Richard W.M. Jones]

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/