Re: [Xen-devel] [PATCH] scripts: Use stat to check lock claim

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Jason Andryuk writes ("[PATCH] scripts: Use stat to check lock claim"):
> Replace the perl locking check with stat(1).  Stat is able to fstat
> stdin (file descriptor 0) when passed '-' as an argument.  This is now
> used to check $_lockfd.  stat(1) support for '-' was introduced to
> coreutils in 2009.
> 
> After A releases its lock, script B will return from flock and execute
> stat.  Since the lockfile has been removed from A, stat prints an error
> to stderr and exits non-zero.  '|| :' is needed to squash the non-zero
> exit status - otherwise the script terminates since `set -e` is enabled.
> stderr needs to be redirected to /dev/null otherwise
> /var/log/xen/xen-hotplug.log will get filled with "No such file or
> directory" messages.

Thanks.  This is looking good.

I have two nits:

> While here, replace some tabs with spaces to match the rest of the file.

Please can you do this in a separate patch, ideally beforehand.  (I
don't think this is a blocker in this case, given how small this patch
is.)

> +        stat=$( stat -L -c '%D.%i' - $_lockfile 0<&$_lockfd 2>/dev/null || : 
> )
> +        if [ -n "$stat" ]; then
> +            fd_stat=$( echo "$stat" | sed -n '1p' )
> +            file_stat=$( echo "$stat" | sed -n '2p' )
> +            if [ "$fd_stat" = "$file_stat" ] ; then break; fi

I think you don't need sed here:

$ ls -li t u
844307 -rw-rw-r-- 1 iwj iwj 117844 Oct 31 12:50 t
826417 -rw-r--r-- 1 iwj iwj   1765 Jan 31  2019 u
$ bash -c 'x=$( stat -L -c "%D.%i" t u 2>/dev/null || : ); echo ${x%
*} = ${x#*
}'
fe04.844307 = fe04.826417
$

The syntax (with newlines within the ${ }) is a bit odd but not
invoking sed here will be faster.

Alternatively, if you don't mind using --printf instead of -c,

$ bash -c 'x=$( stat -L --format "%D.%i " t u 2>/dev/null || : ); echo ${x%% *} 
= ${x#* }'
fe04.844307 = fe04.826417
$

I don't know when --format was introduced.

I'm sorry to bounce the patch over such a small thing, but this is
path is already quite slow and is critical for domain creation and I
would prefer not to add (two) additional subprocess invocations here.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel