Use blivet's "device ID" as a unique device identifier #5435

vojtechtrefny · 2024-01-30T13:56:19Z

Anaconda currently uses name to identify devices internally which causes a lot of problems because multiple devices can share the same name. To fix this blivet introduced a new "device ID" which is unique even for devices with the same name (for technologies that support multiple devices with the same name and for devices sharing the same name but using a different storage technology).

This is not yet finished, but it should be usable for testing and further development on the WebUI side. The blivet change (storaged-project/blivet#1182) is merged but not yet released. Unit tests should be passing on this, I also tried running manual installation with mount point assignment in TUI and also running (most of) kickstart tests and everything seems to be working so far. The Gtk custom partitioning is not yet adjusted to this change but is somewhat working if you ignore the weird "names" displayed in the UI.

pep8speaks · 2024-01-30T13:56:45Z

Hello @vojtechtrefny! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file tests/unit_tests/pyanaconda_tests/modules/storage/test_module_device_tree.py:

Line 28:63: E231 missing whitespace after ','

Comment last updated at 2024-07-12 13:14:41 UTC

M4rtinK

Looks good to me code wise, but I'm afraid I can't really judge the full context of this change in regards to Blivet & Web UI. :P

rvykydal · 2024-02-13T09:22:05Z

If I create biosboot partition, boot partition and btrfs volume with a subvolume in terminal:

set -x

DISK=$1

sgdisk --zap-all ${DISK}
sgdisk --new=0:0:+1MiB --typecode=0:ef02 ${DISK}
sgdisk --new=0:0:+1GB ${DISK}
mkfs.ext4 -F ${DISK}2
sgdisk --new=0:0:+10GiB ${DISK}
mkfs.btrfs -f -L v3 ${DISK}3

# DUP
#sgdisk --new=0:0:+1GiB ${DISK}
#mkfs.btrfs -f -L v4 ${DISK}4


##mkfs.btrfs -f -f -L btrfstest ${DISK}3
##sgdisk --new=0:0:+2GiB ${DISK}
##mkfs.btrfs -f -f -L btrfstest ${DISK}4

TMP_MOUNT="/tmp/btrfs-mount-test"
mkdir -p ${TMP_MOUNT}

mount ${DISK}3 ${TMP_MOUNT}
btrfs subvolume create ${TMP_MOUNT}/root
umount ${TMP_MOUNT}

# DUP
#mount ${DISK}4 ${TMP_MOUNT}
#btrfs subvolume create ${TMP_MOUNT}/root
#umount ${TMP_MOUNT}

rmdir ${TMP_MOUNT}


#sgdisk --new=0:0:0 ${DISK}
#mkfs.xfs -f ${DISK}4


udevadm trigger
udevadm settle --timeout=120

lsblk
blkid

Then rescan the storage, go to mount points assignment, assign the partitions with reformatting I get failure in:

INFO:blivet:registered action: [224] create device btrfs subvolume root (id 219)
DEBUG:blivet:             DeviceTree.get_device_by_device_id: device_id: BTRFS-dafddd9d-7a42-414d-b66c-223d290d6b03-root ; incomplete: False ; hidden: False ;
DEBUG:blivet:             DeviceTree.get_device_by_device_id returned None
INFO:anaconda.core.threads:Thread Failed: AnaTaskThread-ManualPartitioningTask-1 (139822322681536)
ERROR:anaconda.modules.common.task.task:Thread AnaTaskThread-ManualPartitioningTask-1 has failed: Traceback (most recent call last):
  File "/usr/lib64/python3.12/site-packages/pyanaconda/core/threads.py", line 280, in run
    threading.Thread.run(self)
  File "/usr/lib64/python3.12/threading.py", line 1010, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/lib64/python3.12/site-packages/pyanaconda/modules/common/task/task.py", line 94, in _thread_run_callback
    self._task_run_callback()
  File "/usr/lib64/python3.12/site-packages/pyanaconda/modules/common/task/task.py", line 107, in _task_run_callback
    self._set_result(self.run())
                     ^^^^^^^^^^
  File "/usr/lib64/python3.12/site-packages/pyanaconda/modules/storage/partitioning/base_partitioning.py", line 53, in run
    self._run(self._storage)
  File "/usr/lib64/python3.12/site-packages/pyanaconda/modules/storage/partitioning/automatic/noninteractive_partitioning.py", line 43, in _run
    self._configure_partitioning(storage)
  File "/usr/lib64/python3.12/site-packages/pyanaconda/modules/storage/partitioning/manual/manual_partitioning.py", line 53, in _configure_partitioning
    self._setup_mount_point(storage, mount_data)
  File "/usr/lib64/python3.12/site-packages/pyanaconda/modules/storage/partitioning/manual/manual_partitioning.py", line 116, in _setup_mount_point
    mount_data.mount_options = device.format.options
                               ^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'format'

storage.log

I think it happens because the recreated subvolume is not found here:
https://github.com/vojtechtrefny/anaconda/blob/8dba5f232bfc90141a16859fe18f465f7a822345/pyanaconda/modules/storage/partitioning/manual/manual_partitioning.py#L169

It used to work (at least got past this point as I checked) but I am not 100% sure my use case makes sense. (Maybe the issue is that the volume is recreated as well because the only subvolume is destroyed so the uuids are new?)
I am looking into it.

rvykydal · 2024-02-13T09:57:03Z

It used to work (at least got past this point as I checked) but I am not 100% sure my use case makes sense. (Maybe the issue is that the volume is recreated as well because the only subvolume is destroyed so the uuids are new?) I am looking into it.

Yes, the issue here seems that if the volume is destroyed together with the subvolume and then recreated, finding the subvolume by device_id containing uuid of the former subvolume fails. I don't hit the issue if the volume contains 2 subvolumes and I reformat one of them in mount point assignment.

I'll update the webui tests for this PR, add a test for this specific case.
Then I'll perhaps update presentation of the device uuid in the WebUI to the user (like BTRFS-f586c782-f9da-4cfa-b9a4-c3da16914bae-data -> BTRFS subvolume data of volume vol (f586c782-f9da-4cfa-b9a4-c3da16914) - but this will be subject of discussion I guess.

vojtechtrefny · 2024-02-15T12:38:26Z

@rvykydal I've added a new commit that should fix this issue.

rvykydal · 2024-02-15T13:56:05Z

@rvykydal I've added a new commit that should fix this issue.

Thank you, I have just a few nitpicks.

pyanaconda/modules/storage/partitioning/manual/manual_partitioning.py

github-actions · 2024-04-30T02:05:48Z

This PR is stale because it has been open 60 days with no activity.
Remove stale label or comment or this will be closed in 30 days.

vojtechtrefny · 2024-07-09T11:52:48Z

This is mostly finished now. I plan to do some more manual testing to be sure everything works, but other than that I don't plan to make more changes.

If you wish to help with testing, you can find updates image for rawhide with all the changes (and latest blivet which also has some fixes) is available here.

Anaconda currently uses name to identify devices internally which causes a lot of problems because multiple devices can share the same name. To fix this blivet introduced a new "device ID" which is unique even for devices with the same name (for technologies that support multiple devices with the same name and for devices sharing the same name but using a different storage technology).

We can't use the device factory in the manual partitioning because when it removes the last subvolume it also removes the volume which want to keep untouched when just recreating the subvolume.

We want to show device name (or description) in the reclaim space dialog but use the unique device ID internally.

This should cover all places where name was used as device identifier in the storage, advanced storage and custom spokes.

KKoukiou

Should be mark in the specfile tha tthis current anaconda-core is incomptatible with the current version of the Web UI package?

KKoukiou · 2024-07-26T12:41:32Z

pyanaconda/ui/gui/spokes/lib/resize.glade

@@ -26,78 +28,78 @@
 </object>
 <object class="GtkAdjustment" id="resizeAdjustment">
 <property name="upper">100</property>
- <property name="step_increment">1</property>


These changes are unrelated. I guess these are autogenerated. Is there a reason for storing these here?

vojtechtrefny temporarily deployed to gh-cockpituous January 30, 2024 13:56 — with GitHub Actions Inactive

github-actions bot added the f40 label Jan 30, 2024

M4rtinK approved these changes Jan 30, 2024

View reviewed changes

vojtechtrefny force-pushed the master_devices-device-id branch from 804ac0b to 8dba5f2 Compare February 1, 2024 08:08

vojtechtrefny temporarily deployed to gh-cockpituous February 1, 2024 08:08 — with GitHub Actions Inactive

vojtechtrefny temporarily deployed to gh-cockpituous February 15, 2024 09:29 — with GitHub Actions Inactive

rvykydal mentioned this pull request Feb 15, 2024

Update tests for device_id change rhinstaller/anaconda-webui#177

Draft

2 tasks

rvykydal reviewed Feb 15, 2024

View reviewed changes

pyanaconda/modules/storage/partitioning/manual/manual_partitioning.py Outdated Show resolved Hide resolved

rvykydal reviewed Feb 15, 2024

View reviewed changes

pyanaconda/modules/storage/partitioning/manual/manual_partitioning.py Outdated Show resolved Hide resolved

vojtechtrefny force-pushed the master_devices-device-id branch from 537b079 to ffd316a Compare February 15, 2024 15:10

vojtechtrefny temporarily deployed to gh-cockpituous February 15, 2024 15:10 — with GitHub Actions Inactive

vojtechtrefny force-pushed the master_devices-device-id branch from ffd316a to 98db250 Compare February 19, 2024 09:46

vojtechtrefny temporarily deployed to gh-cockpituous February 19, 2024 09:46 — with GitHub Actions Inactive

KKoukiou added f41 and removed f40 labels Feb 29, 2024

github-actions bot added the stale label Apr 30, 2024

KKoukiou removed the stale label May 7, 2024

vojtechtrefny force-pushed the master_devices-device-id branch from 98db250 to 3270ab2 Compare June 25, 2024 08:18

vojtechtrefny temporarily deployed to gh-cockpituous June 25, 2024 08:18 — with GitHub Actions Inactive

vojtechtrefny force-pushed the master_devices-device-id branch from 3270ab2 to 2f1b9a3 Compare June 26, 2024 10:12

vojtechtrefny temporarily deployed to gh-cockpituous June 26, 2024 10:12 — with GitHub Actions Inactive

vojtechtrefny temporarily deployed to gh-cockpituous June 26, 2024 12:58 — with GitHub Actions Inactive

vojtechtrefny force-pushed the master_devices-device-id branch from 3b6955b to c107e89 Compare July 8, 2024 13:01

vojtechtrefny had a problem deploying to gh-cockpituous July 8, 2024 13:01 — with GitHub Actions Failure

vojtechtrefny force-pushed the master_devices-device-id branch from c107e89 to d1befbc Compare July 9, 2024 10:56

vojtechtrefny temporarily deployed to gh-cockpituous July 9, 2024 10:56 — with GitHub Actions Inactive

vojtechtrefny marked this pull request as ready for review July 9, 2024 11:49

vojtechtrefny changed the title ~~[WIP] Use blivet's "device ID" as a unique device identifier~~ Use blivet's "device ID" as a unique device identifier Jul 9, 2024

vojtechtrefny added the manual testing required This issue can't be merged without manual testing label Jul 9, 2024

vojtechtrefny added 3 commits July 11, 2024 09:22

Add a custom function for recreating btrfs subvolumes

2bada3f

We can't use the device factory in the manual partitioning because when it removes the last subvolume it also removes the volume which want to keep untouched when just recreating the subvolume.

Adjust resize module to the device ID API

4c809e9

We want to show device name (or description) in the reclaim space dialog but use the unique device ID internally.

vojtechtrefny force-pushed the master_devices-device-id branch from d1befbc to 651f24c Compare July 11, 2024 08:47

vojtechtrefny temporarily deployed to gh-cockpituous July 11, 2024 08:47 — with GitHub Actions Inactive

Adjust custom partitioning and storage spokes to the device ID API

922df8c

This should cover all places where name was used as device identifier in the storage, advanced storage and custom spokes.

vojtechtrefny force-pushed the master_devices-device-id branch from 651f24c to 922df8c Compare July 12, 2024 13:14

vojtechtrefny temporarily deployed to gh-cockpituous July 12, 2024 13:14 — with GitHub Actions Inactive

KKoukiou reviewed Jul 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use blivet's "device ID" as a unique device identifier #5435

Use blivet's "device ID" as a unique device identifier #5435

vojtechtrefny commented Jan 30, 2024

pep8speaks commented Jan 30, 2024 •

edited

Loading

M4rtinK left a comment

rvykydal commented Feb 13, 2024

rvykydal commented Feb 13, 2024 •

edited

Loading

vojtechtrefny commented Feb 15, 2024

rvykydal commented Feb 15, 2024

github-actions bot commented Apr 30, 2024

vojtechtrefny commented Jul 9, 2024 •

edited

Loading

KKoukiou left a comment

KKoukiou Jul 26, 2024

Use blivet's "device ID" as a unique device identifier #5435

Are you sure you want to change the base?

Use blivet's "device ID" as a unique device identifier #5435

Conversation

vojtechtrefny commented Jan 30, 2024

pep8speaks commented Jan 30, 2024 • edited Loading

Comment last updated at 2024-07-12 13:14:41 UTC

M4rtinK left a comment

Choose a reason for hiding this comment

rvykydal commented Feb 13, 2024

rvykydal commented Feb 13, 2024 • edited Loading

vojtechtrefny commented Feb 15, 2024

rvykydal commented Feb 15, 2024

github-actions bot commented Apr 30, 2024

vojtechtrefny commented Jul 9, 2024 • edited Loading

KKoukiou left a comment

Choose a reason for hiding this comment

KKoukiou Jul 26, 2024

Choose a reason for hiding this comment

pep8speaks commented Jan 30, 2024 •

edited

Loading

rvykydal commented Feb 13, 2024 •

edited

Loading

vojtechtrefny commented Jul 9, 2024 •

edited

Loading