On 03/28/2014 10:31 AM, Cédric Bosdonnat wrote:
--- Makefile.am | 2 +- configure.ac | 1 + examples/lxcconvert/Makefile.am | 18 ++++++ examples/lxcconvert/virt-lxc-convert | 108 +++++++++++++++++++++++++++++++++++ 4 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 examples/lxcconvert/Makefile.am create mode 100644 examples/lxcconvert/virt-lxc-convert
Sorry for the dreadful delay in review...
+++ b/examples/lxcconvert/virt-lxc-convert @@ -0,0 +1,108 @@ +#!/bin/sh + +handler_cleanup()
Missing copyright and license statement.
+{ + if ! test -z "$conf_dir"; then
Works, but I prefer the positive-logic spelling (which also happens to be less typing): if test "$conf_dir"; then (or the slightly-longer test -n "$conf_dir") [side note - there are horror stories of 'test "$var"', 'test -n "$var"', and 'test -z "$var"' misbehaving on non-POSIX hosts; particularly when $var is something like '!', '-', or '(' - but this script is only useful on Linux where we are guaranteed that test obeys POSIX rules.]
+# Do we have lxc.mount, and is it pointing to a readable file? +fstab=$(sed -n '/lxc.mount[[:space:]]*=/ s/[[:space:]]*=[[:space:]]*/=/p' \ + "$conf_new" | cut -f 2 -d '=') +if test -n "$fstab" && test -r "$fstab"; then
The test -n is redundant (test -r "" correctly fails). Can you post a v3 with a copyright blurb and your choice of whether to fix the other nits, and then I can go ahead and push it on your behalf? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org