Use partuuid in installed_os.json #497
Conversation
|
In order for the 1st commit to compile, I also had to move 4 uuid related functions from multiimagewritethread.cpp to util.cpp. |
| @@ -530,7 +530,13 @@ bool MultiImageWriteThread::processImage(OsInfo *image) | |||
| QVariantList vpartitions; | |||
| foreach (PartitionInfo *p, *partitions) | |||
| { | |||
| vpartitions.append(p->partitionDevice()); | |||
| QString part = p->partitionDevice(); | |||
| if (part.left(13) != "/dev/mmcblk0p") | |||
There was a problem hiding this comment.
Just out of curiosity, what happens if you remove this special-casing for /dev/mmcblk0 and instead always use the partuuid?
There was a problem hiding this comment.
We used to have some Linux distributions that came with an initramfs that did not support partuuid parameters (e.g. OSMC).
Not sure if that's still the case in recent versions though.
There was a problem hiding this comment.
@procount I wonder if it'd be worth adding an extra param to the os.json / os_list.json files stipulating whether they support partuuid parameters, and then only using this partuuid code if they do?
There was a problem hiding this comment.
Can we piggy-back the supports_usb_root or support_usb_boot parameters for that? I think they serve a similar purpose...?
There was a problem hiding this comment.
IIRC, if an OS doesn't support partuuid, then it can't be installed to USB. If so, then the additional code won't be necessary. I'm sure @XECDesign or @maxnet explained the use of supports_usb_root and supports_usb_boot somewhere but I can't find the reference.
Might be beneficial to swap the order of the commits? |
|
If I were to rebase to reorder the commits (or even squash them), wouldn't it wreck all this conversation, comments, history etc when I push it up (probably with -f to force it?) |
|
Nope, I don't think so. |
|
OK I'm willing to give it a go - Which do you prefer: swap or squash? |
procount
force-pushed
the
partuuid
branch
2 times, most recently
from
bf6f5e8
to
08fa9ff
Compare
|
I swapped first, then squashed, and both worked, so I guess the git warnings about rebasing and force pushing only apply to the public repository that others have used and not to pull requests. |
|
Yeah, IIRC the git warnings are about mucking up the history if somebody else has already cloned the branch you're rebasing - but since you're rebasing a private branch in order to create a PR, it's very unlikely that somebody else has cloned your |
|
Updated with bugfix to use Hexadecimal partition numbers from PARTUUIDs |
|
@maxnet, does this look okay to you? |
|
Looks ok, although I haven't tried it. |
|
Thanks for taking a look. I'll test it out before it goes in a release. |
|
|
||
| int partitionNr; | ||
| QRegExp parttype("^PARTUUID"); | ||
| if (parttype.indexIn(partition) == -1) |
There was a problem hiding this comment.
You could probably use http://doc.qt.io/archives/qt-4.8/qstring.html#startsWith here instead?
|
@XECDesign - I finally got around to testing this with boot on sd card and root on USB stick. |
|
Many thanks |
When installing an OS to a USB storage device, the partitions used for the OS are referenced assuming the device remains consistent as /dev/sdaX. I had one user who booted PINN with 2 USB sticks - one with the OS installations on it (sda), and the other used as the destination (sdb). Naturally the partition was stored as /dev/sdb6 in installed_os.json, but on later boots, the device was referenced as /dev/sda so the OS could not be found.
I think that OSes installed to an external USB should be referenced by partuuid in os_config.json and installed_os.json, just like is done in cmdline.txt and /etc/fstab.
Do you think the attached Pull Request will solve this issue? Are there any problems with it?