storage: Exclude eMMC "boot" non-partitions from multipath logic #20329
Conversation
mvollmer
force-pushed
the
storage-recognize-mmcblk-boot-devices
branch
from
8a14c96
to
3cf24bc
Compare
|
I have no idea how to test this... We might have to be content with not breaking anything that we can test, and getting feedback from the reporter of #20250. |
|
Agreed - that's why we have packit COPRs, for easy user testing 😁 |
|
I can reproduce this on my Surface: It has an interesting bug with the empty card With the patch: No changes, let me re-check if I patched it correctly. FYI: |
pkg/storaged/client.js
Outdated
| // to the main Drive. We identify them by their name, just like | ||
| // UDisks2. | ||
|
|
||
| if (utils.decode_filename(block.Device).match(/^mmcblk[0-9]boot[0-9]$/)) |
There was a problem hiding this comment.
This won't work, the devices have a /dev/ prefix
There was a problem hiding this comment.
- if (utils.decode_filename(block.Device).match(/^mmcblk[0-9]boot[0-9]$/))
+ if (utils.decode_filename(block.Device).match(/\/dev\/mmcblk[0-9]boot[0-9]$/))
This works.
There was a problem hiding this comment.
Also our add_disk code does this:
559 │ if not serial:
560 │ serial = f"DISK{index}"
561 │ dev = 'sd' + string.ascii_lowercase[index]
562 │ extra = "<boot order='1'/>" if boot_disk else ""
563 │ disk_desc = TEST_DISK_XML % {
564 │ 'file': image,
565 │ 'serial': serial,
566 │ 'unit': index,
567 │ 'dev': dev,
568 │ 'type': image_type,
569 │ 'extra': extra,
570 │ }
So the device name seems user controlled, we could add a test for that? Or maybe via the ram_disk route?
There was a problem hiding this comment.
So the device name seems user controlled
Not really... I think the dev = 'sd' + index is just a good guess at what the device will be called in the guest. It doesn't control it.
There was a problem hiding this comment.
This won't work, the devices have a
/dev/prefix
Ouch, that's why we test, I guess. :-) Thanks!
eMMC devices can have "boot" sub-devices which should be excluded from multipath considerations, just like partitions. Fixes cockpit-project#20250
mvollmer
force-pushed
the
storage-recognize-mmcblk-boot-devices
branch
from
3cf24bc
to
a3f1f0a
Compare
|
@jelly, can you please test again? |
| // UDisks2. | ||
|
|
||
| if (utils.decode_filename(block.Device).match(/\/dev\/mmcblk[0-9]boot[0-9]$/)) | ||
| return false; |
There was a problem hiding this comment.
This added line is not executed by any test.
eMMC devices can have "boot" sub-devices which should be excluded from multipath considerations, just like partitions.
Fixes #20250