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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list