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

Re: use of "stat -"



On Tue, May 12, 2020 at 10:59 AM Jan Beulich <jbeulich@xxxxxxxx> 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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.