Re: [Libguestfs] [PATCH 4/5] New tool: virt-tar

19 Oct 2009


      On 19/10/09 12:24, Richard W.M. Jones wrote:
...
+sub set_mode_x
+{
+    die __"virt-tar: extract/upload mode specified twice on the command line"
+	if $mode;
+    $mode = "x";
+}
+
+sub set_mode_u
+{
+    die __"virt-tar: extract/upload mode specified twice on the command line"
+	if $mode;
+    $mode = "u";
+}
I know we've done this in plenty of other places, but die() really isn't 
a very nice user interface. I would change both of these to:

if($mode) {
   print STDERR __"msg";
   exit(1);
}

Even better, create an error reporting function which also prepends 
'virt-tar: ' so this string doesn't show up repeatedly for translation.
...
+Getopt::Long::Configure ("bundling");
+GetOptions ("help|?" =>  \$help,
+            "version" =>  \$version,
+            "connect|c=s" =>  \$uri,
+	    "extract|download|x" =>  \&set_mode_x,
+	    "upload|u" =>  \&set_mode_u,
+	    "gzip|z" =>  \$gzip,
+    ) or pod2usage (2);
Mixed tabs and spaces.
...
+pod2usage (1) if $help;
+if ($version) {
+    my $g = Sys::Guestfs->new ();
+    my %h = $g->version ();
+    print "$h{major}.$h{minor}.$h{release}$h{extra}\n";
+    exit
+}
+
+pod2usage (__"virt-tar: no image, VM names, directory or filename given")
+    if @ARGV<= 2;
+
+# 'pop' reads arguments right to left.
+my $tarball = pop @ARGV;
+my $directory = pop @ARGV;
+
+die __"virt-tar: either -x or -u must be specified on the command line"
+    unless $mode;
+
+my @args = (\@ARGV);
+push @args, address =>  $uri if $uri;
+push @args, rw =>  1 if $mode eq "u";
+
+my $g = open_guest (@args);
+$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 __"no root device found in this operating system image" if @roots == 0;
+die __"multiboot operating systems are not supported by virt-edit" if @roots>  1;
It's virt-tar, not virt-edit ;)
...
+my $root_dev = $roots[0];
+
+my $os = $oses->{$root_dev};
+mount_operating_system ($g, $os, $mode eq "u" ? 0 : 1);
The cut/paste above suggests we could do with a library function for this.

Matt
-- 
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

M:       +44 (0)7977 267231
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490