Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retrieve path device and filesystem via df command #607

Closed
wants to merge 1 commit into from

Conversation

karlicoss
Copy link

@karlicoss karlicoss commented Jul 28, 2016

So, I finally decided to automate my backups and use backintime. I tested scheduled udev-triggered backups on a flash drive few days ago, everything worked fine. So I decided to configure backups on my external HDD today, but it didn't work, plugging in the drive didn't trigger backintime.

After a short investigation, I found out my udev file was set up to trigger on some unknown UUID, even blkid wouldn't list it. So, I dug in source code and figured out that the tools.device function returned None for my drive.

The reason was: its label ('karlicoss hdd') contained a space, so it was mounted to /media/karlicos/karlicoss hdd/, however, /etc/mtab treats spaces as separators, so he escapes paths like

`/dev/sdb1 /media/karlicos/karlicoss\040hdd fuseblk rw,nosuid,nodev,....`

As you can see, spaces map to \040. So, probably a good idea is, instead of manually parsing /etc/mtab, use the df command from coreutils, which does all the job for us.

A followup question, what is the rationale behind this UUID fallback logic? In my case, it seemed to have cached the flash drives' UUID, and silently used it instead my HDD's UUID. Maybe, we should at list show some warning/notification to user so he knows something went not as expected?

P.S. I messed with previous PR and apparently github doesn't let to reopen branch which you force pushed so I made a new PR.

df command encapsulates mtab so we don't have to parce it ourselves.
In particular, fixes a bug when paths with spaces where not handled
properly.
@coveralls
Copy link

coveralls commented Jul 28, 2016

Coverage Status

Coverage increased (+0.007%) to 70.562% when pulling 790960e on karlicoss:fix-spaces-in-labels into df13c19 on bit-team:master.

@Germar
Copy link
Member

Germar commented Jul 29, 2016

Thanks karlicoss for the Pull Request. I used df for this before but changed it to reduce dependencies. I prefer to stay with parsing /etc/mtab and just decode the string correctly.

The UUID fallback is used if your drive is not connected during you make changes inside a different profile. To prevent using a wrong UUID like it happen in your case I will now remove the cached UUID if a user changes the snapshot path (which will most likely be if the user changes the drive).

@Germar Germar closed this in 532b6bb Jul 29, 2016
@karlicoss
Copy link
Author

karlicoss commented Jul 29, 2016

@Germar hmm, df is in coreutils, so not sure how one could not have it in his system, but okay, thanks for the fix.
Oh, and thanks for the fallback explanation, now I get it :)

@Germar
Copy link
Member

Germar commented Jul 29, 2016

Under normal circumstances it should be on the system, sure. But if you think about chroot or containers it must be a dependency. The ArchLinux folks for example run unittests for packages in a clean chroot environ and already failed without coreutils dependency before. Also Ubuntus new Snap containers for Applications would need the dependency.

@karlicoss
Copy link
Author

Oh I see, I probably didn't have the big picture. Thanks for making this clear :)

@jackyohh
Copy link

jackyohh commented Oct 28, 2017

Is this bug officially fixed? ... because i still get this error with Version 1.1.20. drive names with space(s) still make problems when i want to autobackup on external drives (with udev).

"Error: Couldn't find UUID for "/media/DRIVE WITH SPACE/BACKUP-FOLDER/backintime/PC-NAME/root/1"

fast workaround is to rename external drives and use underline instead of spaces.

@Germar
Copy link
Member

Germar commented Oct 28, 2017

It's fixed in current dev version 1.2.0 but not in 1.1.20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants