[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2] tests: Add URI precedence checking



On 08/22/2013 06:19 AM, Martin Kletzander wrote:
> Commit a0b6a36f is fixing what commit abfff210 broke, so to avoid having
> to deal with this issue again, herec comes "virsh-uriprecedence".
> 
> Signed-off-by: Martin Kletzander <mkletzan redhat com>
> ---

> @@ -235,6 +236,7 @@ test_scripts +=				\
>  	read-bufsiz			\
>  	read-non-seekable		\
>  	start				\
> +	virsh-uriprecedence		\
>  	vcpupin				\

No longer sorted after the rename :)

> +++ b/tests/virsh-uriprecedence
> @@ -0,0 +1,76 @@
> +#!/bin/sh

I'm reviewing this for POSIX sh compatibility...

> +# Create all mock files/directories to avoid permission problems
> +tmphome="$PWD/tmp_home"
> +export XDG_CONFIG_HOME="$tmphome/.config"
> +export XDG_CACHE_HOME="$tmphome/.cache"
> +export XDG_RUNTIME_HOME="XDG_CACHE_HOME"
> +
> +mkdir -p "$XDG_CONFIG_HOME/libvirt" "$XDG_CONFIG_HOME/virsh"
> +mkdir -p "$XDG_CACHE_HOME/libvirt" "$XDG_CACHE_HOME/virsh"
> +mkdir -p "$XDG_RUNTIME_HOME/libvirt" "$XDG_RUNTIME_HOME/virsh"

Isn't this last line redundant, since XDG_RUNTIME_HOME is equal to
XDG_CACHE_HOME?  Also, you could use one 'mkdir -p' to make all the
directories (but shaving processes isn't essential).

> +
> +printf "uri_default=\"%s\"\n" "$good_uri" >"$XDG_CONFIG_HOME/libvirt/libvirt.conf"

Style - slightly shorter as printf 'uri_default="%s"\n' ...
Long line - is it worth wrapping with \-newline?

Everything looks portable - I have no objection to this patch as-is,
although if you haven't pushed yet, you can consider tweaking the nits I
pointed out.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]