Re: [Libguestfs] [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/