use of "stat -"

use of "stat -"

[Jan Beulich]

Hello,

now that I've been able to do a little bit of work from the office
again, I've run into a regression from b72682c602b8 "scripts: Use
stat to check lock claim". On one of my older machines I've noticed
guests wouldn't launch anymore, which I've tracked down to the
system having an old stat on it. Yes, the commit says the needed
behavior has been available since 2009, but please let's not forget
that we continue to support building with tool chains from about
2007.

Putting in place and using newer tool chain versions without
touching the base distro is pretty straightforward. Replacing the
coreutils package isn't, and there's not even an override available
by which one could point at an alternative one. Hence I think
bumping the minimum required versions of basic tools should be
done even more carefully than bumping required tool chain versions
(which we've not dared to do in years). After having things
successfully working again with a full revert, I'm now going to
experiment with adapting behavior to stat's capabilities. Would
something like that be acceptable (if it works out)?

Jan

Re: use of "stat -"

[Wei Liu]

On Tue, May 12, 2020 at 12:58:46PM +0200, Jan Beulich wrote:
> Hello,
> 
> now that I've been able to do a little bit of work from the office
> again, I've run into a regression from b72682c602b8 "scripts: Use
> stat to check lock claim". On one of my older machines I've noticed
> guests wouldn't launch anymore, which I've tracked down to the
> system having an old stat on it. Yes, the commit says the needed
> behavior has been available since 2009, but please let's not forget
> that we continue to support building with tool chains from about
> 2007.
> 
> Putting in place and using newer tool chain versions without
> touching the base distro is pretty straightforward. Replacing the
> coreutils package isn't, and there's not even an override available
> by which one could point at an alternative one. Hence I think
> bumping the minimum required versions of basic tools should be
> done even more carefully than bumping required tool chain versions
> (which we've not dared to do in years). After having things
> successfully working again with a full revert, I'm now going to
> experiment with adapting behavior to stat's capabilities. Would
> something like that be acceptable (if it works out)?

Are you asking for reverting that patch?

Wei.

> 
> Jan

Re: use of "stat -"

[Jan Beulich]

On 12.05.2020 16:19, Wei Liu wrote:
> On Tue, May 12, 2020 at 12:58:46PM +0200, Jan Beulich wrote:
>> now that I've been able to do a little bit of work from the office
>> again, I've run into a regression from b72682c602b8 "scripts: Use
>> stat to check lock claim". On one of my older machines I've noticed
>> guests wouldn't launch anymore, which I've tracked down to the
>> system having an old stat on it. Yes, the commit says the needed
>> behavior has been available since 2009, but please let's not forget
>> that we continue to support building with tool chains from about
>> 2007.
>>
>> Putting in place and using newer tool chain versions without
>> touching the base distro is pretty straightforward. Replacing the
>> coreutils package isn't, and there's not even an override available
>> by which one could point at an alternative one. Hence I think
>> bumping the minimum required versions of basic tools should be
>> done even more carefully than bumping required tool chain versions
>> (which we've not dared to do in years). After having things
>> successfully working again with a full revert, I'm now going to
>> experiment with adapting behavior to stat's capabilities. Would
>> something like that be acceptable (if it works out)?
> 
> Are you asking for reverting that patch?

Well, I assume the patch has its merits, even if I don't really
understand what they are from its description. I'm instead asking
whether something like the below (meanwhile tested) would be
acceptable.

Jan

--- unstable.orig/tools/hotplug/Linux/locking.sh
+++ unstable/tools/hotplug/Linux/locking.sh
@@ -42,6 +42,12 @@ claim_lock()
     # it being possible to safely remove the lockfile when done.
     # See below for a correctness proof.
     local stat
+    local use_stat
+    local rightfile
+    if stat - 2>/dev/null >/dev/null
+    then
+        use_stat=y
+    fi
     while true; do
         eval "exec $_lockfd<>$_lockfile"
         flock -x $_lockfd || return $?
@@ -56,7 +62,17 @@ claim_lock()
         # WW.XXX
         # YY.ZZZ
         # which need to be separated and compared.
-        if stat=$( stat -L -c '%D.%i' - $_lockfile 0<&$_lockfd 2>/dev/null )
+        if [ x$use_stat != xy ]
+        then
+            # Fall back to the original Perl approach.
+            rightfile=$( perl -e '
+                open STDIN, "<&'$_lockfd'" or die $!;
+                my $fd_inum = (stat STDIN)[1]; die $! unless defined $fd_inum;
+                my $file_inum = (stat $ARGV[0])[1];
+                print "y\n" if $fd_inum eq $file_inum;
+                                 ' "$_lockfile" )
+            if [ x$rightfile = xy ]; then break; fi
+        elif stat=$( stat -L -c '%D.%i' - $_lockfile 0<&$_lockfd 2>/dev/null )
         then
             local file_stat
             local fd_stat

Re: use of "stat -"

[Andrew Cooper]

On 12/05/2020 15:33, Jan Beulich wrote:
> On 12.05.2020 16:19, Wei Liu wrote:
>> On Tue, May 12, 2020 at 12:58:46PM +0200, Jan Beulich wrote:
>>> now that I've been able to do a little bit of work from the office
>>> again, I've run into a regression from b72682c602b8 "scripts: Use
>>> stat to check lock claim". On one of my older machines I've noticed
>>> guests wouldn't launch anymore, which I've tracked down to the
>>> system having an old stat on it. Yes, the commit says the needed
>>> behavior has been available since 2009, but please let's not forget
>>> that we continue to support building with tool chains from about
>>> 2007.
>>>
>>> Putting in place and using newer tool chain versions without
>>> touching the base distro is pretty straightforward. Replacing the
>>> coreutils package isn't, and there's not even an override available
>>> by which one could point at an alternative one. Hence I think
>>> bumping the minimum required versions of basic tools should be
>>> done even more carefully than bumping required tool chain versions
>>> (which we've not dared to do in years). After having things
>>> successfully working again with a full revert, I'm now going to
>>> experiment with adapting behavior to stat's capabilities. Would
>>> something like that be acceptable (if it works out)?
>> Are you asking for reverting that patch?
> Well, I assume the patch has its merits, even if I don't really
> understand what they are from its description.

What is in any away unclear about the final paragraph in the commit message?

> I'm instead asking
> whether something like the below (meanwhile tested) would be
> acceptable.

Not really, seeing as removing perl was the whole point.

~Andrew

Re: use of "stat -"

[Jan Beulich]

On 12.05.2020 16:47, Andrew Cooper wrote:
> On 12/05/2020 15:33, Jan Beulich wrote:
>> On 12.05.2020 16:19, Wei Liu wrote:
>>> On Tue, May 12, 2020 at 12:58:46PM +0200, Jan Beulich wrote:
>>>> now that I've been able to do a little bit of work from the office
>>>> again, I've run into a regression from b72682c602b8 "scripts: Use
>>>> stat to check lock claim". On one of my older machines I've noticed
>>>> guests wouldn't launch anymore, which I've tracked down to the
>>>> system having an old stat on it. Yes, the commit says the needed
>>>> behavior has been available since 2009, but please let's not forget
>>>> that we continue to support building with tool chains from about
>>>> 2007.
>>>>
>>>> Putting in place and using newer tool chain versions without
>>>> touching the base distro is pretty straightforward. Replacing the
>>>> coreutils package isn't, and there's not even an override available
>>>> by which one could point at an alternative one. Hence I think
>>>> bumping the minimum required versions of basic tools should be
>>>> done even more carefully than bumping required tool chain versions
>>>> (which we've not dared to do in years). After having things
>>>> successfully working again with a full revert, I'm now going to
>>>> experiment with adapting behavior to stat's capabilities. Would
>>>> something like that be acceptable (if it works out)?
>>> Are you asking for reverting that patch?
>> Well, I assume the patch has its merits, even if I don't really
>> understand what they are from its description.
> 
> What is in any away unclear about the final paragraph in the commit message?

This being the last sentence instead of the first (or even the
subject) makes it look like this is a nice side effect, not
like this was the goal of the change.

>> I'm instead asking
>> whether something like the below (meanwhile tested) would be
>> acceptable.
> 
> Not really, seeing as removing perl was the whole point.

The suggested change doesn't re-introduce a runtime dependency on
Perl, _except_ on very old systems.

Jan

Re: use of "stat -"

[Jason Andryuk]

On Tue, May 12, 2020 at 10:59 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 12.05.2020 16:47, Andrew Cooper wrote:
> > On 12/05/2020 15:33, Jan Beulich wrote:
> >> On 12.05.2020 16:19, Wei Liu wrote:
> >>> On Tue, May 12, 2020 at 12:58:46PM +0200, Jan Beulich wrote:
> >>>> now that I've been able to do a little bit of work from the office
> >>>> again, I've run into a regression from b72682c602b8 "scripts: Use
> >>>> stat to check lock claim". On one of my older machines I've noticed
> >>>> guests wouldn't launch anymore, which I've tracked down to the
> >>>> system having an old stat on it. Yes, the commit says the needed
> >>>> behavior has been available since 2009, but please let's not forget
> >>>> that we continue to support building with tool chains from about
> >>>> 2007.

Sorry for regressing your old system setup.  Out of curiosity, what OS
version are you using?

> >>>> Putting in place and using newer tool chain versions without
> >>>> touching the base distro is pretty straightforward. Replacing the
> >>>> coreutils package isn't, and there's not even an override available
> >>>> by which one could point at an alternative one. Hence I think
> >>>> bumping the minimum required versions of basic tools should be
> >>>> done even more carefully than bumping required tool chain versions
> >>>> (which we've not dared to do in years). After having things
> >>>> successfully working again with a full revert, I'm now going to
> >>>> experiment with adapting behavior to stat's capabilities. Would
> >>>> something like that be acceptable (if it works out)?
> >>> Are you asking for reverting that patch?
> >> Well, I assume the patch has its merits, even if I don't really
> >> understand what they are from its description.
> >
> > What is in any away unclear about the final paragraph in the commit message?
>
> This being the last sentence instead of the first (or even the
> subject) makes it look like this is a nice side effect, not
> like this was the goal of the change.

I see how the motivation wasn't conveyed properly in the commit
message.  It was captured in the cover letter, but that doesn't make
it into the repo. :(

> >> I'm instead asking
> >> whether something like the below (meanwhile tested) would be
> >> acceptable.
> >
> > Not really, seeing as removing perl was the whole point.
>
> The suggested change doesn't re-introduce a runtime dependency on
> Perl, _except_ on very old systems.

Yes, the runtime detection looks okay.  However, Ian may not like
testing `stat -` since he previously disliked the extra overhead of
calling sed.

v1 of the patchset created a dedicated C utility, but Ian preferred
using stat(1).

Qubes uses a different approach to remove perl to bypass stat-ing the
FD.  "Use plain flock on open FD. This makes the locking mechanism not
tolerate removing the lock file once created."
https://github.com/QubesOS/qubes-vmm-xen/blob/xen-4.13/patch-tools-hotplug-drop-perl-usage-in-locking-mechanism.patch
 So they have lockfiles persist even when no process holds the lock.

I was just looking to remove perl since it's a large dependency for
this single use.  I'm not tied to a particular approach.

Regards,
Jason

Re: use of "stat -"

[Elliott Mitchell]

On Tue, May 12, 2020 at 11:52:25AM -0400, Jason Andryuk wrote:
> I was just looking to remove perl since it's a large dependency for
> this single use.  I'm not tied to a particular approach.

Have you tried to remove Perl from a system?  This is possible, but on
many systems there will be hundreds or thousands of other programs which
already cause Perl to be installed.

Perl doesn't have the mindshare it once did, but it is far from dead.  A
new desktop Linux installation might have less than a hundred programs
depending on Perl.  A new developer Linux installation will likely have
hundreds of programs depending on Perl.  A decades old system like Jan
is testing, there will like thousands.

Removing dependancies is good.  Perhaps this is looking a few years into
the future where Perl is even less common.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

Re: use of "stat -"

[Andrew Cooper]

On 12/05/2020 20:50, Elliott Mitchell wrote:
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
>
> On Tue, May 12, 2020 at 11:52:25AM -0400, Jason Andryuk wrote:
>> I was just looking to remove perl since it's a large dependency for
>> this single use.  I'm not tied to a particular approach.
> Have you tried to remove Perl from a system?  This is possible, but on
> many systems there will be hundreds or thousands of other programs which
> already cause Perl to be installed.
>
> Perl doesn't have the mindshare it once did, but it is far from dead.  A
> new desktop Linux installation might have less than a hundred programs
> depending on Perl.  A new developer Linux installation will likely have
> hundreds of programs depending on Perl.  A decades old system like Jan
> is testing, there will like thousands.
>
> Removing dependancies is good.  Perhaps this is looking a few years into
> the future where Perl is even less common.

Not everyone has a fully fat Linux running as dom0.  There are real
systems using Xen which have already successfully purged perl.

~Andrew

Re: use of "stat -"

[Elliott Mitchell]

On Tue, May 12, 2020 at 08:54:26PM +0100, Andrew Cooper wrote:
> On 12/05/2020 20:50, Elliott Mitchell wrote:
> > On Tue, May 12, 2020 at 11:52:25AM -0400, Jason Andryuk wrote:
> >> I was just looking to remove perl since it's a large dependency for
> >> this single use.  I'm not tied to a particular approach.

> > Removing dependancies is good.  Perhaps this is looking a few years into
> > the future where Perl is even less common.
> 
> Not everyone has a fully fat Linux running as dom0.?? There are real
> systems using Xen which have already successfully purged perl.

Gah!  Misread an earlier message.  Upon rereading the message seems my
thinking was wrong.  Yes, it is pretty reasonable to setup a system
without Perl.

Sorry for interrupting with the braindead comment.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

Re: use of "stat -"

[Ian Jackson]

I've read this thread.  Jan, I'm sorry that this causes you
inconvenience.  I'm hoping it won't come down to a choice between
supporting people who want to ship a dom0 without perl, and people who
want a dom0 using more-than-a-decade-old coreutils.

Jan, can you tell me what the output is of this on your ancient
system:

  $ rm -f t
  $ >t
  $ exec 3<t
  $ stat -L -c '%F %i' /dev/stdin <&3
  regular empty file 393549
  $ rm t
  $ stat -L -c '%F %i' /dev/stdin <&3
  regular empty file 393549
  $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
  $

Also, the contents of the file "u" afterwards, please.

Thanks,
Ian.

Re: use of "stat -"

[Jan Beulich]

On 14.05.2020 13:02, Ian Jackson wrote:
> I've read this thread.  Jan, I'm sorry that this causes you
> inconvenience.  I'm hoping it won't come down to a choice between
> supporting people who want to ship a dom0 without perl, and people who
> want a dom0 using more-than-a-decade-old coreutils.

Well, there are options, like producing the actual script from
a locking.sh.in template, with a configure control over whether
the Perl fallback is needed / wanted.

> Jan, can you tell me what the output is of this on your ancient
> system:
> 
>   $ rm -f t
>   $ >t
>   $ exec 3<t
>   $ stat -L -c '%F %i' /dev/stdin <&3
>   regular empty file 393549
>   $ rm t
>   $ stat -L -c '%F %i' /dev/stdin <&3
>   regular empty file 393549
>   $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
>   $
> 
> Also, the contents of the file "u" afterwards, please.

Will do early next week, as the system is in the office (and off).

Jan

Re: use of "stat -"

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

On 14.05.2020 13:02, Ian Jackson wrote:
> I've read this thread.  Jan, I'm sorry that this causes you
> inconvenience.  I'm hoping it won't come down to a choice between
> supporting people who want to ship a dom0 without perl, and people who
> want a dom0 using more-than-a-decade-old coreutils.
> 
> Jan, can you tell me what the output is of this on your ancient
> system:
> 
>   $ rm -f t
>   $ >t
>   $ exec 3<t
>   $ stat -L -c '%F %i' /dev/stdin <&3
>   regular empty file 393549
>   $ rm t
>   $ stat -L -c '%F %i' /dev/stdin <&3
>   regular empty file 393549
>   $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
>   $

$ rm -f t
$ >t
$ exec 3<t
$ stat -L -c '%F %i' /dev/stdin <&3
regular empty file 3380369
$ rm t
$ stat -L -c '%F %i' /dev/stdin <&3
regular empty file 3380369
$ strace -ou stat -L -c '%F %i' /dev/stdin <&3
regular empty file 3380369

> Also, the contents of the file "u" afterwards, please.

Attached.

Thanks for looking into this, Jan

[-- Attachment #2: u --]
[-- Type: text/plain, Size: 13502 bytes --]

execve("/usr/bin/stat", ["stat", "-L", "-c", "%F %i", "/dev/stdin"], [/* 89 vars */]) = 0
brk(0)                                  = 0x8bd000
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fa332c89000
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
open("/usr/local/lib64/tls/x86_64/libselinux.so.1", O_RDONLY) = -1 ENOENT (No such file or directory)
stat("/usr/local/lib64/tls/x86_64", 0x7fffe2b6ad70) = -1 ENOENT (No such file or directory)
open("/usr/local/lib64/tls/libselinux.so.1", O_RDONLY) = -1 ENOENT (No such file or directory)
stat("/usr/local/lib64/tls", 0x7fffe2b6ad70) = -1 ENOENT (No such file or directory)
open("/usr/local/lib64/x86_64/libselinux.so.1", O_RDONLY) = -1 ENOENT (No such file or directory)
stat("/usr/local/lib64/x86_64", 0x7fffe2b6ad70) = -1 ENOENT (No such file or directory)
open("/usr/local/lib64/libselinux.so.1", O_RDONLY) = -1 ENOENT (No such file or directory)
stat("/usr/local/lib64", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
open("/usr/local/lib/tls/x86_64/libselinux.so.1", O_RDONLY) = -1 ENOENT (No such file or directory)
stat("/usr/local/lib/tls/x86_64", 0x7fffe2b6ad70) = -1 ENOENT (No such file or directory)
open("/usr/local/lib/tls/libselinux.so.1", O_RDONLY) = -1 ENOENT (No such file or directory)
stat("/usr/local/lib/tls", 0x7fffe2b6ad70) = -1 ENOENT (No such file or directory)
open("/usr/local/lib/x86_64/libselinux.so.1", O_RDONLY) = -1 ENOENT (No such file or directory)
stat("/usr/local/lib/x86_64", 0x7fffe2b6ad70) = -1 ENOENT (No such file or directory)
open("/usr/local/lib/libselinux.so.1", O_RDONLY) = -1 ENOENT (No such file or directory)
stat("/usr/local/lib", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
open("/opt/intel/compiler-8.1-026/lib/tls/x86_64/libselinux.so.1", O_RDONLY) = -1 ENOENT (No such file or directory)
stat("/opt/intel/compiler-8.1-026/lib/tls/x86_64", 0x7fffe2b6ad70) = -1 ENOENT (No such file or directory)
open("/opt/intel/compiler-8.1-026/lib/tls/libselinux.so.1", O_RDONLY) = -1 ENOENT (No such file or directory)
stat("/opt/intel/compiler-8.1-026/lib/tls", 0x7fffe2b6ad70) = -1 ENOENT (No such file or directory)
open("/opt/intel/compiler-8.1-026/lib/x86_64/libselinux.so.1", O_RDONLY) = -1 ENOENT (No such file or directory)
stat("/opt/intel/compiler-8.1-026/lib/x86_64", 0x7fffe2b6ad70) = -1 ENOENT (No such file or directory)
open("/opt/intel/compiler-8.1-026/lib/libselinux.so.1", O_RDONLY) = -1 ENOENT (No such file or directory)
stat("/opt/intel/compiler-8.1-026/lib", 0x7fffe2b6ad70) = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY)      = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=93598, ...}) = 0
mmap(NULL, 93598, PROT_READ, MAP_PRIVATE, 4, 0) = 0x7fa332c72000
close(4)                                = 0
open("/lib64/libselinux.so.1", O_RDONLY) = 4
read(4, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\0_\0\0\0\0\0\0"..., 832) = 832
fstat(4, {st_mode=S_IFREG|0755, st_size=118080, ...}) = 0
mmap(NULL, 2217768, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 4, 0) = 0x7fa33284e000
fadvise64(4, 0, 2217768, POSIX_FADV_WILLNEED) = 0
mprotect(0x7fa33286a000, 2093056, PROT_NONE) = 0
mmap(0x7fa332a69000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 4, 0x1b000) = 0x7fa332a69000
mmap(0x7fa332a6b000, 1832, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7fa332a6b000
close(4)                                = 0
open("/usr/local/lib64/libc.so.6", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/local/lib/libc.so.6", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/lib64/libc.so.6", O_RDONLY)      = 4
read(4, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0@\355\1\0\0\0\0\0"..., 832) = 832
fstat(4, {st_mode=S_IFREG|0755, st_size=1690993, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fa332c71000
mmap(NULL, 3557448, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 4, 0) = 0x7fa3324e9000
fadvise64(4, 0, 3557448, POSIX_FADV_WILLNEED) = 0
mprotect(0x7fa332645000, 2093056, PROT_NONE) = 0
mmap(0x7fa332844000, 20480, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 4, 0x15b000) = 0x7fa332844000
mmap(0x7fa332849000, 18504, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7fa332849000
close(4)                                = 0
open("/usr/local/lib64/libdl.so.2", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/local/lib/libdl.so.2", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/lib64/libdl.so.2", O_RDONLY)     = 4
read(4, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\360\r\0\0\0\0\0\0"..., 832) = 832
fstat(4, {st_mode=S_IFREG|0755, st_size=19149, ...}) = 0
mmap(NULL, 2109696, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 4, 0) = 0x7fa3322e5000
fadvise64(4, 0, 2109696, POSIX_FADV_WILLNEED) = 0
mprotect(0x7fa3322e7000, 2097152, PROT_NONE) = 0
mmap(0x7fa3324e7000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 4, 0x2000) = 0x7fa3324e7000
close(4)                                = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fa332c70000
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fa332c6e000
arch_prctl(ARCH_SET_FS, 0x7fa332c6e7a0) = 0
mprotect(0x7fa3324e7000, 4096, PROT_READ) = 0
mprotect(0x7fa332844000, 16384, PROT_READ) = 0
mprotect(0x7fa332a69000, 4096, PROT_READ) = 0
mprotect(0x609000, 4096, PROT_READ)     = 0
mprotect(0x7fa332c8a000, 4096, PROT_READ) = 0
munmap(0x7fa332c72000, 93598)           = 0
statfs("/selinux", {f_type="EXT2_SUPER_MAGIC", f_bsize=4096, f_blocks=5126165, f_bfree=4641521, f_bavail=4379461, f_files=1310720, f_ffree=1241977, f_fsid={-304735720, 1406917494}, f_namelen=255, f_frsize=4096}) = 0
brk(0)                                  = 0x8bd000
brk(0x8de000)                           = 0x8de000
open("/proc/filesystems", O_RDONLY)     = 4
fstat(4, {st_mode=S_IFREG|0444, st_size=0, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fa332c88000
read(4, "nodev\tsysfs\nnodev\trootfs\nnodev\tr"..., 1024) = 265
read(4, "", 1024)                       = 0
close(4)                                = 0
munmap(0x7fa332c88000, 4096)            = 0
open("/usr/lib/locale/locale-archive", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/locale.alias", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=2512, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fa332c88000
read(4, "# Locale name alias data base.\n#"..., 4096) = 2512
read(4, "", 4096)                       = 0
close(4)                                = 0
munmap(0x7fa332c88000, 4096)            = 0
open("/usr/lib/locale/en_US.UTF-8/LC_IDENTIFICATION", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/locale/en_US.utf8/LC_IDENTIFICATION", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=373, ...}) = 0
mmap(NULL, 373, PROT_READ, MAP_PRIVATE, 4, 0) = 0x7fa332c88000
close(4)                                = 0
open("/usr/lib64/gconv/gconv-modules.cache", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=26050, ...}) = 0
mmap(NULL, 26050, PROT_READ, MAP_SHARED, 4, 0) = 0x7fa332c81000
close(4)                                = 0
open("/usr/lib/locale/en_US.UTF-8/LC_MEASUREMENT", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/locale/en_US.utf8/LC_MEASUREMENT", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=23, ...}) = 0
mmap(NULL, 23, PROT_READ, MAP_PRIVATE, 4, 0) = 0x7fa332c80000
close(4)                                = 0
open("/usr/lib/locale/en_US.UTF-8/LC_TELEPHONE", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/locale/en_US.utf8/LC_TELEPHONE", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=59, ...}) = 0
mmap(NULL, 59, PROT_READ, MAP_PRIVATE, 4, 0) = 0x7fa332c7f000
close(4)                                = 0
open("/usr/lib/locale/en_US.UTF-8/LC_ADDRESS", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/locale/en_US.utf8/LC_ADDRESS", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=155, ...}) = 0
mmap(NULL, 155, PROT_READ, MAP_PRIVATE, 4, 0) = 0x7fa332c7e000
close(4)                                = 0
open("/usr/lib/locale/en_US.UTF-8/LC_NAME", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/locale/en_US.utf8/LC_NAME", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=77, ...}) = 0
mmap(NULL, 77, PROT_READ, MAP_PRIVATE, 4, 0) = 0x7fa332c7d000
close(4)                                = 0
open("/usr/lib/locale/en_US.UTF-8/LC_PAPER", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/locale/en_US.utf8/LC_PAPER", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=34, ...}) = 0
mmap(NULL, 34, PROT_READ, MAP_PRIVATE, 4, 0) = 0x7fa332c7c000
close(4)                                = 0
open("/usr/lib/locale/en_US.UTF-8/LC_MESSAGES", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/locale/en_US.utf8/LC_MESSAGES", O_RDONLY) = 4
fstat(4, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
close(4)                                = 0
open("/usr/lib/locale/en_US.utf8/LC_MESSAGES/SYS_LC_MESSAGES", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=57, ...}) = 0
mmap(NULL, 57, PROT_READ, MAP_PRIVATE, 4, 0) = 0x7fa332c7b000
close(4)                                = 0
open("/usr/lib/locale/en_US.UTF-8/LC_MONETARY", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/locale/en_US.utf8/LC_MONETARY", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=286, ...}) = 0
mmap(NULL, 286, PROT_READ, MAP_PRIVATE, 4, 0) = 0x7fa332c7a000
close(4)                                = 0
open("/usr/lib/locale/en_US.UTF-8/LC_COLLATE", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/locale/en_US.utf8/LC_COLLATE", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=1163682, ...}) = 0
mmap(NULL, 1163682, PROT_READ, MAP_PRIVATE, 4, 0) = 0x7fa332b51000
close(4)                                = 0
open("/usr/lib/locale/en_US.UTF-8/LC_TIME", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/locale/en_US.utf8/LC_TIME", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=2454, ...}) = 0
mmap(NULL, 2454, PROT_READ, MAP_PRIVATE, 4, 0) = 0x7fa332c79000
close(4)                                = 0
open("/usr/lib/locale/en_US.UTF-8/LC_NUMERIC", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/locale/en_US.utf8/LC_NUMERIC", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=54, ...}) = 0
mmap(NULL, 54, PROT_READ, MAP_PRIVATE, 4, 0) = 0x7fa332c78000
close(4)                                = 0
open("/usr/lib/locale/en_US.UTF-8/LC_CTYPE", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/lib/locale/en_US.utf8/LC_CTYPE", O_RDONLY) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=256324, ...}) = 0
mmap(NULL, 256324, PROT_READ, MAP_PRIVATE, 4, 0) = 0x7fa332b12000
close(4)                                = 0
stat("/dev/stdin", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
open("/usr/share/locale-langpack/en_US.UTF-8/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en_US.UTF-8/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale-bundle/en_US.UTF-8/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale-langpack/en_US.utf8/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en_US.utf8/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale-bundle/en_US.utf8/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale-langpack/en_US/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en_US/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale-bundle/en_US/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale-langpack/en.UTF-8/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en.UTF-8/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale-bundle/en.UTF-8/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale-langpack/en.utf8/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en.utf8/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale-bundle/en.utf8/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale-langpack/en/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale-bundle/en/LC_MESSAGES/coreutils.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
fstat(1, {st_mode=S_IFCHR|0600, st_rdev=makedev(136, 4), ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fa332c77000
write(1, "regular empty file 3380369\n", 27) = 27
close(1)                                = 0
munmap(0x7fa332c77000, 4096)            = 0
close(2)                                = 0
exit_group(0)                           = ?

Ping: use of "stat -"

[Jan Beulich]

On 18.05.2020 12:34, Jan Beulich wrote:
> On 14.05.2020 13:02, Ian Jackson wrote:
>> I've read this thread.  Jan, I'm sorry that this causes you
>> inconvenience.  I'm hoping it won't come down to a choice between
>> supporting people who want to ship a dom0 without perl, and people who
>> want a dom0 using more-than-a-decade-old coreutils.
>>
>> Jan, can you tell me what the output is of this on your ancient
>> system:
>>
>>   $ rm -f t
>>   $ >t
>>   $ exec 3<t
>>   $ stat -L -c '%F %i' /dev/stdin <&3
>>   regular empty file 393549
>>   $ rm t
>>   $ stat -L -c '%F %i' /dev/stdin <&3
>>   regular empty file 393549
>>   $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
>>   $
> 
> $ rm -f t
> $ >t
> $ exec 3<t
> $ stat -L -c '%F %i' /dev/stdin <&3
> regular empty file 3380369
> $ rm t
> $ stat -L -c '%F %i' /dev/stdin <&3
> regular empty file 3380369
> $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
> regular empty file 3380369
> 
>> Also, the contents of the file "u" afterwards, please.
> 
> Attached.
> 
> Thanks for looking into this, Jan

Is there (still) a plan to get this addressed for 4.14? Apologies if
I missed something having been posted / committed...

Jan

[XEN RFC for-4.14] Re: use of "stat -"

[Ian Jackson]

Jan Beulich writes ("Re: use of "stat -""):
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
> On 14.05.2020 13:02, Ian Jackson wrote:
> > I've read this thread.  Jan, I'm sorry that this causes you
> > inconvenience.  I'm hoping it won't come down to a choice between
> > supporting people who want to ship a dom0 without perl, and people who
> > want a dom0 using more-than-a-decade-old coreutils.
> > 
> > Jan, can you tell me what the output is of this on your ancient
> > system:
> > 
> >   $ rm -f t
> >   $ >t
> >   $ exec 3<t
> >   $ stat -L -c '%F %i' /dev/stdin <&3
> >   regular empty file 393549
> >   $ rm t
> >   $ stat -L -c '%F %i' /dev/stdin <&3
> >   regular empty file 393549
> >   $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
> >   $
> 
> $ rm -f t
> $ >t
> $ exec 3<t
> $ stat -L -c '%F %i' /dev/stdin <&3
> regular empty file 3380369
> $ rm t
> $ stat -L -c '%F %i' /dev/stdin <&3
> regular empty file 3380369
> $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
> regular empty file 3380369
> 
> > Also, the contents of the file "u" afterwards, please.
> 
> Attached.

Thanks.

I think this means that `stat -' can be replaced by `stat /dev/stdin'.

This script is only run on Linux where /dev/stdin has existed
basically forever.  The strace output shows
  stat("/dev/stdin", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
and the transcript shows that your stat(1) behaves as I hope.

Jan, will you send a patch ?  It is best if someone else but me writes
it and tests it because then I am a "clean" reviewer.

Paul, supposing I review such a patch and say it is low risk, and we
commit it by Friday, can it have a release-ack ?

Thanks,
Ian.

Re: [XEN RFC for-4.14] Re: use of "stat -"

[Jason Andryuk]

On Wed, Jun 24, 2020 at 12:19 PM Ian Jackson <ian.jackson@citrix.com> wrote:
>
> Jan Beulich writes ("Re: use of "stat -""):
> > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
> > On 14.05.2020 13:02, Ian Jackson wrote:
> > > I've read this thread.  Jan, I'm sorry that this causes you
> > > inconvenience.  I'm hoping it won't come down to a choice between
> > > supporting people who want to ship a dom0 without perl, and people who
> > > want a dom0 using more-than-a-decade-old coreutils.
> > >
> > > Jan, can you tell me what the output is of this on your ancient
> > > system:
> > >
> > >   $ rm -f t
> > >   $ >t
> > >   $ exec 3<t
> > >   $ stat -L -c '%F %i' /dev/stdin <&3
> > >   regular empty file 393549
> > >   $ rm t
> > >   $ stat -L -c '%F %i' /dev/stdin <&3
> > >   regular empty file 393549
> > >   $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
> > >   $
> >
> > $ rm -f t
> > $ >t
> > $ exec 3<t
> > $ stat -L -c '%F %i' /dev/stdin <&3
> > regular empty file 3380369
> > $ rm t
> > $ stat -L -c '%F %i' /dev/stdin <&3
> > regular empty file 3380369
> > $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
> > regular empty file 3380369
> >
> > > Also, the contents of the file "u" afterwards, please.
> >
> > Attached.
>
> Thanks.
>
> I think this means that `stat -' can be replaced by `stat /dev/stdin'.
>
> This script is only run on Linux where /dev/stdin has existed
> basically forever.  The strace output shows
>   stat("/dev/stdin", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> and the transcript shows that your stat(1) behaves as I hope.
>
> Jan, will you send a patch ?  It is best if someone else but me writes
> it and tests it because then I am a "clean" reviewer.
>
> Paul, supposing I review such a patch and say it is low risk, and we
> commit it by Friday, can it have a release-ack ?

I was going to just write a patch to replace - with /dev/stdin and ask
Jan to test it.  When I opened the script, this comment was staring at
me:
        # We can't just stat /dev/stdin or /proc/self/fd/$_lockfd or
        # use bash's test -ef because those all go through what is
        # actually a synthetic symlink in /proc and we aren't
        # guaranteed that our stat(2) won't lose the race with an
        # rm(1) between reading the synthetic link and traversing the
        # file system to find the inum.

On my system:
$ ls -l /dev/stdin
lrwxrwxrwx 1 root root 15 Jun 24 21:13 /dev/stdin -> /proc/self/fd/0
$ ls -l /proc/self/fd/0 0<lockfile
lrwx------ 1 jason jason 64 Jun 24 21:26 /proc/self/fd/0 -> /home/jason/lockfile

stat /dev/stdin will work around the lack of `stat -` support, but it
doesn't address the race in the comment.  Is the comment valid?  How
would we prove there is no race for /dev/stdin?  And as a refresher,
`stat -` does an fstat(0), so there is no symlink lookup.  Or is there
no race since `stat /proc/self/fd/0` isn't a symlink lookup but just
accessing the already open fd via the proc special file? i.e.
equivalent to fstat.

I've mentioned it before, but maybe we should just use the Qubes
patch.  It leaves the lockfile even when no-one is holding the lock,
but it simplifies the code and sidesteps the issues we are discussing
here.
https://github.com/QubesOS/qubes-vmm-xen/blob/xen-4.13/patch-tools-hotplug-drop-perl-usage-in-locking-mechanism.patch

Regards,
Jason

RE: [XEN RFC for-4.14] Re: use of "stat -"

[Paul Durrant]

> -----Original Message-----
> From: Ian Jackson <ian.jackson@citrix.com>
> Sent: 24 June 2020 17:20
> To: Jan Beulich <jbeulich@suse.com>
> Cc: Elliott Mitchell <ehem+xen@m5p.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>; Jason Andryuk
> <jandryuk@gmail.com>; Paul Durrant <paul@xen.org>; Wei Liu <wl@xen.org>; xen-
> devel@lists.xenproject.org
> Subject: [XEN RFC for-4.14] Re: use of "stat -"
> 
> Jan Beulich writes ("Re: use of "stat -""):
> > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified
> the sender and know the content is safe.
> > On 14.05.2020 13:02, Ian Jackson wrote:
> > > I've read this thread.  Jan, I'm sorry that this causes you
> > > inconvenience.  I'm hoping it won't come down to a choice between
> > > supporting people who want to ship a dom0 without perl, and people who
> > > want a dom0 using more-than-a-decade-old coreutils.
> > >
> > > Jan, can you tell me what the output is of this on your ancient
> > > system:
> > >
> > >   $ rm -f t
> > >   $ >t
> > >   $ exec 3<t
> > >   $ stat -L -c '%F %i' /dev/stdin <&3
> > >   regular empty file 393549
> > >   $ rm t
> > >   $ stat -L -c '%F %i' /dev/stdin <&3
> > >   regular empty file 393549
> > >   $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
> > >   $
> >
> > $ rm -f t
> > $ >t
> > $ exec 3<t
> > $ stat -L -c '%F %i' /dev/stdin <&3
> > regular empty file 3380369
> > $ rm t
> > $ stat -L -c '%F %i' /dev/stdin <&3
> > regular empty file 3380369
> > $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
> > regular empty file 3380369
> >
> > > Also, the contents of the file "u" afterwards, please.
> >
> > Attached.
> 
> Thanks.
> 
> I think this means that `stat -' can be replaced by `stat /dev/stdin'.
> 
> This script is only run on Linux where /dev/stdin has existed
> basically forever.  The strace output shows
>   stat("/dev/stdin", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> and the transcript shows that your stat(1) behaves as I hope.
> 
> Jan, will you send a patch ?  It is best if someone else but me writes
> it and tests it because then I am a "clean" reviewer.
> 
> Paul, supposing I review such a patch and say it is low risk, and we
> commit it by Friday, can it have a release-ack ?
> 

In principle yes but, given Jason's response, it doesn't sound like we have agreement on what the final patch will look like yet.

  Paul

> Thanks,
> Ian.

Re: [XEN RFC for-4.14] Re: use of "stat -"

[Jan Beulich]

On 25.06.2020 04:37, Jason Andryuk wrote:
> On Wed, Jun 24, 2020 at 12:19 PM Ian Jackson <ian.jackson@citrix.com> wrote:
>>
>> Jan Beulich writes ("Re: use of "stat -""):
>>> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
>>> On 14.05.2020 13:02, Ian Jackson wrote:
>>>> I've read this thread.  Jan, I'm sorry that this causes you
>>>> inconvenience.  I'm hoping it won't come down to a choice between
>>>> supporting people who want to ship a dom0 without perl, and people who
>>>> want a dom0 using more-than-a-decade-old coreutils.
>>>>
>>>> Jan, can you tell me what the output is of this on your ancient
>>>> system:
>>>>
>>>>   $ rm -f t
>>>>   $ >t
>>>>   $ exec 3<t
>>>>   $ stat -L -c '%F %i' /dev/stdin <&3
>>>>   regular empty file 393549
>>>>   $ rm t
>>>>   $ stat -L -c '%F %i' /dev/stdin <&3
>>>>   regular empty file 393549
>>>>   $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
>>>>   $
>>>
>>> $ rm -f t
>>> $ >t
>>> $ exec 3<t
>>> $ stat -L -c '%F %i' /dev/stdin <&3
>>> regular empty file 3380369
>>> $ rm t
>>> $ stat -L -c '%F %i' /dev/stdin <&3
>>> regular empty file 3380369
>>> $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
>>> regular empty file 3380369
>>>
>>>> Also, the contents of the file "u" afterwards, please.
>>>
>>> Attached.
>>
>> Thanks.
>>
>> I think this means that `stat -' can be replaced by `stat /dev/stdin'.
>>
>> This script is only run on Linux where /dev/stdin has existed
>> basically forever.  The strace output shows
>>   stat("/dev/stdin", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
>> and the transcript shows that your stat(1) behaves as I hope.
>>
>> Jan, will you send a patch ?  It is best if someone else but me writes
>> it and tests it because then I am a "clean" reviewer.

I was about to, when I saw this reply from Jason.

>> Paul, supposing I review such a patch and say it is low risk, and we
>> commit it by Friday, can it have a release-ack ?
> 
> I was going to just write a patch to replace - with /dev/stdin and ask
> Jan to test it.  When I opened the script, this comment was staring at
> me:
>         # We can't just stat /dev/stdin or /proc/self/fd/$_lockfd or
>         # use bash's test -ef because those all go through what is
>         # actually a synthetic symlink in /proc and we aren't
>         # guaranteed that our stat(2) won't lose the race with an
>         # rm(1) between reading the synthetic link and traversing the
>         # file system to find the inum.
> 
> On my system:
> $ ls -l /dev/stdin
> lrwxrwxrwx 1 root root 15 Jun 24 21:13 /dev/stdin -> /proc/self/fd/0
> $ ls -l /proc/self/fd/0 0<lockfile
> lrwx------ 1 jason jason 64 Jun 24 21:26 /proc/self/fd/0 -> /home/jason/lockfile
> 
> stat /dev/stdin will work around the lack of `stat -` support, but it
> doesn't address the race in the comment.  Is the comment valid?  How
> would we prove there is no race for /dev/stdin?  And as a refresher,
> `stat -` does an fstat(0), so there is no symlink lookup.  Or is there
> no race since `stat /proc/self/fd/0` isn't a symlink lookup but just
> accessing the already open fd via the proc special file? i.e.
> equivalent to fstat.

Looking at vfs_statx() in the kernel, I can't see any provisions to
get at the data without traversing the specified path.

Jan

Re: [XEN RFC for-4.14] Re: use of "stat -"

[Rich Persaud]

On Jun 24, 2020, at 22:39, Jason Andryuk <jandryuk@gmail.com> wrote:
> 
> On Wed, Jun 24, 2020 at 12:19 PM Ian Jackson <ian.jackson@citrix.com> wrote:
>> 
>> Jan Beulich writes ("Re: use of "stat -""):
>>> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
>>>> On 14.05.2020 13:02, Ian Jackson wrote:
>>>>> I've read this thread.  Jan, I'm sorry that this causes you
>>>>> inconvenience.  I'm hoping it won't come down to a choice between
>>>>> supporting people who want to ship a dom0 without perl, and people who
>>>>> want a dom0 using more-than-a-decade-old coreutils.
>>>>> 
>>>>> Jan, can you tell me what the output is of this on your ancient
>>>>> system:
>>>>> 
>>>>>  $ rm -f t
>>>>>  $ >t
>>>>>  $ exec 3<t
>>>>>  $ stat -L -c '%F %i' /dev/stdin <&3
>>>>>  regular empty file 393549
>>>>>  $ rm t
>>>>>  $ stat -L -c '%F %i' /dev/stdin <&3
>>>>>  regular empty file 393549
>>>>>  $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
>>>>>  $
>>> 
>>> $ rm -f t
>>> $ >t
>>> $ exec 3<t
>>> $ stat -L -c '%F %i' /dev/stdin <&3
>>> regular empty file 3380369
>>> $ rm t
>>> $ stat -L -c '%F %i' /dev/stdin <&3
>>> regular empty file 3380369
>>> $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
>>> regular empty file 3380369
>>> 
>>>> Also, the contents of the file "u" afterwards, please.
>>> 
>>> Attached.
>> 
>> Thanks.
>> 
>> I think this means that `stat -' can be replaced by `stat /dev/stdin'.
>> 
>> This script is only run on Linux where /dev/stdin has existed
>> basically forever.  The strace output shows
>>  stat("/dev/stdin", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
>> and the transcript shows that your stat(1) behaves as I hope.
>> 
>> Jan, will you send a patch ?  It is best if someone else but me writes
>> it and tests it because then I am a "clean" reviewer.
>> 
>> Paul, supposing I review such a patch and say it is low risk, and we
>> commit it by Friday, can it have a release-ack ?
> 
> I was going to just write a patch to replace - with /dev/stdin and ask
> Jan to test it.  When I opened the script, this comment was staring at
> me:
>        # We can't just stat /dev/stdin or /proc/self/fd/$_lockfd or
>        # use bash's test -ef because those all go through what is
>        # actually a synthetic symlink in /proc and we aren't
>        # guaranteed that our stat(2) won't lose the race with an
>        # rm(1) between reading the synthetic link and traversing the
>        # file system to find the inum.
> 
> On my system:
> $ ls -l /dev/stdin
> lrwxrwxrwx 1 root root 15 Jun 24 21:13 /dev/stdin -> /proc/self/fd/0
> $ ls -l /proc/self/fd/0 0<lockfile
> lrwx------ 1 jason jason 64 Jun 24 21:26 /proc/self/fd/0 -> /home/jason/lockfile
> 
> stat /dev/stdin will work around the lack of `stat -` support, but it
> doesn't address the race in the comment.  Is the comment valid?  How
> would we prove there is no race for /dev/stdin?  And as a refresher,
> `stat -` does an fstat(0), so there is no symlink lookup.  Or is there
> no race since `stat /proc/self/fd/0` isn't a symlink lookup but just
> accessing the already open fd via the proc special file? i.e.
> equivalent to fstat.
> 
> I've mentioned it before, but maybe we should just use the Qubes
> patch.  It leaves the lockfile even when no-one is holding the lock,
> but it simplifies the code and sidesteps the issues we are discussing
> here.
> https://github.com/QubesOS/qubes-vmm-xen/blob/xen-4.13/patch-tools-hotplug-drop-perl-usage-in-locking-mechanism.patch

Is there any practical downside to the Qubes patch, which is already deployed on production systems?  The complexity of other approaches has been reflected in code and prior discussion.  If needed, the comments in the Qubes patch could be expanded, when merging that well-tested code into 4.14.

Rich

Re: [XEN RFC for-4.14] Re: use of "stat -"

[Jason Andryuk]

On Thu, Jun 25, 2020 at 3:05 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 25.06.2020 04:37, Jason Andryuk wrote:
> > On Wed, Jun 24, 2020 at 12:19 PM Ian Jackson <ian.jackson@citrix.com> wrote:
> >>
> >> Jan Beulich writes ("Re: use of "stat -""):
> >>> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
> >>> On 14.05.2020 13:02, Ian Jackson wrote:
> >>>> I've read this thread.  Jan, I'm sorry that this causes you
> >>>> inconvenience.  I'm hoping it won't come down to a choice between
> >>>> supporting people who want to ship a dom0 without perl, and people who
> >>>> want a dom0 using more-than-a-decade-old coreutils.
> >>>>
> >>>> Jan, can you tell me what the output is of this on your ancient
> >>>> system:
> >>>>
> >>>>   $ rm -f t
> >>>>   $ >t
> >>>>   $ exec 3<t
> >>>>   $ stat -L -c '%F %i' /dev/stdin <&3
> >>>>   regular empty file 393549
> >>>>   $ rm t
> >>>>   $ stat -L -c '%F %i' /dev/stdin <&3
> >>>>   regular empty file 393549
> >>>>   $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
> >>>>   $
> >>>
> >>> $ rm -f t
> >>> $ >t
> >>> $ exec 3<t
> >>> $ stat -L -c '%F %i' /dev/stdin <&3
> >>> regular empty file 3380369
> >>> $ rm t
> >>> $ stat -L -c '%F %i' /dev/stdin <&3
> >>> regular empty file 3380369
> >>> $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
> >>> regular empty file 3380369
> >>>
> >>>> Also, the contents of the file "u" afterwards, please.
> >>>
> >>> Attached.
> >>
> >> Thanks.
> >>
> >> I think this means that `stat -' can be replaced by `stat /dev/stdin'.
> >>
> >> This script is only run on Linux where /dev/stdin has existed
> >> basically forever.  The strace output shows
> >>   stat("/dev/stdin", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> >> and the transcript shows that your stat(1) behaves as I hope.
> >>
> >> Jan, will you send a patch ?  It is best if someone else but me writes
> >> it and tests it because then I am a "clean" reviewer.
>
> I was about to, when I saw this reply from Jason.
>
> >> Paul, supposing I review such a patch and say it is low risk, and we
> >> commit it by Friday, can it have a release-ack ?
> >
> > I was going to just write a patch to replace - with /dev/stdin and ask
> > Jan to test it.  When I opened the script, this comment was staring at
> > me:
> >         # We can't just stat /dev/stdin or /proc/self/fd/$_lockfd or
> >         # use bash's test -ef because those all go through what is
> >         # actually a synthetic symlink in /proc and we aren't
> >         # guaranteed that our stat(2) won't lose the race with an
> >         # rm(1) between reading the synthetic link and traversing the
> >         # file system to find the inum.
> >
> > On my system:
> > $ ls -l /dev/stdin
> > lrwxrwxrwx 1 root root 15 Jun 24 21:13 /dev/stdin -> /proc/self/fd/0
> > $ ls -l /proc/self/fd/0 0<lockfile
> > lrwx------ 1 jason jason 64 Jun 24 21:26 /proc/self/fd/0 -> /home/jason/lockfile
> >
> > stat /dev/stdin will work around the lack of `stat -` support, but it
> > doesn't address the race in the comment.  Is the comment valid?  How
> > would we prove there is no race for /dev/stdin?  And as a refresher,
> > `stat -` does an fstat(0), so there is no symlink lookup.  Or is there
> > no race since `stat /proc/self/fd/0` isn't a symlink lookup but just
> > accessing the already open fd via the proc special file? i.e.
> > equivalent to fstat.
>
> Looking at vfs_statx() in the kernel, I can't see any provisions to
> get at the data without traversing the specified path.

Ian, you wrote the comment originally.  Would you please clarify the
scenario where there is a race?  Only the lock holder is allowed to rm
the lockfile, so how is there a race between rm and stat?

Regards,
Jason

Re: [XEN RFC for-4.14] Re: use of "stat -"

[Ian Jackson]

Jason Andryuk writes ("Re: [XEN RFC for-4.14] Re: use of "stat -""):
> I was going to just write a patch to replace - with /dev/stdin and ask
> Jan to test it.  When I opened the script, this comment was staring at
> me:
>         # We can't just stat /dev/stdin or /proc/self/fd/$_lockfd or
>         # use bash's test -ef because those all go through what is
>         # actually a synthetic symlink in /proc and we aren't
>         # guaranteed that our stat(2) won't lose the race with an
>         # rm(1) between reading the synthetic link and traversing the
>         # file system to find the inum.
> 
> On my system:
> $ ls -l /dev/stdin
> lrwxrwxrwx 1 root root 15 Jun 24 21:13 /dev/stdin -> /proc/self/fd/0
> $ ls -l /proc/self/fd/0 0<lockfile
> lrwx------ 1 jason jason 64 Jun 24 21:26 /proc/self/fd/0 -> /home/jason/lockfile
> 
> stat /dev/stdin will work around the lack of `stat -` support, but it
> doesn't address the race in the comment.  Is the comment valid?

Thanks, but:

The tests in my transcript show that the comment (which I wrote) is
false.  It shows that stat /dev/stdin works on deleted files, and
stats the right file even if the name has been rused.

>  How would we prove there is no race for /dev/stdin?

It is easy to create the "bad" situation by hand, without racing.

The transcript shows that the output from readlink(2) is a fiction and
that stat works to stat the actual open-file.

> I've mentioned it before, but maybe we should just use the Qubes
> patch.  It leaves the lockfile even when no-one is holding the lock,
> but it simplifies the code and sidesteps the issues we are discussing
> here.
> https://github.com/QubesOS/qubes-vmm-xen/blob/xen-4.13/patch-tools-hotplug-drop-perl-usage-in-locking-mechanism.patch

I don't like that because this locking code might be reused (or maybe
already is used) in contexts with a varying lockfile filename, leaving
many lockfiles.  And because having lockfiles lying about might
confuse sysadmins who are used to programs which use (the broken)
LOCK_EX-style locking paradigm.

So tl;dr: yes, we need that patch to replace - with /dev/stdin.

Thanks,
Ian.

Re: [XEN RFC for-4.14] Re: use of "stat -"

[Ian Jackson]

Jan Beulich writes ("Re: [XEN RFC for-4.14] Re: use of "stat -""):
> Looking at vfs_statx() in the kernel, I can't see any provisions to
> get at the data without traversing the specified path.

The question is what "traversing the path" means.

How do you explain this ?

$ >t
$ exec 3>t
$ stat -L -c '%F %i' /dev/stdin <&3
regular empty file 421124
$ ll /dev/stdin <&3
lrwxrwxrwx 1 root root 15 Jun  7 02:01 /dev/stdin -> /proc/self/fd/0
$ ll /proc/self/fd/0 <&3
l-wx------ 1 ian ian 64 Jun 25 14:29 /proc/self/fd/0 -> /home/ian/t
$ mv t u
$ ll /proc/self/fd/0 <&3
l-wx------ 1 ian ian 64 Jun 25 14:29 /proc/self/fd/0 -> /home/ian/u
$ rm u
$ ll /proc/self/fd/0 <&3
l-wx------ 1 ian ian 64 Jun 25 14:29 /proc/self/fd/0 -> '/home/ian/u (deleted)'
$ stat -L -c '%F %i' /dev/stdin <&3
regular empty file 421124
$ ll -Li /dev/stdin <&3
421124 -rw-rw-r-- 0 ian ian 0 Jun 25 14:28 /dev/stdin
$

Clearly it isn't actually following this synthetic symlink to
"/home/ian/u (deleted)".

Ian.

Re: [XEN RFC for-4.14] Re: use of "stat -"

[Jan Beulich]

On 25.06.2020 15:31, Ian Jackson wrote:
> Jan Beulich writes ("Re: [XEN RFC for-4.14] Re: use of "stat -""):
>> Looking at vfs_statx() in the kernel, I can't see any provisions to
>> get at the data without traversing the specified path.
> 
> The question is what "traversing the path" means.
> 
> How do you explain this ?
> 
> $ >t
> $ exec 3>t
> $ stat -L -c '%F %i' /dev/stdin <&3
> regular empty file 421124
> $ ll /dev/stdin <&3
> lrwxrwxrwx 1 root root 15 Jun  7 02:01 /dev/stdin -> /proc/self/fd/0
> $ ll /proc/self/fd/0 <&3
> l-wx------ 1 ian ian 64 Jun 25 14:29 /proc/self/fd/0 -> /home/ian/t
> $ mv t u
> $ ll /proc/self/fd/0 <&3
> l-wx------ 1 ian ian 64 Jun 25 14:29 /proc/self/fd/0 -> /home/ian/u
> $ rm u
> $ ll /proc/self/fd/0 <&3
> l-wx------ 1 ian ian 64 Jun 25 14:29 /proc/self/fd/0 -> '/home/ian/u (deleted)'
> $ stat -L -c '%F %i' /dev/stdin <&3
> regular empty file 421124
> $ ll -Li /dev/stdin <&3
> 421124 -rw-rw-r-- 0 ian ian 0 Jun 25 14:28 /dev/stdin
> $
> 
> Clearly it isn't actually following this synthetic symlink to
> "/home/ian/u (deleted)".

Okay, so there's clearly some trickery then which don't know where
to find.

Jan

Re: [XEN RFC for-4.14] Re: use of "stat -"

[Jan Beulich]

On 25.06.2020 15:27, Ian Jackson wrote:
> Jason Andryuk writes ("Re: [XEN RFC for-4.14] Re: use of "stat -""):
>> I was going to just write a patch to replace - with /dev/stdin and ask
>> Jan to test it.  When I opened the script, this comment was staring at
>> me:
>>         # We can't just stat /dev/stdin or /proc/self/fd/$_lockfd or
>>         # use bash's test -ef because those all go through what is
>>         # actually a synthetic symlink in /proc and we aren't
>>         # guaranteed that our stat(2) won't lose the race with an
>>         # rm(1) between reading the synthetic link and traversing the
>>         # file system to find the inum.
>>
>> On my system:
>> $ ls -l /dev/stdin
>> lrwxrwxrwx 1 root root 15 Jun 24 21:13 /dev/stdin -> /proc/self/fd/0
>> $ ls -l /proc/self/fd/0 0<lockfile
>> lrwx------ 1 jason jason 64 Jun 24 21:26 /proc/self/fd/0 -> /home/jason/lockfile
>>
>> stat /dev/stdin will work around the lack of `stat -` support, but it
>> doesn't address the race in the comment.  Is the comment valid?
> 
> Thanks, but:
> 
> The tests in my transcript show that the comment (which I wrote) is
> false.  It shows that stat /dev/stdin works on deleted files, and
> stats the right file even if the name has been rused.
> 
>>  How would we prove there is no race for /dev/stdin?
> 
> It is easy to create the "bad" situation by hand, without racing.
> 
> The transcript shows that the output from readlink(2) is a fiction and
> that stat works to stat the actual open-file.
> 
>> I've mentioned it before, but maybe we should just use the Qubes
>> patch.  It leaves the lockfile even when no-one is holding the lock,
>> but it simplifies the code and sidesteps the issues we are discussing
>> here.
>> https://github.com/QubesOS/qubes-vmm-xen/blob/xen-4.13/patch-tools-hotplug-drop-perl-usage-in-locking-mechanism.patch
> 
> I don't like that because this locking code might be reused (or maybe
> already is used) in contexts with a varying lockfile filename, leaving
> many lockfiles.  And because having lockfiles lying about might
> confuse sysadmins who are used to programs which use (the broken)
> LOCK_EX-style locking paradigm.
> 
> So tl;dr: yes, we need that patch to replace - with /dev/stdin.

I'm about to test this then, but to be honest I have no idea what
to do with the comment. I don't think I could properly justify its
deletion in the description (beyond saying it's not really true),
nor would I be certain whether to e.g. leave the test -ef part
there.

Also is there any reason to go through two symlinks then, rather
than using /proc/self/fd/$_lockfd directly?

Jan

Re: [XEN RFC for-4.14] Re: use of "stat -"

[Jason Andryuk]

On Thu, Jun 25, 2020 at 9:48 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 25.06.2020 15:31, Ian Jackson wrote:
> > Jan Beulich writes ("Re: [XEN RFC for-4.14] Re: use of "stat -""):
> >> Looking at vfs_statx() in the kernel, I can't see any provisions to
> >> get at the data without traversing the specified path.
> >
> > The question is what "traversing the path" means.
> >
> > How do you explain this ?
> >
> > $ >t
> > $ exec 3>t
> > $ stat -L -c '%F %i' /dev/stdin <&3
> > regular empty file 421124
> > $ ll /dev/stdin <&3
> > lrwxrwxrwx 1 root root 15 Jun  7 02:01 /dev/stdin -> /proc/self/fd/0
> > $ ll /proc/self/fd/0 <&3
> > l-wx------ 1 ian ian 64 Jun 25 14:29 /proc/self/fd/0 -> /home/ian/t
> > $ mv t u
> > $ ll /proc/self/fd/0 <&3
> > l-wx------ 1 ian ian 64 Jun 25 14:29 /proc/self/fd/0 -> /home/ian/u
> > $ rm u
> > $ ll /proc/self/fd/0 <&3
> > l-wx------ 1 ian ian 64 Jun 25 14:29 /proc/self/fd/0 -> '/home/ian/u (deleted)'
> > $ stat -L -c '%F %i' /dev/stdin <&3
> > regular empty file 421124
> > $ ll -Li /dev/stdin <&3
> > 421124 -rw-rw-r-- 0 ian ian 0 Jun 25 14:28 /dev/stdin
> > $
> >
> > Clearly it isn't actually following this synthetic symlink to
> > "/home/ian/u (deleted)".
>
> Okay, so there's clearly some trickery then which don't know where
> to find.

I can't say I've taken the time to read and understand all this, but
the code in here
https://elixir.bootlin.com/linux/latest/source/fs/proc/fd.c#L147 seems
to lookup FDs to existing structs.

-Jason

Re: [XEN RFC for-4.14] Re: use of "stat -"

[Jason Andryuk]

On Thu, Jun 25, 2020 at 10:08 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 25.06.2020 15:27, Ian Jackson wrote:
> > Jason Andryuk writes ("Re: [XEN RFC for-4.14] Re: use of "stat -""):
> >> I was going to just write a patch to replace - with /dev/stdin and ask
> >> Jan to test it.  When I opened the script, this comment was staring at
> >> me:
> >>         # We can't just stat /dev/stdin or /proc/self/fd/$_lockfd or
> >>         # use bash's test -ef because those all go through what is
> >>         # actually a synthetic symlink in /proc and we aren't
> >>         # guaranteed that our stat(2) won't lose the race with an
> >>         # rm(1) between reading the synthetic link and traversing the
> >>         # file system to find the inum.
> >>
> >> On my system:
> >> $ ls -l /dev/stdin
> >> lrwxrwxrwx 1 root root 15 Jun 24 21:13 /dev/stdin -> /proc/self/fd/0
> >> $ ls -l /proc/self/fd/0 0<lockfile
> >> lrwx------ 1 jason jason 64 Jun 24 21:26 /proc/self/fd/0 -> /home/jason/lockfile
> >>
> >> stat /dev/stdin will work around the lack of `stat -` support, but it
> >> doesn't address the race in the comment.  Is the comment valid?
> >
> > Thanks, but:
> >
> > The tests in my transcript show that the comment (which I wrote) is
> > false.  It shows that stat /dev/stdin works on deleted files, and
> > stats the right file even if the name has been rused.
> >
> >>  How would we prove there is no race for /dev/stdin?
> >
> > It is easy to create the "bad" situation by hand, without racing.
> >
> > The transcript shows that the output from readlink(2) is a fiction and
> > that stat works to stat the actual open-file.
> >
> >> I've mentioned it before, but maybe we should just use the Qubes
> >> patch.  It leaves the lockfile even when no-one is holding the lock,
> >> but it simplifies the code and sidesteps the issues we are discussing
> >> here.
> >> https://github.com/QubesOS/qubes-vmm-xen/blob/xen-4.13/patch-tools-hotplug-drop-perl-usage-in-locking-mechanism.patch
> >
> > I don't like that because this locking code might be reused (or maybe
> > already is used) in contexts with a varying lockfile filename, leaving
> > many lockfiles.  And because having lockfiles lying about might
> > confuse sysadmins who are used to programs which use (the broken)
> > LOCK_EX-style locking paradigm.
> >
> > So tl;dr: yes, we need that patch to replace - with /dev/stdin.
>
> I'm about to test this then, but to be honest I have no idea what
> to do with the comment. I don't think I could properly justify its
> deletion in the description (beyond saying it's not really true),
> nor would I be certain whether to e.g. leave the test -ef part
> there.

Yes, this is what made me pause yesterday.  Also, I'm not sure how
test -ef would be used to check the file & FD.

> Also is there any reason to go through two symlinks then, rather
> than using /proc/self/fd/$_lockfd directly?

I think /proc/self/fd/$_lockfd should just be used to avoid playing
unnecessary FD games.

Regards,
Jason

Re: [XEN RFC for-4.14] Re: use of "stat -"

[Ian Jackson]

Jan Beulich writes ("Re: [XEN RFC for-4.14] Re: use of "stat -""):
> I'm about to test this then, but to be honest I have no idea what
> to do with the comment. I don't think I could properly justify its
> deletion in the description (beyond saying it's not really true),
> nor would I be certain whether to e.g. leave the test -ef part
> there.

You should delete the comment.  You could replace it with its
opposite.  Something like:

 # Although /dev/stdin (ie /proc/self/fd/0) looks like a symlink,
 # stat(2) bypasses the synthetic symlink and directly accesses the
 # underlying open-file.  So this works correctly even if the file
 # has been renamed or unlinked.

> Also is there any reason to go through two symlinks then, rather
> than using /proc/self/fd/$_lockfd directly?

/dev/stdin is clearer, to my mind.

(The tiny performence difference is not relevant here.  It's also
possible that at some point stat(1) might handle it specially.)

Thanks,
Ian.

Re: [XEN RFC for-4.14] Re: use of "stat -"

[Ian Jackson]

Jason Andryuk writes ("Re: [XEN RFC for-4.14] Re: use of "stat -""):
> On Thu, Jun 25, 2020 at 10:08 AM Jan Beulich <jbeulich@suse.com> wrote:
> > I'm about to test this then, but to be honest I have no idea what
> > to do with the comment. I don't think I could properly justify its
> > deletion in the description (beyond saying it's not really true),
> > nor would I be certain whether to e.g. leave the test -ef part
> > there.
> 
> Yes, this is what made me pause yesterday.  Also, I'm not sure how
> test -ef would be used to check the file & FD.

$ >t
$ exec 3>t
$ rm u
rm: cannot remove 'u': No such file or directory
$ ln t u
$ test t -ef u && echo y
y
$ test /dev/stdin <&3 -ef u && echo y
y
$ mv t v
$ test /dev/stdin <&3 -ef u && echo y
y
$ rm v
$ test /dev/stdin <&3 -ef u && echo y
y
$

This appears to work.  In principle we could switch to this, but (i)
we would want to check that all (recent?) versions of bash do the
sensible thing (ii) we are in a release freeze.

> > Also is there any reason to go through two symlinks then, rather
> > than using /proc/self/fd/$_lockfd directly?
> 
> I think /proc/self/fd/$_lockfd should just be used to avoid playing
> unnecessary FD games.

Xen is frozen at the moment.  Can we please make the minimal change,
rather than the change which cleans the code up the most ?

That does imply some technical debt, but has the lowest risk for the
release.

If you like, combine it with a second patch that changes things to use
test -ef, for post-4.14 ?

Thanks,
Ian.