Skip to content
This repository has been archived by the owner on Feb 27, 2023. It is now read-only.

LibreELEC labels #456

Closed
procount opened this issue Nov 16, 2017 · 18 comments
Closed

LibreELEC labels #456

procount opened this issue Nov 16, 2017 · 18 comments

Comments

@procount
Copy link
Contributor

The partition labels provided in partitions.json for LibreELEC are too long. i.e. "LibreELEC_RPi_System" and "LibreELEC_RPi_Storage".

If NOOBS finds a label longer than 15 characters, it will completely clear the label and not give the partition any label at all.

As this is an externally hosted OS, could you pass on this request to the LibreELEC maintainers to reduce their partition names to 15 characters or less?

@procount
Copy link
Contributor Author

Actually, a max length of 15 characters (allowing an extra character for an additional number to disambiguate labels with the same name) is only applicable to ext partitions. In FAT it should be 10 (+1) and in NTFS it is 31 (+1). Not sure about others fstypes. so the LibreELEC FAT partition should be 10 chars or less. Maybe NOOBS needs a little tweak for this?

@lurch
Copy link
Collaborator

lurch commented Nov 16, 2017

I did a quick search, and found the code responsible for this at https://github.com/raspberrypi/noobs/blob/master/recovery/multiimagewritethread.cpp#L452

It does seem a bit odd to completely clear the label, perhaps truncating it (again taking the destination FS into account) would be more appropriate?

BTW @procount does this actually cause install or boot problems, or is it merely a 'cosmetic' issue?

@procount
Copy link
Contributor Author

Yep, that's the spot. Sorry I should have linked to it.

AFAIK it does not cause a problem with NOOBS, except that the 2 LibreELEC partitions will have no names to identify them. So yes, it is more cosmetic than anything else. Although if there was an OS that referenced it's partitions (e.g. in fstab) by label (which has always been a possibility in NOOBS), then that WOULD be an issue.

I haven't checked any other OSes to see if any others are affected.

This problem was identified when testing a new feature for the next version of PINN, so it is causing me a problem and I shall have to work around it somehow. Truncating the label appropriately seems like a solution, but from which end should it be truncated in order for the remainder still to be meaningful? In the case of LibreELEC, restricting both labels to 15 characters from the start would end up with "LibreELEC_RPi2_" for both labels, so maybe truncating the beginning might be better? I don't know of any conventions here 🤷‍♂️

@lurch
Copy link
Collaborator

lurch commented Nov 16, 2017

One 'truncation-convention' that sometimes gets used when both the beginning and end of a filename need to be seen, is to truncate e.g. "LibreELEC_RPi_Storage" to ""LibreE...torage" - but I've got no idea if dots are allowable in filesystem labels! ;-)

@XECDesign
Copy link
Contributor

There are no plans to make further changes to NOOBS in the near future.
Issues related to LibreELEC can be addressed here through a pull request:
https://github.com/LibreELEC/LibreELEC.tv

@procount
Copy link
Contributor Author

Hmm.

LibreELEC uses "@DISTRONAME@_@PROJECT@_System" for the label name
https://github.com/LibreELEC/LibreELEC.tv/blob/master/config/noobs/partitions.json#L4

They have no issue system, just PRs. so you can only report an issue by providing a solution....
I'll see what I can do within PINN to resolve this.

@lurch
Copy link
Collaborator

lurch commented Nov 16, 2017

They have no issue system, just PRs.

The README on the repo that @XECDesign linked to says "Please report issues via the LibreELEC forum: Bug Reports" 😉

@procount
Copy link
Contributor Author

procount commented Nov 16, 2017

@lurch - Doh! I missed that. Thanks, I'll raise a bug there.

@procount
Copy link
Contributor Author

procount commented Nov 17, 2017

A proposal to fix this...

EDIT: Deleted for improvements

@procount
Copy link
Contributor Author

I'm not sure the code to clear out the partitions is working sufficiently.
https://github.com/raspberrypi/noobs/blob/master/recovery/multiimagewritethread.cpp#L301
It seems to work ok for FAT partitions, but not for ext. I suspect the label is still being retrieved from a backup superblock(?)
If I install 3 OSes, and then reinstall them again straightaway, over the top of the existing, isLabelAvailable() indicates the labels for the ext partitions are still there. Dropping to a shell whilst the OSes are installing and using lsblk shows the ext labels are still there, but the Fat labels are blank.
Maybe before wiping partitions we need to clear out ext labels using e2label or something first so all the superblocks are cleared? It seems to work manually.
I will keep experimenting....

@lurch
Copy link
Collaborator

lurch commented Nov 18, 2017

Nice catch - that might explain why, when I was doing lots of NOOBS testing many years ago, I'd sometimes "mysteriously" get numbers added to the end of my partition labels, even though there didn't appear to be any label conflicts ;-)

But again, this is merely a cosmetic issue.

@procount
Copy link
Contributor Author

Yes, that's exactly why!
Cosmetic only for NOOBS, but I shall make it more robust in PINN because I need to for some additional features.
When it's done & tested I'll submit a PR for NOOBS just in case someone finds some time for another release.

@XECDesign -despite there not being any plans to update noobs in the near future for additional features, you have done a lot of recent tidy ups and added some new translations since the last release. Will you be making a minor release to include those latest commits at any time?

@procount
Copy link
Contributor Author

Actually, I think the fix is quite simple - change count=1 to count=3 in the above line.
According to https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout, the first 1024 bytes of an EXT4 partition is just padding and the Superblock follows it. so the existing line does nothing on an ext4 partition and explains why the volume names are still present.
Wipefs seems to just erase the Magic number at 0x438, but wiping the first 3 sectors should cater for FAT and EXT4 partitions.
Repeating the above tests shows the labels have now gone.

@XECDesign
Copy link
Contributor

@XECDesign -despite there not being any plans to update noobs in the near future for additional features, you have done a lot of recent tidy ups and added some new translations since the last release. Will you be making a minor release to include those latest commits at any time?

Tidying up leaves less things to worry about in the future and I had some time to spare. That wasn't done in preparation for an update. At some point I will do another pass to test and merge the simple fixes and close off the wontfix issues. Then maybe it will make sense to do a point release.

@procount
Copy link
Contributor Author

OK, thanks for the clarifications.

BTW I've been checking other OSes and RISC-OS also has a FAT partition label that is too long: "RISC_OS_Boot", although enough of it is different to the ROOT partition name to be unambiguous.

@procount
Copy link
Contributor Author

procount commented Nov 20, 2017

@lurch - I came up with an algorithm that would shorten overly long volume names. What do you think of the outcome for NTFS(32), EXT4(16) and FAT(11):

"LibreELEC_RPi2_System" 32
"Libr_RPi2_System" 16
"L_i2_System" 11
"LibreELEC_RPi2_Storage" 32
"Lib_RPi2_Storage" 16
"L_2_Storage" 11
"RISC_OS_Boot" 32
"RISC_OS_Boot" 16
"RI_OS_Boot" 11
"boot_picorea" 32
"boot_picorea" 16
"boo_picorea" 11

Code available at https://gist.github.com/procount/50f8fbfec0bfe88bc6f24cddef0ca430

@procount
Copy link
Contributor Author

There's another problem in that the boot partitions will not be cleared at all if installing to USB (i.e. under a _multidrive situation). I have a fix that I will include in my PR

@procount
Copy link
Contributor Author

Reopened now that fixes have been provided as PR #460 and PR #461 so that this is not forgotten.

@XECDesign - If you do another tidy-up release in the future, please consider implementing these bugfixes: #460,#461,#462 and #381

@procount procount reopened this Nov 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants