[PATCH 0/5] Add new tools virt-tar and virt-ls and tidy up the tools code
This patch series starts by tidying up the code for the virt-cat, virt-df, virt-edit, etc tools, moving them into a single directory and making the build system much simpler as a consequence. Then we add two new tools, virt-tar (a general purpose archive and upload tool) and virt-ls (for listing directories). No new functionality is enabled by these tools -- that is to say, you can do everything that these tools do using guestfish -- but having separate programs and manual pages for them should make them more accessible for simple tasks to some users. Rich. .gitignore | 7 +- HACKING | 18 +-- Makefile.am | 24 ++-- cat/Makefile.am | 45 ------ cat/run-cat-locally | 52 ------- cat/virt-cat | 194 ------------------------ configure.ac | 25 +--- df/Makefile.am | 45 ------ df/run-df-locally | 52 ------- df/virt-df | 366 --------------------------------------------- edit/Makefile.am | 45 ------ edit/run-edit-locally | 52 ------- edit/virt-edit | 210 -------------------------- guestfish.pod | 4 +- po/POTFILES.in | 10 +- rescue/Makefile.am | 45 ------ rescue/run-rescue-locally | 53 ------- rescue/virt-rescue | 169 --------------------- tools/Makefile.am | 50 ++++++ tools/run-locally | 56 +++++++ tools/virt-cat | 194 ++++++++++++++++++++++++ tools/virt-df | 366 +++++++++++++++++++++++++++++++++++++++++++++ tools/virt-edit | 213 ++++++++++++++++++++++++++ tools/virt-ls | 242 ++++++++++++++++++++++++++++++ tools/virt-rescue | 173 +++++++++++++++++++++ tools/virt-tar | 275 ++++++++++++++++++++++++++++++++++ 26 files changed, 1603 insertions(+), 1382 deletions(-) -- 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
-- 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 19/10/09 12:22, Richard W.M. Jones wrote:
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
ACK. -- 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
-- 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
As mentioned on IRC, I'm going to reply quoting git's rename-friendly output rather than the original patch. --- diff --git a/edit/Makefile.am b/tools/Makefile.am similarity index 64% rename from edit/Makefile.am rename to tools/Makefile.am index 6afdd62..d5fc23c 100644 --- a/edit/Makefile.am +++ b/tools/Makefile.am @@ -1,4 +1,4 @@ -# libguestfs virt-edit +# libguestfs virt-* tools # Copyright (C) 2009 Red Hat Inc. # # This program is free software; you can redistribute it and/or modify @@ -15,31 +15,36 @@ # along with this program; if not, write to the Free Software # Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. +tools = cat df edit rescue + EXTRA_DIST = \ - run-edit-locally \ - virt-edit + run-locally \ + $(tools:%=virt-%) + +if HAVE_TOOLS -if HAVE_EDIT +bin_SCRIPTS = $(tools:%=virt-%) --- You use the $(tools:%=virt-%) construction in a few places. It would improve readability if you pulled it up to the top and reused it. Maybe define: virt_tools = $(tools:%=virt-%) right below the tools definition. --- -$(top_builddir)/html/virt-edit.1.html: virt-edit +$(top_builddir)/html/virt-%.1.html: virt-% mkdir -p $(top_builddir)/html cd $(top_builddir) && pod2html \ --css 'pod.css' \ - --title 'virt-edit, edit a file in a virtual machine' \ --htmldir html \ - --outfile html/virt-edit.1.html \ - edit/$< + --outfile html/$<.1.html \ + tools/$< --- You lost the title in the html documentation. Not a huge deal, I guess, but still a regression. --- diff --git a/rescue/run-rescue-locally b/tools/run-locally similarity index 80% rename from rescue/run-rescue-locally rename to tools/run-locally index 18097cf..0bf1c0a 100755 --- a/rescue/run-rescue-locally +++ b/tools/run-locally @@ -50,4 +52,5 @@ $ENV{LD_LIBRARY_PATH} = $path.'/src/.libs'; $ENV{LIBGUESTFS_PATH} = $path.'/appliance'; $ENV{PERL5LIB} = $path.'/perl/blib/lib:'.$path.'/perl/blib/arch'; -exec('perl', $path.'/rescue/virt-rescue', @ARGV); +print (join " ", ("$path/tools/virt-$tool", @ARGV), "\n"); +exec('perl', "$path/tools/virt-$tool", @ARGV); --- The print looks like debug output. Did you intend to leave it in there? 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
On Mon, Oct 19, 2009 at 04:55:51PM +0100, Matthew Booth wrote:
- --title 'virt-edit, edit a file in a virtual machine' \ --htmldir html \ - --outfile html/virt-edit.1.html \ - edit/$< + --outfile html/$<.1.html \ + tools/$< ---
You lost the title in the html documentation. Not a huge deal, I guess, but still a regression.
No actually I checked the HTML here and it's fine - it still includes the all-important title. I used to work in SEO you know :-)
-exec('perl', $path.'/rescue/virt-rescue', @ARGV); +print (join " ", ("$path/tools/virt-$tool", @ARGV), "\n"); +exec('perl', "$path/tools/virt-$tool", @ARGV); ---
The print looks like debug output. Did you intend to leave it in there?
I did actually intend to leave it there, but it might be better to remove it on consideration. 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 19/10/09 18:01, Richard W.M. Jones wrote:
On Mon, Oct 19, 2009 at 04:55:51PM +0100, Matthew Booth wrote:
- --title 'virt-edit, edit a file in a virtual machine' \ --htmldir html \ - --outfile html/virt-edit.1.html \ - edit/$< + --outfile html/$<.1.html \ + tools/$< ---
You lost the title in the html documentation. Not a huge deal, I guess, but still a regression.
No actually I checked the HTML here and it's fine - it still includes the all-important title.
I used to work in SEO you know :-)
Ah, yes. It still gets it from the POD. 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
A timely warning not to try running certain tools on live VMs. Sys::Guestfs::Lib detects this some of the time, but can't do it always. 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 19/10/09 12:24, Richard W.M. Jones wrote:
A timely warning not to try running certain tools on live VMs. Sys::Guestfs::Lib detects this some of the time, but can't do it always.
Rich.
ACK. -- 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
-- 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 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
On Mon, Oct 19, 2009 at 05:32:05PM +0100, Matthew Booth wrote:
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.
Hmmmm I like die. It also shows people where the error happened which I think is an advantage (but without the verbosity of Python stack traces).
+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.
Fixed these in the upcoming patch.
+die __"multiboot operating systems are not supported by virt-edit" if @roots> 1;
It's virt-tar, not virt-edit ;)
And fixed this one. New patch attached. 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 19/10/09 18:14, Richard W.M. Jones wrote:
Hmmmm I like die. It also shows people where the error happened which I think is an advantage (but without the verbosity of Python stack traces).
If the message is completely descriptive on its own (as these are), there's no added benefit *to the user* in die: it's just untidy. You like it, but you're not the user ;) In my mind, die indicates that the program has not operated correctly. 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
On 19/10/09 18:14, Richard W.M. Jones wrote:
+die __"multiboot operating systems are not supported by virt-edit" if @roots> 1;
It's virt-tar, not virt-edit ;)
And fixed this one.
New patch attached.
No library function? That's a non-trivial amount of duplicate code. 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
On Tue, Oct 20, 2009 at 09:26:56AM +0100, Matthew Booth wrote:
On 19/10/09 18:14, Richard W.M. Jones wrote:
+die __"multiboot operating systems are not supported by virt-edit" if @roots> 1;
It's virt-tar, not virt-edit ;)
And fixed this one.
New patch attached.
No library function? That's a non-trivial amount of duplicate code.
Trouble is, where do we put the library function? If we share it somewhere then we add cohesion between these tools which are currently completely separate programs. We could add it to Sys::Guestfs::Lib, but it doesn't fit very well. (And the code isn't exactly duplicated anyway, but different for each tool). I think it's fine as it is for now ... Rich. -- 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
-- 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
This just fixes a tab/space issue in the previous patch. -- 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
participants (2)
-
Matthew Booth -
Richard W.M. Jones