gen_udev_rules: add raw partition rule generator - #156
Conversation
|
Hi Igor Opaniuk (@igoropaniuk), a friendly review ping when you have time. This PR provides the qcom-ptool side of qualcomm-linux/meta-qcom#2865. I would particularly appreciate feedback on:
All current CI checks are passing. The meta-qcom integration is waiting on this interface, so an initial review or direction would be very helpful. |
|
Hi Wenwen Fu (@wenwfu) . Hi, thanks for the ping and apologies for the delay. Is qcom-ptool the right place? CLI and policy format I've review changes later today and leave comments. |
162137c to
acb0c28
Compare
Hi Igor Opaniuk (@igoropaniuk), this PR has been rebased onto the latest main and the conflict has been resolved. All 128 local tests pass. Could you please review it when you have time? Thanks! |
| try: | ||
| content = generate_rules(args.inputs) | ||
| if not content: | ||
| args.output.unlink(missing_ok=True) |
There was a problem hiding this comment.
a typo'd -o pointing at an existing unrelated file (say, another rules file in /etc/udev/rules.d/) gets deleted, with exit 0 and only a "skipped" message. Writing nothing, or refusing to touch an existing file it didn't create, would be safer.
| GOTO="qcom_raw_noblkid_end" | ||
|
|
||
| LABEL="qcom_raw_noblkid" | ||
| ENV{UDEV_DISABLE_PERSISTENT_STORAGE_BLKID_FLAG}="1" |
There was a problem hiding this comment.
This will work only on recent versions of systemd.
There was a problem hiding this comment.
Good point. UDEV_DISABLE_PERSISTENT_STORAGE_BLKID_FLAG was introduced in systemd v252, so this rule does require systemd >= v252.
The older alternative is UDEV_DISABLE_PERSISTENT_STORAGE_RULES_FLAG, but that skips the entire 60-persistent-storage.rules file, including parent metadata imports and other persistent links. Supporting that safely would require duplicating more version-specific systemd behavior in this rule.
The blkid-specific flag is narrower: it skips only filesystem probing while preserving the rest of the standard persistent-storage processing. Therefore I would prefer to keep it and document the systemd >= v252 requirement. Do you expect this generator to support pre-v252 systemd as well?
There was a problem hiding this comment.
The current meta-qcom integration uses OE-Core with systemd v259, so the flag is supported by its intended consumer.
There was a problem hiding this comment.
What about other distros? Debian trixie? Ubuntu LTS?
There was a problem hiding this comment.
Good point. The current rule relies on UDEV_DISABLE_PERSISTENT_STORAGE_BLKID_FLAG, which was added in systemd v252. Debian Trixie and Ubuntu 24.04 LTS are covered, but Ubuntu 22.04 LTS is not.
I will either add a compatibility path for older systemd versions, or document systemd v252 as the minimum requirement and make sure the integration does not silently claim support for older systems. I agree that the current version should make this requirement explicit.
| TEMPLATE_FILE = DATA_DIR / "55-qcom-raw-partitions-noblkid.rules.in" | ||
| RULES_PLACEHOLDER = "@QCOM_RAW_PARTITION_RULES@" | ||
| LABEL_RE = re.compile(r"[A-Za-z0-9_.+-]+") | ||
| PATTERN_RE = re.compile(r"[A-Za-z0-9_.+*?\[\]-]+") |
There was a problem hiding this comment.
Seems that PATTERN_RE rejects [!...] negation globs, which udev and fnmatch both support
acb0c28 to
31bf38d
Compare
|
LGTM |
|
Thanks Igor Opaniuk (@igoropaniuk) for the review! All comments have been addressed. Dmitry Baryshkov (@lumag), could you please take a final look and approve/merge when convenient? |
| # SPDX-License-Identifier: BSD-3-Clause | ||
|
|
||
| # Partition-name patterns reviewed as safe to exclude from blkid probing. | ||
| # Keep one udev glob per line. A new partition label is not implicitly safe. |
There was a problem hiding this comment.
I really don't like having this list of partitions. I though that the idea was to generate it on the fly. If so, it should not be a part of the commit.
There was a problem hiding this comment.
The generated rules are still machine-specific: the generator collects the partition labels from the selected layouts and emits rules only for labels present in those layouts.
approved-raw-partition-patterns.list is not generated output; it is the reviewed allowlist used to decide which labels are safe to exclude from blkid. We cannot generate the rules from all layout labels blindly, since that could accidentally disable probing for filesystem or custom partitions.
If you prefer this policy to be owned by meta-qcom rather than packaged in qcom-ptool, I can make it an explicit policy input to the generator. Do you have a preference?
There was a problem hiding this comment.
We can't generate the rules per the machine. A single rootfs needs to support multiple machine configurations.
There was a problem hiding this comment.
We can't generate the rules per the machine. A single rootfs needs to support multiple machine configurations.
Thanks, I understand the concern.
In the current meta-qcom integration, the rules are intentionally machine-specific:
qcom-raw-partitions-udev-rulesis packaged asMACHINE_ARCH.- The generator receives the partition layouts selected by the current
MACHINEthroughQCOM_PARTITION_FILES_SUBDIRandQCOM_PARTITION_FILES_SUBDIR_SPINOR. - The generated rule is installed only in that machine's image.
Therefore, we are not currently treating one rootfs artifact as reusable across multiple MACHINEs.
If the intended requirement is that a generic rootfs must be reusable across multiple MACHINEs, I agree that the rules should be generated from the union of all supported layouts, or from a common approved policy.
Could you confirm that this is the expected image model for meta-qcom?
There was a problem hiding this comment.
The expected model is that the rootfs should be as unified as possible. Other QLI distros (Debian) don't have machine-specific packages, so the generated rules, if they are to be usable, should be generic.
There was a problem hiding this comment.
This will break for the USB or uSD devices attached to the system. However you can use that filter to generate a set of rules from the existing conf files. As we discussed beforehand, skip the occasional one-off cases.
There was a problem hiding this comment.
Thanks, I understand the proposal: use the small filesystem-label filter only at generation time, scan the existing upstream partition conf files, skip the occasional one-off cases, and emit a generic set of explicit PARTNAME rules. This avoids a runtime catch-all that would affect USB or uSD devices.
One concern remains: if a downstream/custom partitions.conf adds a filesystem partition under a new name, generating by subtraction will classify it as raw and disable blkid. This is the unknown/custom-label case I mentioned earlier.
There was a problem hiding this comment.
Let me restate the proposed approach to make sure I understand it correctly:
- Scan all supported in-tree partitions.conf files, rather than layouts selected for one machine.
- Exclude a small fixed set of filesystem-capable partition names.
- Ignore legacy or occasional one-off layouts/labels.
- Deduplicate the remaining labels and emit a generic set of explicit PARTNAME rules.
- Do not use an inverted catch-all rule at udev runtime.
Is this the intended design?
Two details are still unclear:
- Which layouts or labels should be considered one-offs and excluded?
- If a downstream/custom layout adds a filesystem partition with a new name, should it be classified as raw, or should it retain normal blkid probing?
I would like to clarify these points before reworking the generator again, since the latter choice determines whether unknown/custom layouts remain fail-safe.
There was a problem hiding this comment.
You would already have a list of exclusions, like rootfs or esp. If necessary, that list can be extended.
There was a problem hiding this comment.
Thanks, updated. The generator now scans all in-tree partition layouts and uses a small filesystem exclusion list, including rootfs and efi. All other discovered partitions get explicit raw-partition rules.
31bf38d to
fde5dc9
Compare
fde5dc9 to
1b7aeee
Compare
|
Hi Dmitry Baryshkov (@lumag), updated. The generator now produces a single generic rule set without machine-specific layout inputs. All tests pass and the generated rules pass udevadm verify. Could you please take another look? |
1b7aeee to
04edea5
Compare
|
Hi Dmitry Baryshkov (@lumag), I updated the PR based on your feedback. The generator now scans all in-tree All 130 tests pass, and the generated rules pass |
Generate udev rules from the union of all in-tree partition layouts by default, while allowing repeatable -i options for selected layouts. Keep a small reviewed list of filesystem partition names and emit exact PARTNAME matches for the remaining labels. Names outside the supplied layouts are not emitted, so unknown downstream partitions retain normal blkid probing. This keeps the generated rules machine-independent and avoids a runtime catch-all rule. Signed-off-by: Wenwen Fu <wenwfu@qti.qualcomm.com>
04edea5 to
573879a
Compare
Summary
Add a
gen_udev_rulessubcommand that generates machine-specific udevrules for reviewed Qualcomm raw GPT partitions.
The generator:
labels are present
Example:
qcom-ptool gen_udev_rules \ -i platforms/qcs6490-rb3gen2/ufs/partitions.conf \ -o 55-qcom-raw-partitions-noblkid.rulesMotivation
This functionality was originally implemented in
qualcomm-linux/meta-qcom#2865.
Move the generic generator, reviewed policy, and udev template into
qcom-ptool so they can be reused outside the Yocto layer. The meta-qcom
change will be updated to invoke this subcommand and retain only the
machine-specific integration and packaging.
Unknown or custom partition labels continue through the normal systemd
blkid probing path.