[VU+ Uno4KSE] Enigma2 Timeshift Bug

[vampyrebat]

Hi there,

Am fairly confident I've uncovered a longstanding bug (regression) with time shift that should be a fairly easy fix. I've proved it by downloading the enigma-src and adding further debug to the code.

I say regression because the part of the code in question suspiciously has a commented line with a different check commented out to be replaced by something that definitely isn't working.

If you trawl the forums you'll often run up against people complaining about the error message that they have to format their drive with ext2/ext3 to support hard links and sure most of them probably screwed up but there are a minority like me that have done things correctly but have been affected by this bug which makes them feel they've not formatted their drive correctly when in reality it's a totally misleading error message. So...

In:

https://github.com/OpenViX/enigma2/blob/master/lib/python/Components/Timeshift.py

At about line 884:

# if filename.startswith("timeshift") and not os.path.splitext(filename)[1]:
if filename.startswith("timeshift") and not filename.endswith(".sc") and not filename.endswith(".del") and not filename.endswith(".copy"):

We are effectively checking to make sure we don't enter this part of the code for certain file extensions but what is happening is we are not checking for ".ap" files and for some reason (of which I'm not quite sure), sometimes on rare occasion one of those exists and is being processed here. So that later when we try to do:

os.link("%s%s.sc" % (config.usage.timeshift_path.value,filename), "%spts_livebuffer_%s.sc" % (config.usage.timeshift_path.value,self.pts_eventcount))

.. with an .ap file it is actually trying to hard link a "ap.sc" file which will never exist! (It's either .ap or .sc) causing the os.link to fail and the exception to think that it just can't make a hard link because of the drive format whereas in reality, it shouldn't be processing this .ap file.

The fix seems simple (but maybe implemented better by someone more familiar with this code), to add 'and not filename.endswith(".ap")' to the end of the 'if' statement at the top and to add a delete for old '.ap' files with all those other deletes just below it.

Hope this helps! This is working well for me and should help people out with the occasional error about hard links and ext3 formats. Happy to chat over.

Cheers.

 

[abu baniaz]

Can you attach your modified file please?

 

[vampyrebat]

No problem! Please find attached. Includes the debug lines proving the problem the output of which is here:

< 207.559> hl1a /media/hdd/timeshift/timeshift.yiRdFW.ap
< 207.559> hl1b /media/hdd/timeshift/pts_livebuffer_1
< 207.559> hl2a /media/hdd/timeshift/timeshift.yiRdFW.ap.sc
< 207.559> hl2b /media/hdd/timeshift/pts_livebuffer_1.sc
< 207.564> [Skin] processing screen MessageBoxSimple:
< 207.569> [GUISkin] warning, skin is missing element autoresize in <class 'Screens.MessageBox.MessageBox'>(Creating Hardlink to Timeshift file failed!
The Filesystem on your Timeshift-Device does not support hardlinks.
Make sure it is formatted in EXT2 or EXT3!

[Errno 2] No such file or directory)< 207.569>

Obviously that debug output is prior to adding the check for .ap files. I hope this helps!

 

[vampyrebat]

To prevent enigma2 trying to hard link .ap files line 885:

if filename.startswith("timeshift") and not filename.endswith(".sc") and not filename.endswith(".del") and not filename.endswith(".copy"):

Should probably become:

if filename.startswith("timeshift") and filename.count('.') == 1:

All seems to be working well for me now but maybe someone with a better understanding of that code can verify it for me and hopefully fix it for a future release.

Thank you!

 

[birdman]

if filename.startswith("timeshift") and filename.count('.') == 1:

Which would appear to be checking the number of "." chars in the filename.
From your analysis it seem that you really just want to know whether the file actually exists, so better to check for that rather than some indirect concept of why it might not exist.

So,

        if filename.startswith("timeshift") and os.path.isfile(filename):

 

[vampyrebat]

Hi Birdman!

It's a little different from that. I'm checking that I get a file without two extensions.

i.e that block of code needs to process:

timeshift.yiRdFW

but not:

timeshift.yiRdFW.ap or timeshift.yiRdFW.sc

If it processes the .ap it will throw an exception. The original bug. I was toying with the idea of checking the filename from the dvb stream processor but I don't want to get my hands too dirty yet before I understand more of the system, I'm just a noobie

Hope that explanation helps!

 

[birdman]

Hi Birdman!

It's a little different from that. I'm checking that I get a file without two extensions.

Hmmmm. a pity the timeshift tag wasn't appended with a "_" rather than a ".". I suspect it needs to ignore anything that has an actual extension rather than a tag.

I'd suggest adding a comment to indicate why that test is there.

 

[birdman]

A fix has been added (to add ".ap" to the check list):

 https://github.com/OpenViX/enigma2/commit/7105f6d7b2107b2ef4367531a0a3533faaf021ad

However, such a convoluted test strikes me as indicating there is something fundamentally wrong elsewhere.

 

[vampyrebat]

Thanks for the reply. I made that commit last night. There's now an open PR. I opted for adding just the .ap check for clarity and I know that definitely fixes the original bug I was encountering. I completely agree with what you say. This bug might even just be the side effect of a different bug. I wish I understood the code a bit better so I could help clean it up a bit, there are almost certainly other solution opportunities here. I might try on the weekend.

 

[abu baniaz]

Can you please submit any further pull requests to the Dev branch?

 

[vampyrebat]

Can you please submit any further pull requests to the Dev branch?

Makes sense! Sure thing.