Wow, I had forgotten about this one :)
On Thu, 2015-08-06 at 14:38 +0200, Martin Kletzander wrote:
> Since this script is not part of the _data_ for nodeinfo test, I'd
> rather put it in the tests/ directory. But I don't have a strict
> preference.
The scripts are not themselves test data, but they're not
test cases either, and they work on test data.
So I'd rather have them inside nodeinfodata/, and since
you don't feel too strongly about that I havent' moved
them :)
> > +matches_any_pattern() {
> > + typeset name=$1
> > + typeset patterns=$2
> > + typeset ret=1
>
> I'm not sure why typeset is better then (or compatible as):
>
> name="$1"
>
> and somewhere the parameters naming almost doubles the function
> length, but this is just a support script and not something we call
> on
> all machines, so I'm OK with keeping these as they are.
Naming the parameters, even though it increases the script's
size, improves its readability.
Using typeset makes parameters and other variables local
to the function, instead of messing with the caller's
environment - which is a terrible default, by the way.
But it's not very portable indeed (works on bash and ksh)
so I've just dropped any pretense that the scripts are
pure POSIX sh by changing the sha-bang line to explicitly
use bash and switched from typeset to local.
> > + # Copy data from host. Errors are ignored because some files
> > we don't
> > + # care about always give input/output error when read
> > + cp -r "$SYSFS_PATH/cpu" "$name/" >/dev/null 2>&1
> > + cp -r "$SYSFS_PATH/node" "$name/" >/dev/null 2>&1
> > +
>
> If there are errors we don't care about, why don't you just copy the
> files that match wither KEEP_LINKS or KEEP_DIRECTORIES? You can
> check
> that all of them copied correctly and you don't need to redirect the
> output, so any possible error gives a clue on what's wrong.
Very good point. I've changed the script to just copy the
data we're interested in instead of copying everything
and cleaning it up afterwards, and would you believe it?
It's not only easier to understand and more robust, but
also quite a bit shorter :)
I'll send a v2 right away.
Cheers.
--
Andrea Bolognani
Software Engineer - Virtualization Team
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list