duplicity-talk
[Top][All Lists]
Advanced

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

Re: [Duplicity-talk] Backup succeeding but not writing manifest / sigtar


From: edgar . soldin
Subject: Re: [Duplicity-talk] Backup succeeding but not writing manifest / sigtar
Date: Tue, 1 Mar 2016 17:13:10 +0100
User-agent: Mozilla/5.0 (Windows NT 5.1; rv:38.0) Gecko/20100101 Thunderbird/38.5.1

On 01.03.2016 16:48, Mark Grandi wrote:
> 
>> On Mar 1, 2016, at 5:04 AM, address@hidden <mailto:address@hidden> wrote:
>>
>> On 01.03.2016 05:50, Mark Grandi wrote:
>>> I am backing up root, because on Mac OS X, almost all of the applications 
>>> are stored in /Applications , (shared between all users), so I am backing 
>>> up root, and excluding all of the other directories besides /Users , but 
>>> that doesn't seem to stop Duplicity from trying to access them anyway, and 
>>> failing due to not having the read permissions.
>>>
>>> Anyway, I looked into my problem myself since I had a free afternoon, and 
>>> found myself honestly quite concerned with what some of the code does, or 
>>> rather doesn't do. For instance, with Par2Backend, if for some reason the 
>>> par2 command doesn't return 0, then it just silently doesn't create any 
>>> par2 files without any notice to the user. In LocalBackend, if any of the 
>>> methods in there that have try/except blocks, in the except blocks, they 
>>> are either 'pass' or just 'return False', and often the caller doesn't care 
>>> about the return result anyway, so any errors that occur during 
>>> moving/copying files are silently discarded and not presented to the user, 
>>> nor does it invoke any of the retry logic in Duplicity.
>>>
>>> I mentioned the two above things, because I had trouble compiling the par2 
>>> binary on my Mac, and upon googling, I found a Mac OS X app that is a GUI 
>>> frontend to par and par2, and it appears that the author created their own 
>>> fork of par2, that works on mac (and has improvements such as using grand 
>>> central dispatch, the website is here: 
>>> https://gp.home.xs4all.nl/Site/MacPAR_deLuxe.html). Anyway, it sees this 
>>> version of par2 does not like symlinks, and the Par2Backend class uses 
>>> symlinks when creating the par2 files. Discovering why the par2 files were 
>>> not being created required me stepping into the debugger, because no errors 
>>> are logged to the user.
>>>
>>> Once I changed the symlink to a hardlink, and modified the command line 
>>> being passed to par2 somewhat, I was stuck on the issue that I originally 
>>> posted:  the difftar files + par2 files were being moved to the correct 
>>> spot, but not the manifest or sigtar files and their par2 counterparts. 
>>> Stepping into the debugger, i found that the difftar files are being 
>>> written with Par2Backend wrapping LocalBackend.put(), but the sigtar and 
>>> manifest files are wrapping LocalBackend._move(). However, 
>>> LocalBackend._move() is using Path.rename() instead of Path.move() ! It 
>>> even says in the docstring for Path.move(): "Like rename but destination 
>>> may be on different file system". 
>>>
>>> So there lies my problem, my backup destination was on a different file 
>>> system, but due to LocalBackend._move() calling the wrong underlying Path 
>>> method, and swallowing exceptions and only returning false (that 
>>> Par2Backend doesn't even check), it was not moving the sigtar and manifest 
>>> files + par2 files because of "[Errno 18] Cross-device link". 
>>>
>>> The patch to fix this is just simply replacing LocalBackend._move() with:
>>>
>>>    def _move(self, source_path, remote_filename):
>>>        target_path = self.remote_pathdir.append(remote_filename)
>>>        try:
>>>            source_path.setdata() # maybe only needed in Par2Backend?
>>>            source_path.move(target_path)
>>>            return True
>>>        except OSError as e:
>>>            return False
>>>
>>> This does not solve the concerns I mentioned with swallowing exceptions 
>>> however. 
>>>
>>
>> hey Mark,
>>
>> well done debugging the issue! i totally agree w/ you that the duplicity 
>> code is not in perfect shape. thanks for investing the time to find and fix 
>> your issue.
>>
>> wrt. to the sym-/hardlink par2 issue. as hardlinks are only avail to root 
>> users, would you mind to have a look if you can come up w/ a solution that 
>> works w/o links at all for the par2 backend? if you provide a patch i will 
>> create a branch that fixes your issues that can be merged into the next 
>> release by Ken.
>>
>> ..ede
>>
>> _______________________________________________
>> Duplicity-talk mailing list
>> address@hidden <mailto:address@hidden>
>> https://lists.nongnu.org/mailman/listinfo/duplicity-talk
> 
> Well, the only reason I changed os.symlink to os.link was because i'm not 
> using the 'official' par2 I guess you could say, but par2SL? Either way, I 
> think it should be a separate backend, or the par2 backend should handle both 
> versions of par2.

w/o looking into the par2 backend. why is it actually necessary to create links 
at all?
 
> Either way, I'm also confused by the statement that hardlinks are only 
> available to root users? This certainly isn't the case for Mac OS X as well 
> as linux (at least the version of Ubuntu that I run on my server). Perhaps 
> you are thinking of windows and NTFS junction points, which require some 
> policy enabled for that user (which is the bane of every VCS software that 
> supports symlinks).

just using hard links to sparsely (pun intended).. with linux you have to be 
root to hardlink folders, but files work fine - you are correct.

> 
> HOWEVER, I do agree that it is probably easiest to just refactor it to not 
> need a symlink or a hardlink by just doing some renaming magic, so i'll work 
> on that. I use bazaar myself so i'll create a branch on launchpad for both 
> this (and the other issue that i posted in a different thread on this mailing 
> list)

just looked a bit further into the move issue and path.py:Path.move(...) seems 
to copy/del all of the time. wouldn't it make sense performancewise to add 
os.rename() here and only copy/del if the rename failed?
i guess the author of the code you fixed had the performance advantage of 
rename in mind when he wrote that code.

wish we had more contributors like you!!.. thanks in advance.. ede



reply via email to

[Prev in Thread] Current Thread [Next in Thread]