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

Don't convert partuuids for nonexistent source partitions #8

Merged
merged 1 commit into from
Mar 8, 2024

Conversation

aaronkollasch
Copy link

Copying this issue from billw2#149 as that repo is abandoned.

Issue

When converting to partuuids, if there is an empty element in ${src_partition[@]}, then /etc/fstab will have /dev/ replaced with PARTUUID= without actually changing the device name to a PARTUUID.

I don't have the exact logs when this happened to me, but it looked something like this:

$ sudo rpi-clone -l sda --convert-fstab-to-partuuid

This will change your /etc/fstab, are you sure?  (yes/no): yes

Converting /etc/fstab from device names to PARTUUID
lsblk: /dev/: not a block device
  Editing /etc/fstab, changing /dev/ to
...

and I ended up with a broken fstab:

proc            /proc           proc    defaults          0       0
PARTUUID=mmcblk0p6  /boot           vfat    defaults          0       2
PARTUUID=mmcblk0p7  /               ext4    defaults,noatime  0       1
# a swapfile is not a swap partition, no line here
#   use  dphys-swapfile swap[on|off]  for that

I had previously run sudo rpi-clone -l sda so perhaps that is why there was an empty element in src_partition, I'm not sure.

Changes

Check that ${src_partition[p]} is not empty before editing /etc/fstab.

Then, the command I used will instead exit with an error, which is much safer:

Could not find any sda partition names in /etc/fstab, nothing changed.

@geerlingguy
Copy link
Owner

Since I don't personally use this feature, if someone else can give the thumbs up on the change, I'd be happy to merge!

@framps
Copy link

framps commented Mar 7, 2024

Wouldn't it make sense to make sure there is no empty element in src_partition? Would be interesting to know you partitioning which causes this issue 😉

@aaronkollasch
Copy link
Author

Wouldn't it make sense to make sure there is no empty element in src_partition? Would be interesting to know you partitioning which causes this issue 😉

My disk has primary partitions sda1 and sda2, and then logical partitions sda5+, so there will be indices in between without partitions, whereas the code iterates through indices 1-n. If you look at where src_partition is created, there is actually a check for empty lines:

rpi-clone/rpi-clone

Lines 677 to 694 in 423470c

for ((p = 1; p <= n_src_parts; p++))
do
line=$(echo "$src_partition_table" | grep -e "^${p}:")
if [ "$line" == "" ]
then
src_exists[p]=0
continue
fi
src_exists[p]=1
if ((p == root_part_num))
then
src_partition[p]=${src_root_dev#/dev/}
src_device[p]=$src_root_dev
else
src_partition[p]="${src_part_base}${p}"
src_device[p]="/dev/${src_partition[p]}"
fi

and there's also a check later on in the main code path:

rpi-clone/rpi-clone

Lines 1001 to 1006 in 423470c

for ((p = 1; p <= n_src_parts; p++))
do
if ((!src_exists[p]))
then
continue
fi

But it doesn't seem to have made it to the part where partuuids get converted:

rpi-clone/rpi-clone

Lines 958 to 967 in 423470c

for ((p = 1; p <= n_src_parts; p++))
do
if grep -q "^/dev/${src_partition[p]}" $fstab_tmp
then
partuuid=$(lsblk -n -o PARTUUID /dev/${src_partition[p]})
sed -i "s/\/dev\/${src_partition[p]}/PARTUUID=$partuuid/" $fstab_tmp
printf " Editing $fstab, changing /dev/${src_partition[p]} to $partuuid\n"
((++count))
fi
done

(actually I might change my code to match L1003-L1006 and check src_exists[p])

@framps
Copy link

framps commented Mar 7, 2024

I didn't detect Bill already used an array to mark a partition as not to exist :-(. Frankly I would just delete this partition in the list instead of mark it as non existing. But then you can't use a simple array with partition indices any more and will need much more complicated logic :-(

I agree to use src_exists[p] is a much more clean way to handle this exception.

@aaronkollasch aaronkollasch changed the title Don't convert partuuids for empty src_partition Don't convert partuuids for nonexistent source partitions Mar 8, 2024
Copy link

@framps framps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@geerlingguy geerlingguy merged commit 966b2b2 into geerlingguy:master Mar 8, 2024
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.

3 participants