[Libguestfs] [PATCH v4] RFC: New tool: virt-dib

[Richard W.M. Jones]

On Tue, Jun 16, 2015 at 12:15:22PM +0200, Pino Toscano wrote:
[...]

There is some trailing whitespace on one line.  'git show' should
highlight it.

> diff --git a/appliance/packagelist.in b/appliance/packagelist.in
> index 76c7293..a4f814b 100644
> --- a/appliance/packagelist.in
> +++ b/appliance/packagelist.in
> @@ -255,5 +255,12 @@ zerofree
>  
>  ifelse(VALGRIND_DAEMON,1,valgrind)
>  
> +dnl tools needed by virt-dib
> +curl
> +qemu-img
> +debootstrap
> +apt
> +which

What's the purpose of each of these new tools added to the appliance?
Is qemu-img really run inside the appliance?

In particular, is apt (which probably requires perl) only used on
Debian (host), or would it be pulled in on all builds?

I think you can consider moving these to the respective
distro-specific sections of appliance/packagelist.in, and you don't
need the comment.  However when packaging it for Fedora we'll need to
split out the dependencies again.

> +let prepare_external ~dib_args ~dib_vars ~out_name ~root_label ~rootfs_uuid
> +  ~image_cache ~arch ~network ~debug
> +  destdir libdir hooksdir tmpdir fakebindir all_elements element_paths =
> +  let network_string = if network then "" else "1" in
> +
> +  let run_extra = sprintf "\
> +#!/bin/bash

There are a bunch of embedded bash scripts.  Probably they need to use
'set -e'.  Quoting seems OK, but if there are any quotes wrong, then
it might lead to an appliance exploit, which worried me.  Can we do it
in any other way apart from using shell scripts (this is supposed to
be a secure alternative to diskimage-builder after all ..)

I thought the rest of the implementation was surprisingly simple,
which is good.

> +=item B<--mkfs-options> C<OPTION STRING>
> +
> +Add the specified options to L<mkfs(1)>, to be able to fine-tune
> +the root filesystem creation.  Note that this is not possible
> +to override the filesystem type.
> +
> +You should use I<--mkfs-options> at most once.  To pass multiple
> +options, separate them with space, eg:
> +
> + virt-dib ... --mkfs-options '-O someopt -I foo'

Not sure I want to know what horrors allow this to work ...

And quoting -- is it safe?  What if I want to put a space into a mkfs
parameter?

> +=item B<--qemu-img-options> option[,option,...]
> +
> +Pass I<--qemu-img-options> option(s) to the L<qemu-img(1)> command
> +to fine-tune the output format.  Options available depend on
> +the output format (see I<--formats>) and the installed version
> +of the qemu-img program.
> +
> +You should use I<--qemu-img-options> at most once.  To pass multiple
> +options, separate them with commas, eg:
> +
> + virt-dib ... -qemu--img-options cluster_size=512,preallocation=metadata ...

Ditto.

> +Do not compress resulting qcow2 images.  The default is
> +to compressed them.

^ compress

> +=head1 COMPARISON WITH DISKIMAGE-BUILDER
> +
> +Virt-dib is intended as safe replacement for C<diskimage-builder>
> +and its C<ramdisk-image-create> mode; the user-notable differences
> +consist in:
> +
> +=over 4
> +
> +=item
> +
> +the command line arguments; some of the arguments are the same as
> +available in C<diskimage-builder>, while some have different names:
> +
> + disk-image-create             virt-dib
> + -----------------             --------
> + -a ARCH                       --arch ARCH
> + --image-size SIZE             --size SIZE
> + --max-online-resize SIZE      doable using --mkfs-options
> + -n                            --skip-base
> + -o IMAGENAME                  --name IMAGENAME
> + -p PACKAGE(S)                 --extra-packages PACKAGE(S)
> + -t FORMAT(S)                  --formats FORMAT(S)
> + -x                            --debug N

Good way to get people started with virt-dib.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/