Bug 10623

Summary: removing hardlink removes xattrs
Product: [Yocto Project Subprojects] Pseudo Reporter: Patrick Ohly <patrick.ohly>
Component: pseudoAssignee: Joshua Lock <joshuagloe>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: Medium+ CC: benjamin.esquivel, joshuagloe, seebs, yp.pseudo.watcher, yp.watcher
Version: unspecified   
Target Milestone: 2.3 M3   
Hardware: x86   
OS: Multiple   
Whiteboard:
OS type for building Yocto: --- Type of Regression: ---
Verified: Documentation change: No (bug/feature does not impact docs)

Description Patrick Ohly 2016-11-08 15:36:24 UTC 
Bug #9317 fixed the missing sharing of xattrs when creating hardlinks: if foo has an xattr and bar is hard-linked to it, bar must share xattrs with foo.

However, now something else is broken: when deleting bar in the example above, the xattrs are also deleted despite still being used by foo.

To reproduce using roughly OE-core (= pseudo 1.8.1) with a local patch for the xattr corruption bug:

$ rm -rf foo bar /tmp/pseudo; env PSEUDO_PREFIX=tmp-glibc/sysroots/x86_64-linux/usr PSEUDO_PASSWD=tmp-glibc/work/intel_corei7_64-ostro-linux/ostro-image-swupd/1.0-r0/rootfs:tmp-glibc/sysroots/x86_64-linux PSEUDO_NOSYMLINKEXP=1 PSEUDO_DISABLED=0 PSEUDO_LOCALSTATEDIR=/tmp/pseudo tmp-glibc/sysroots/x86_64-linux/usr/bin/pseudo sh -c 'set -x; touch foo; setfattr -n security.SMACK64 -v System::Shared foo; ln foo bar; getfattr -d -m . foo bar; rm bar; getfattr -d -m . foo'

+ touch foo
+ setfattr -n security.SMACK64 -v System::Shared foo
+ ln foo bar
+ getfattr -d -m . foo bar
# file: foo
security.SMACK64="System::Shared"

# file: bar
security.SMACK64="System::Shared"

+ rm bar
+ getfattr -d -m . foo
# no output from getfattr here

This shows that bug #9317 is fixed (bar has same xattr as foo). But after "rm bar", "getfattr foo" should still show the same xattr as before and doesn't.

This blocks further work on improving meta-swupd because the main directory tree gets corrupted after swupd_make_pack from zero. Current usage of meta-swupd is not affected because it only uses the tree once, but that's meant to change.
Comment 1 Seebs 2016-11-08 18:02:54 UTC 
So this is a side-effect of the performance fixes from a bit back. When we delete a file, we used to nuke any xattrs with no remaining files. Now we delete only the xattrs that match the deleted file... But delete them even if other files still match. Should be fixable.

This thing here: DELETE FROM xattrs WHERE dev = ? AND ino = ?;

should probably be something like

... AND COUNT (SELECT * FROM files WHERE dev = ? AND ino = ?) == 0;
Comment 2 Patrick Ohly 2016-11-08 19:59:06 UTC 
Will fixing this bring back the performance problem?
Comment 3 Seebs 2016-11-08 20:40:25 UTC 
I don't think so. The performance problem required a check of every dev/ino pair found in the xattrs table against the whole database, this would check only one.

I have this and a couple of other patches to look at. (Someone sent me a fix for Python3 compat. Yay.)
Comment 4 Benjamin Esquivel 2016-11-17 15:45:02 UTC 
if this bug is blocking the swupd progress, can the swupd bug be noted in the blockers of this bug?
Comment 5 Patrick Ohly 2016-11-17 15:57:34 UTC 
This is not blocking meta-swupd progress because I am using a workaround (don't remove hardlinked files). Would be nicer to not have to carry that workaround, though.
Comment 6 Seebs 2017-02-01 18:37:56 UTC 
I've made a corresponding patch in upstream pseudo that appears to address this. It's a trivial couple-of-line patch. Although past-seebs said "COUNT (SELECT...)" which is of course wrong, it's "SELECT COUNT(*)".

Anyway, checked in upstream, commit 02168305b0a19f981ffe857f36eb256ba8810b77.
Comment 7 Joshua Lock 2017-02-07 13:15:36 UTC 
Once there's a tarball for the 1.8.2 release I'll take care of updating the recipe.
Comment 8 Joshua Lock 2017-02-07 15:54:00 UTC 
Richard generated the release tarball today, patch sent:

https://patchwork.openembedded.org/patch/136835/