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

Re: [libvirt] [PATCH 2/2] [TCK] nwfilter: Add test cases for ipset

On 04/23/2012 06:20 AM, Stefan Berger wrote:
> Add test cases for the ipset extension.
> Since ipset may not be available on all system, the first line of the XML
> file containing the test filter has been extended with a specially formatted
> XML comment containing a command line test for whether the test case can be
> run at all. The format of that line is:
> <!-- #<command line test># -->
> If the tests in this line don't succeed, the test case is skipped.

Seems like a slick idea.

> -  ${VIRSH} nwfilter-define "${xmlfile}" > /dev/null
> +  # Check whether we can run this test at all
> +  cmd=`sed -n '1,1 s/^<\!--[ ^I]*#\(.*\)#[ ^I]*-->/\1/p' ${xmlfile}`

Use $(), not `` (since we're already using $(()), we don't have to worry
about Solaris /bin/sh, but might as well stick to the preferred POSIX
shell interface).

1,1 as a sed address selection is redundant; you could shorten it to 1.

In sed, ^I does NOT mean tab, but the two characters ^ and I.  Use a
literal tab instead (and to avoid space-tab warnings, list the bracket
expression as [tab-space], as in '[	 ]*').

> +  if [ -n "${cmd}" ]; then
> +    eval "${cmd}" 2>&1 1>/dev/null

This says output any errors from command to our stdout, and to ignore
normal output of $cmd.  Is that what you meant, or did you want to
ignore both output and errors from $cmd, in which case you should swap
the redirection operators?

Otherwise, it looks okay to me.

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]