Preparing to merge metadata stuff
Avery Pennarun
I think the set of bup metadata changes are getting a bit heavyweight,
and I would like to start merging them before they get so unwieldy
that they're too scary to review/test.
I think we can divide the functionality into four broad phases:
1) 'bup meta' and tests pass without crashing (and handles errors
correctly) if all python modules are installed;
2) 'bup meta' works even if some python modules (eg. xattrs) are
old/unavailable, and on non-Linux platforms;
3) 'bup save' incorporates metadata;
4) 'bup restore' restores metadata (and ideally, 'bup fuse' presents it too).
Right now rlb is working on some combination of #1 through #3, which I
think is getting too confusing. I would really like it if we could
stabilize things in the above order, so that I can merge into bup
incrementally.
Now, we *almost* had #1 a few months ago; that's what I have sitting
on my 'meta' branch in git. The only big problem was that it didn't
handle error cases very well. I think I would be okay with merging #1
into master as long as all the error handling works well.
I can't really make a bup *release* until #2 is completed. Right now
bup works fine on (at least) Linux and MacOS, without any special
python modules installed, and because of the BetterThanLastTime rule,
I can't make a release with metadata until it's back to at least
passing all tests (with or without 'bup meta') on all supported
platforms. But I think this will get done really fast as soon as we
merge #1 into master. I understand that it's hard for rlb to test
every possible platform, so this is one of the places where we'll
benefit a lot from other people helping to test with different
versions of python, etc.
Now, with just #2, people can roll their own backup scripts that
actually usefully deal with metadata. That's well worth releasing,
even if it's not totally user friendly. People who are in a hurry can
actually use it for real work, and it'll help us discover any weird
edge cases as early as possible.
The next step is #3, which I know rlb has already started on. I
greatly look forward to this, of course, but it won't really benefit
anyone much until we have #4. We can still release after #3 but
before #4, since the lack of restore-ability doesn't hurt anybody.
Thus, it makes sense to not really work on #4 until after #3 is
working. (By the same logic, 'bup restore' didn't get written until
about 9 months after 'bup save' :))
So anyway, the short version is, I'd like to see a maximally
cleaned-up set of patches for #1 ASAP. I haven't looked at rlb's meta
branch lately, but I'm hoping rlb can do something like this:
- branch off a 'pre-meta' branch from some slightly earlier version of
his 'meta' branch, before any patches that modify 'bup save';
- apply any necessary error handling patches as we've discussed;
- merge in the current apenwarr/master and resolve any conflicts;
- tell me from where to pull the results;
- push that to the new apenwarr/master.
- rebase any of rlb's remaining patches (the stuff to make 'bup save'
work) on top of the new master.
That gives us #1. Then we let it simmer for a while, and anybody with
some time can contribute patches to:
- degrade gracefully in case of old/missing python modules
- work with different filesystems (ext3, ext4, etc)
- work with different OSes (MacOS, etc)
- maybe even work with Windows xattrs
Once this is looking sane - degrading gracefully, at least, when
things don't quite work, and maybe just issuing warnings - then we can
make an actual bup release with the 'bup meta' command included.
In the *meantime*, rlb (and maybe others) can be working on #3 and #4.
I don't expect rlb to spend a lot of time on #2, since he can't
easily test against a zillion different systems.
Does that make sense?
I'm hoping that if we do things in the right order, I'll be able to
merge #1 into my master within the next couple of days. I think we're
very, very close, and I very much look forward to it :)
Have fun,
Avery
Rob Browning
> I think we can divide the functionality into four broad phases:
>
> 1) 'bup meta' and tests pass without crashing (and handles errors
> correctly) if all python modules are installed;
>
> 2) 'bup meta' works even if some python modules (eg. xattrs) are
> old/unavailable, and on non-Linux platforms;
>
> 3) 'bup save' incorporates metadata;
>
> 4) 'bup restore' restores metadata (and ideally, 'bup fuse' presents it too).
>
> Right now rlb is working on some combination of #1 through #3
Actually, I've also started looking at #4, but I haven't done much yet.
> Now, we *almost* had #1 a few months ago; that's what I have sitting
> on my 'meta' branch in git. The only big problem was that it didn't
> handle error cases very well. I think I would be okay with merging #1
> into master as long as all the error handling works well.
I made some changes that were intended to fix some of your problems wrt
#1, and last I recall, I think I was waiting to see if you could re-try
make check and let me know what happens.
As far as #1 goes -- we should be good, just grab my latest public
branch. I haven't committed or pushed anything past #1/#2 yet.
It's also worth noting that so far it looks like #3 may not be all that
much code.
One general concern I still have is the "giant metadata" question.
Though I suppose it could be OK for "bup meta" to generate everything
inline, even if that's not what we always do during bup save. And until
we make an actual release, I suppose we still have time for format
changes.
I've actually been thinking about the problem a bit -- I'll post
something separately about it later.
> - branch off a 'pre-meta' branch from some slightly earlier version of
> his 'meta' branch, before any patches that modify 'bup save';
Right -- just check out my current meta branch.
I probably need to make a few more changes, like dropping the
Metadata*Error exceptions, but even so, if you have time, I'd still like
to see what fails for you with the current code.
> Does that make sense?
Does to me.
Thanks
--
Rob Browning
rlb @defaultvalue.org and @debian.org
GPG as of 2002-11-03 14DD 432F AE39 534D B592 F9A0 25C8 D377 8C7E 73A4
Avery Pennarun
> Right -- just check out my current meta branch.
>
> I probably need to make a few more changes, like dropping the
> Metadata*Error exceptions, but even so, if you have time, I'd still like
> to see what fails for you with the current code.
Well, I don't have a huge amount of time to test things (as I suppose
became obvious over the last month or so of falling behind on patches)
so it would be better if you could get as far as possible. If it at
least works on your computer, and you've done everything we talked
about already (ie. the exception handling is the right way) then
hopefully I can just try it out one more time, and maybe fix/report a
couple of minor things, and then we'll be ready to go.
I realize that step #3 is probably actually much easier than all the
stuff you've done so far - which is great. It also pulls the 'bup
meta' stuff much closer to people's day-to-day usage though, so it
will make bup not work at all for people if the metadata code doesn't
work. So I'd still like to do it one bit at a time.
Thanks,
Avery
Rob Browning
> Well, I don't have a huge amount of time to test things (as I suppose
> became obvious over the last month or so of falling behind on patches)
> so it would be better if you could get as far as possible.
For those interested, I just pushed additional meta branch changes, and
"./bup meta -cf /dev/null /" as a normal user doesn't choke here
anymore, though it's quite the complainer.
This work is not so much for Avery yet, but if anyone else would like to
try make check, that might be helpful:
git clone http://git.debian.org/git/users/rlb/bup.git
However, I'm still working on it. One thing that's definitely not
included is testing for the non-stat metadata, i.e. ACLs, atrs, and
xattrs. Adding even trivial tests for those bits may take a while given
the time I have, and it'll probably require some special handling --
root privileges, lvm or losetup, etc. If someone else would like to
work on it, please do.
Oh, and I did a trial merge from master, and that went pretty smoothly.
Rob Browning
> This work is not so much for Avery yet, but if anyone else would like to
> try make check, that might be helpful:
>
> git clone http://git.debian.org/git/users/rlb/bup.git
>
> However, I'm still working on it. One thing that's definitely not
> included is testing for the non-stat metadata, i.e. ACLs, atrs, and
> xattrs. Adding even trivial tests for those bits may take a while given
> the time I have, and it'll probably require some special handling --
> root privileges, lvm or losetup, etc. If someone else would like to
> work on it, please do.
I've pushed some more changes. There are now some (trivial) root-only
metadata tests involving attrs, xattrs, and POSIX.1e ACLs, and some
consequent bug-fixes. The tests will only run if you're root, and
they'll use a loopback device to set up an ext3 filesystem with all the
trimmings. At the moment, they'll probably only work correctly on
Linux.
See t/test-meta.sh for the details, and I'd recommend taking a look at
that code before you run it. It works here, but I'm the only one who's
tested it, and you will be running it as root...
We certainly want additional (less trivial) tests for these bits, so if
anyone has time to work on them, I'd definitely appreciate help.
Oh, and I just realized that I forgot to add a Signed-off-by header to
my more recent commits. I'll leave it alone for now, in case anyone's
actually tracking my branch, but I suppose I'll need to fix that
eventually.
Rob Browning
> I think we can divide the functionality into four broad phases:
>
> 1) 'bup meta' and tests pass without crashing (and handles errors
> correctly) if all python modules are installed;
> So anyway, the short version is, I'd like to see a maximally
> cleaned-up set of patches for #1 ASAP.
BTW, in case it wasn't clear from my earlier message, I think we're at
step 1 now.
As I mentioned, I've merged master, and I think that according to your
plan, we're at the point where you should take a look at the meta branch
and let me know what other changes you might want.
Avery Pennarun
> Avery Pennarun <apen...@gmail.com> writes:
>> 1) 'bup meta' and tests pass without crashing (and handles errors
>> correctly) if all python modules are installed;
>>
>> So anyway, the short version is, I'd like to see a maximally
>> cleaned-up set of patches for #1 ASAP.
>
> BTW, in case it wasn't clear from my earlier message, I think we're at
> step 1 now.
>
> As I mentioned, I've merged master, and I think that according to your
> plan, we're at the point where you should take a look at the meta branch
> and let me know what other changes you might want.
Okay, I've copied your latest 'meta' branch into my own and merged the
very latest master (which had a few minor conflicts, but went fairly
smoothly). I get the following output from 'make test' on my machine
(Linux 2.6.36, ext3 filesystem, Debian-lenny userspace):
! lib/bup/t/tmetadata.py test_clean_up_archive_path:
................. 0.001s ok
! lib/bup/t/tmetadata.py test_risky_path: ............... 0.002s ok
! lib/bup/t/tmetadata.py test_clean_up_extract_path:
................... 0.002s ok
! lib/bup/t/tmetadata.py test_from_path_error: .. 0.007s ok
! lib/bup/t/tmetadata.py test_apply_to_path_restricted_access: .. 0.006s ok
! lib/bup/t/tmetadata.py test_restore_restricted_user_group: .
! tmetadata.py:126 '/tmp/bup-tmetadata-5f-2F1/foo' ==
'/tmp/bup-tmetadata-5f-2F1/foo' ok
Traceback (most recent call last):
File "wvtest.py", line 123, in _runtest
f()
File "/home/apenwarr/src/bup/lib/bup/t/tmetadata.py", line 127, in
test_restore_restricted_user_group
WVPASSEQ(m.apply_to_path(path), None)
File "../../bup/metadata.py", line 542, in apply_to_path
self._apply_linux_xattr_rec(path, restore_numeric_ids=num_ids)
File "../../bup/metadata.py", line 470, in _apply_linux_xattr_rec
xattr.set(path, k, v, nofollow=True)
IOError: [Errno 1] Operation not permitted
! tmetadata.py:127 [Errno 1] Operation not permitted EXCEPTION
0.005s ok
! lib/bup/t/tmetadata.py test_restore_nonexistent_user_group: .
! tmetadata.py:151 '/tmp/bup-tmetadata-NvFiJn/foo' ==
'/tmp/bup-tmetadata-NvFiJn/foo' ok
Traceback (most recent call last):
File "wvtest.py", line 123, in _runtest
f()
File "/home/apenwarr/src/bup/lib/bup/t/tmetadata.py", line 154, in
test_restore_nonexistent_user_group
WVPASSEQ(m.apply_to_path(path, restore_numeric_ids=True), None)
File "../../bup/metadata.py", line 542, in apply_to_path
self._apply_linux_xattr_rec(path, restore_numeric_ids=num_ids)
File "../../bup/metadata.py", line 470, in _apply_linux_xattr_rec
xattr.set(path, k, v, nofollow=True)
IOError: [Errno 1] Operation not permitted
! tmetadata.py:154 [Errno 1] Operation not permitted EXCEPTION
0.004s ok
! lib/bup/t/tmetadata.py test_restore_over_existing_target:
.......... 0.050s ok
! lib/bup/t/toptions.py test_optdict: ....... 0.000s ok
! lib/bup/t/toptions.py test_options: ........... 0.003s ok
! lib/bup/t/tshquote.py test_shquote: ............... 0.002s ok
! lib/bup/t/tvint.py test_vuint: ....... 0.000s ok
! lib/bup/t/tvint.py test_vint: ........... 0.001s ok
! lib/bup/t/tvint.py test_bvec: .......... 0.001s ok
! lib/bup/t/tvint.py test_pack_and_unpack: .................. 0.002s ok
! lib/bup/t/txstat.py test_fstime:
.................................................. 0.019s ok
! t/test.sh init: . 0.073s ok
! t/test.sh index:
.............................................................. 16.759s
ok
! t/test.sh split: ............................................... 0.002s ok
! t/test.sh bloom: ....... 0.000s ok
! t/test.sh memtest: .. 0.000s ok
! t/test.sh join: ......... 0.000s ok
! t/test.sh save/git-fsck: .. 0.000s ok
! t/test.sh restore: ...... 0.000s ok
! t/test.sh ftp: ............ 0.000s ok
! t/test.sh tag: ...... 0.000s ok
! t/test.sh fsck: .. 0.000s ok
! t/test.sh fsck (par2): ............... 3.546s ok
! t/test.sh exclude-bupdir: ...... 0.619s ok
! t/test.sh exclude: ......... 1.010s ok
! t/test.sh exclude-from: ...... 0.617s ok
! t/test.sh strip: ...... 0.610s ok
! t/test.sh strip-path: ...... 0.612s ok
! t/test.sh graft_points: ........ 0.948s ok
! t/test.sh indexfile: ....... 0.896s ok
! t/test.sh import-rsnapshot: ..... 0.775s ok
! t/test-meta.sh meta - general: ..
! t/test-meta.sh:55 genstat ok
! t/test-meta.sh:56 bup meta --create --recurse --file src.meta src ok
Traceback (most recent call last):
File "/home/apenwarr/src/bup/cmd/bup-meta", line 142, in <module>
create_symlinks=handle_symlinks)
File "../../bup/metadata.py", line 675, in extract
File "../../bup/metadata.py", line 542, in apply_to_path
File "../../bup/metadata.py", line 470, in _apply_linux_xattr_rec
IOError: [Errno 1] Operation not permitted
! t/test-meta.sh:61 bup meta --extract --file ../src.meta FAILED
0.286s ok
WvTest: 2578 tests, 3 failures, total time 32.627s.
So the good news is that your exception reporting is giving useful
information now :) The bad news is something should be degrading more
gracefully when I run 'make test' as a non-root user. If we can get
that sorted out, I think we can merge into master.
Have fun,
Avery
Rob Browning
> Okay, I've copied your latest 'meta' branch into my own and merged the
> very latest master (which had a few minor conflicts, but went fairly
> smoothly). I get the following output from 'make test' on my machine
> (Linux 2.6.36, ext3 filesystem, Debian-lenny userspace):
OK, I don't know why you see that, but I don't, but I'll track it down.
Rob Browning
> Avery Pennarun <apen...@gmail.com> writes:
>
>> Okay, I've copied your latest 'meta' branch into my own and merged the
>> very latest master (which had a few minor conflicts, but went fairly
>> smoothly). I get the following output from 'make test' on my machine
>> (Linux 2.6.36, ext3 filesystem, Debian-lenny userspace):
>
> OK, I don't know why you see that, but I don't, but I'll track it down.
FWIW, I just tested on ext3 here (no special mount options), and
make check of your meta branch still works fine.
I wonder if python-pyxattr's behavior might have changed; I have 0.5.0-3,
and I suspect you have 0.4.0-1.
Avery Pennarun
> Rob Browning <r...@defaultvalue.org> writes:
>> Avery Pennarun <apen...@gmail.com> writes:
>>> Okay, I've copied your latest 'meta' branch into my own and merged the
>>> very latest master (which had a few minor conflicts, but went fairly
>>> smoothly). I get the following output from 'make test' on my machine
>>> (Linux 2.6.36, ext3 filesystem, Debian-lenny userspace):
>>
>> OK, I don't know why you see that, but I don't, but I'll track it down.
>
> FWIW, I just tested on ext3 here (no special mount options), and
> make check of your meta branch still works fine.
>
> I wonder if python-pyxattr's behavior might have changed; I have 0.5.0-3,
> and I suspect you have 0.4.0-1.
Yup, that's the version I have. Does your newer one just never throw
that exception, or does it just throw a different one?
Avery
Rob Browning
> Yup, that's the version I have. Does your newer one just never throw
> that exception, or does it just throw a different one?
I haven't looked at their source, but I don't think the current metadata
code catches any exception for that call, so for whatever reason, it's
not being thrown on my system.
Rob Browning
> Avery Pennarun <apen...@gmail.com> writes:
>
>> Yup, that's the version I have. Does your newer one just never throw
>> that exception, or does it just throw a different one?
>
> I haven't looked at their source, but I don't think the current metadata
> code catches any exception for that call, so for whatever reason, it's
> not being thrown on my system.
I just tried python-pyxattr from the upstream v0.4.0 tag, and still no
exception here. I wonder what else could be different.
Here's what I have for the machine where I just ran the latest tests
(ext4 and ext3):
kernel: 2.6.32-5-686-bigmem
libattr1: 2.4.44-2
python-pyxattr: 0.4.0 and 0.5.0-3.
filesystems:
/dev/dm-0 on / type ext4 (rw,noatime)
/root/tmp/trash-fs.img on /root/tmp/trash-fs type ext3 (rw,loop=/dev/loop0)
Anything unusual on your end?
Thanks
Rob Browning
> Here's what I have for the machine where I just ran the latest tests
> (ext4 and ext3):
>
> kernel: 2.6.32-5-686-bigmem
> libattr1: 2.4.44-2
> python-pyxattr: 0.4.0 and 0.5.0-3.
> filesystems:
> /dev/dm-0 on / type ext4 (rw,noatime)
> /root/tmp/trash-fs.img on /root/tmp/trash-fs type ext3 (rw,loop=/dev/loop0)
>
> Anything unusual on your end?
Also, can anyone else try this? Now that it's in Avery's tree, you can
just:
git checkout -b meta origin/meta
make clean
make check
and optionally:
fakeroot make check
su -
cd wherever-the-tree-was
make check
make clean
Gabriel Filion
> just:
my setup:
ubuntu 10.10
kernel: 2.6.35-25-generic #44-Ubuntu SMP
libattr1: 1:2.4.44-2
attr: 1:2.4.44-2
python-pyxattr: 0.5.0-2
acl: 2.2.49-3
filesystem: ext4
some prerequisite setup:
sudo apt-get install python-pyxattr python-pylibacl
> git checkout -b meta origin/meta
> make clean
> make check
pass.
> and optionally:
>
> fakeroot make check
pass.
the part below required I install some other packages:
sudo apt-get install acl attr
after installing those, I ran the previous commands again to make sure
they still passed. and they did.
> su -
> cd wherever-the-tree-was
> make check
> make clean
pass.
--
Gabriel Filion
Gabriel Filion
> On 11-02-20 03:43 PM, Rob Browning wrote:
>> Rob Browning <r...@defaultvalue.org> writes:
>> Also, can anyone else try this? Now that it's in Avery's tree, you can
>> just:
I also took the time to run it under Debian squeeze and lenny
squeeze had almost the same setup as ubuntu's and passed all checks:
kernel: 2.6.35-2-amd64
libattr1: 1:2.4.44-2
attr: 1:2.4.44-2
python-pyxattr: 0.5.0-3
python-pylibacl: 0.5.0-3
acl: 2.2.49-4
filesystem: ext4
lenny had some issues. here are the app versions:
kernel: 2.6.26-2-amd64
libattr1: 1:2.4.43-2
attr: 1:2.4.43-2
python-pyxattr: 0.4.0-1
python-pylibacl: 0.4.0-2
acl: 2.2.47-2
filesystem: ext3
running checks as a user and with fakeroot gave out an error about diff
-u5 not liking the '-5' argument. but it's also included in the output
for checks as root so I'll only include one trace.
running the checks as root complained about a whole bunch of files not
being found under testfs/, possibly something weird happening with the
loopback-mounted file.
I tried running the mount command found in the t/test-meta.sh script by
hand as root and it worked..
The output from the tests is attached.
--
Gabriel Filion
Rob Browning
> running checks as a user and with fakeroot gave out an error about diff
> -u5 not liking the '-5' argument. but it's also included in the output
> for checks as root so I'll only include one trace.
If you still have that system handy, does it work if you change the
command to "-U 5"? Or rather, if you can see "man diff", does it say
anything about how to ask for 5 lines of unified context? That's what
we want.
> running the checks as root complained about a whole bunch of files not
> being found under testfs/, possibly something weird happening with the
> loopback-mounted file. I tried running the mount command found in the
> t/test-meta.sh script by hand as root and it worked..
I wonder if the lenny tools might not have support for "mount -o loop"
(with no device specified).
I suppose we still don't know why Avery's having trouble -- neither of
us have seen his problem.
Thanks for all the testing.
Gabriel Filion
> Gabriel Filion <lel...@gmail.com> writes:
>> running checks as a user and with fakeroot gave out an error about diff
>> -u5 not liking the '-5' argument. but it's also included in the output
>> for checks as root so I'll only include one trace.
>
> If you still have that system handy, does it work if you change the
> command to "-U 5"? Or rather, if you can see "man diff", does it say
> anything about how to ask for 5 lines of unified context? That's what
> we want.
the man page says "-u -U NUM --unified[=NUM]". Changing both diff
commands to use "-U 5" made the diff errors go away. I also tested on
debian sid and ubuntu lucid and -U 5 seems to work everywhere.
>> running the checks as root complained about a whole bunch of files not
>> being found under testfs/, possibly something weird happening with the
>> loopback-mounted file. I tried running the mount command found in the
>> t/test-meta.sh script by hand as root and it worked..
>
> I wonder if the lenny tools might not have support for "mount -o loop"
> (with no device specified).
I don't know if it can help us find anything, but I tracked down the
place where I get all the errors about "operation not permitted" or "no
such file or directory". It comes from the second call to force-delete
(with the argument "${TOP}/bupmeta.tmp")
--
Gabriel Filion
Rob Browning
> the man page says "-u -U NUM --unified[=NUM]". Changing both diff
> commands to use "-U 5" made the diff errors go away. I also tested on
> debian sid and ubuntu lucid and -U 5 seems to work everywhere.
OK, so just "-U 5", without a "-u", right?
> I don't know if it can help us find anything, but I tracked down the
> place where I get all the errors about "operation not permitted" or "no
> such file or directory". It comes from the second call to force-delete
> (with the argument "${TOP}/bupmeta.tmp")
I wonder if the umount failed. Is there a portable way to ask if a
directory is a mount point? That would let us do something nicer than
the "umount || true".
In any case, was the "operation not permitted error" during a non-root
run? I'm wondering if you could have had a mount left over from a
failed run as root.
Thanks
Rob Browning
> Anything unusual on your end?
Assuming I'm reading things right, an error at that particular point is
a little surprising. The test code has just recorded the metadata for a
path, and is now trying to apply that metadata back to the same path.
So either the fs is mounted with user_xattr, in which case the
read/write should work, or it doesn't, in which case the
_add_linux_xattr() call shouldn't have added an xattr record in the
first place.
Assuming your fs doesn't have user_xattr enabled, could you put a print
statement in _add_linux_xattr() and see if it's returning
errno.EOPNOTSUPP as expected. If not, perhaps that's the problem.
BTW, what version of libattr1 do you have?
Avery Pennarun
> Rob Browning <r...@defaultvalue.org> writes:
>> Anything unusual on your end?
>
> Assuming I'm reading things right, an error at that particular point is
> a little surprising. The test code has just recorded the metadata for a
> path, and is now trying to apply that metadata back to the same path.
>
> So either the fs is mounted with user_xattr, in which case the
> read/write should work, or it doesn't, in which case the
> _add_linux_xattr() call shouldn't have added an xattr record in the
> first place.
>
> Assuming your fs doesn't have user_xattr enabled, could you put a print
> statement in _add_linux_xattr() and see if it's returning
> errno.EOPNOTSUPP as expected. If not, perhaps that's the problem.
>
> BTW, what version of libattr1 do you have?
I'm running as non-root. libattr1 is Debian 1:2.4.43-2. For the
flags, my /proc/mounts says:
/dev/root / ext3 rw,relatime,errors=remount-ro,barrier=0,data=writeback 0 0
I was able to reproduce the error quite easily without running the
tests, like this:
cd src/bup
bup meta -cf test main.py
bup meta -xf test
From strace, the call returning EPERM is:
[pid 31779] lsetxattr("main.py", "security.SMACK64", "_", 2, 0) = -1
EPERM (Operation not permitted)
Don't really know what that is. Wasn't there going to be a way to
dump the metadata file as text to help with debugging?
Have fun,
Avery
Rob Browning
> From strace, the call returning EPERM is:
>
> [pid 31779] lsetxattr("main.py", "security.SMACK64", "_", 2, 0) = -1
> EPERM (Operation not permitted)
OK, so I had wondered if you might have some kind of security-related
difference (i.e. selinux or similar) from our systems, and it looks like
you probably do. While I'm not familiar with the details yet, it looks
like you're probably being affected by Smack:
http://www.mjmwired.net/kernel/Documentation/Smack.txt
So your filess have attributes, just not user attributes. I suspect
that the extraction code just needs to be adjusted to log the EPERM via
add_error().
> Don't really know what that is. Wasn't there going to be a way to
> dump the metadata file as text to help with debugging?
Not at the moment, at least not published. Eventually "meta -tv" should
actually be verbose.
Avery Pennarun
> Avery Pennarun <apen...@gmail.com> writes:
>> From strace, the call returning EPERM is:
>> [pid 31779] lsetxattr("main.py", "security.SMACK64", "_", 2, 0) = -1
>> EPERM (Operation not permitted)
>
> OK, so I had wondered if you might have some kind of security-related
> difference (i.e. selinux or similar) from our systems, and it looks like
> you probably do. While I'm not familiar with the details yet, it looks
> like you're probably being affected by Smack:
>
> http://www.mjmwired.net/kernel/Documentation/Smack.txt
>
> So your filess have attributes, just not user attributes. I suspect
> that the extraction code just needs to be adjusted to log the EPERM via
> add_error().
At least that.
Arguably, if the file already has the desired permissions (it seems
like *all* the files on my system have that particular xattr) then we
shouldn't be trying to set it. That would avoid the EPERM in this
case.
>> Don't really know what that is. Wasn't there going to be a way to
>> dump the metadata file as text to help with debugging?
>
> Not at the moment, at least not published. Eventually "meta -tv" should
> actually be verbose.
Ah. That would certainly be useful eventually :)
Have fun,
Avery
Rob Browning
> At least that.
>
> Arguably, if the file already has the desired permissions (it seems
> like *all* the files on my system have that particular xattr) then we
> shouldn't be trying to set it. That would avoid the EPERM in this
> case.
So are you suggesting that we test all the xattrs before setting them?
i.e.
if not exists(fs.key) or fs.value != archived.value:
set(file, key, archived.value)
That would certainly be friendlier, and I suppose we could only check
when we're not root. Though in the case where most of the existing fs
xattrs already match the archive, always checking could avoid a lot of
writes.
On the other hand, if there are a lot of large archived xattrs that
don't match their (also large) values in the filesystem, the checks
would just be an extra expense. Possibly just a theoretical concern.
Rob Browning
> On the other hand, if there are a lot of large archived xattrs that
> don't match their (also large) values in the filesystem, the checks
> would just be an extra expense. Possibly just a theoretical concern.
Actually, I hadn't thought this through well yet, and thinking it
through makes me realize that "meta -x" may not work correctly at the
moment.
I'll have to double-check, but I think right now it only restores
xattrs, but it doesn't clear any that shouldn't be there. That's not a
problem when you're restoring a tree, but it is a problem when you're
applying metadata to existing paths.
I'll need to fix that.
Rob Browning
> So are you suggesting that we test all the xattrs before setting them?
> i.e.
>
> if not exists(fs.key) or fs.value != archived.value:
> set(file, key, archived.value)
For now, this is what I've done. Of course it's not going to be pretty
if/when fs.value or archived.value is "large".
The code also handles removing any inappropriate existing xattrs, and
it converts EPERM to an ApplyError().
I should be able to push the fix soon, and you can re-test. Hopefully
this will finally resolve your xattr problem.
Rob Browning
> I should be able to push the fix soon, and you can re-test. Hopefully
> this will finally resolve your xattr problem.
OK, I've pushed the fix. Could you test it when you have a chance?
(Gabriel, if you have time, could you check it too?)
Avery Pennarun
> Rob Browning <r...@defaultvalue.org> writes:
>> So are you suggesting that we test all the xattrs before setting them?
>> i.e.
>>
>> if not exists(fs.key) or fs.value != archived.value:
>> set(file, key, archived.value)
>
> For now, this is what I've done. Of course it's not going to be pretty
> if/when fs.value or archived.value is "large".
>
> The code also handles removing any inappropriate existing xattrs, and
> it converts EPERM to an ApplyError().
>
> I should be able to push the fix soon, and you can re-test. Hopefully
> this will finally resolve your xattr problem.
Sounds good. I'm not sure why large xattrs are such a problem -
surely we can abort comparison as soon as they match?
Anyway, your current branch passes on my system - other than that I
now get the "diff -u5" error that Gabriel was seeing, so I changed it
to -U5 and committed. I'll merge it into my master shortly.
Congratulations! Thanks for all the work! Hopefully the remainder
won't be as painful (although I can tell we've got a lot of
compatibility problems on the way... patches to fix any other
platforms will be welcome).
Have fun,
Avery
Rob Browning
> Sounds good. I'm not sure why large xattrs are such a problem -
> surely we can abort comparison as soon as they match?
We have to make sure they match completely in order to know that it's OK
to skip setting them.
Here's the thing -- so far I've seen no limit on the size of the values
for some of these metadta bits (including xattrs). That's what has me
a bit concerned.
> Anyway, your current branch passes on my system - other than that I
> now get the "diff -u5" error that Gabriel was seeing, so I changed it
> to -U5 and committed. I'll merge it into my master shortly.
Thanks. I forgot that.
> Congratulations! Thanks for all the work! Hopefully the remainder
> won't be as painful (although I can tell we've got a lot of
> compatibility problems on the way... patches to fix any other
> platforms will be welcome).
Yes, I'll certainly expect compatibility problems for a while.
At this point, I think I'll switch back to "bup restore". IIRC I nearly
had that working correctly.
Rob Browning
> At this point, I think I'll switch back to "bup restore". IIRC I nearly
> had that working correctly.
That should have been "save/restore", and I believe it was save that I
nearly had working correctly.
Avery Pennarun
> Avery Pennarun <apen...@gmail.com> writes:
>> Sounds good. I'm not sure why large xattrs are such a problem -
>> surely we can abort comparison as soon as they match?
>
> We have to make sure they match completely in order to know that it's OK
> to skip setting them.
>
> Here's the thing -- so far I've seen no limit on the size of the values
> for some of these metadta bits (including xattrs). That's what has me
> a bit concerned.
I haven't looked at the xattr API - is it an open/read/read/read/close
sort of thing? If so, you could use a checksum algorithm, and just
store the checksum or something. Or else just stream from the
metadata file at the same time as the filesystem, throwing away each
block as you compare it.
>> Congratulations! Thanks for all the work! Hopefully the remainder
>> won't be as painful (although I can tell we've got a lot of
>> compatibility problems on the way... patches to fix any other
>> platforms will be welcome).
>
> Yes, I'll certainly expect compatibility problems for a while.
>
> At this point, I think I'll switch back to "bup restore". IIRC I nearly
> had that working correctly.
Hopefully you'll maintain that on a separate branch.
I'm going to be going on vacation again tomorrow (I know! It's a busy
year for me and vacations!) so if you could review and capture any
patches that get sent out for cross-platform fixes, I'll try to pull
from you when I get the chance.
Thanks,
Avery
Rob Browning
> I haven't looked at the xattr API - is it an open/read/read/read/close
> sort of thing?
No, getxattr() and friends simply write to a buffer you specify. If the
value won't fit, you get ERANGE. You can also call pass a buffer length
of 0 to find out how large the value is.
Avery Pennarun
> Avery Pennarun <apen...@gmail.com> writes:
> > I haven't looked at the xattr API - is it an open/read/read/read/close
> > sort of thing?
>
> No, getxattr() and friends simply write to a buffer you specify. If the
> value won't fit, you get ERANGE. You can also call pass a buffer length
> of 0 to find out how large the value is.
Hmm, well, if that's the case, then they must expect the xattrs to be
reasonably small. For example, small enough to load into RAM on a tiny
embedded system. Thus I wouldn't worry too much about loading a *bunch* of
them into RAM on a bigger system like the kind we'd expect to run bup.
If we ever find out otherwise, we can always fix it at that time :)
Have fun,
Avery