Skip to content

blockdev: use blkid instead of lsblk for querying all filesystems#813

Merged
jlebon merged 1 commit intocoreos:mainfrom
nikita-dubrovskii:issue-1105
Apr 22, 2022
Merged

blockdev: use blkid instead of lsblk for querying all filesystems#813
jlebon merged 1 commit intocoreos:mainfrom
nikita-dubrovskii:issue-1105

Conversation

@nikita-dubrovskii
Copy link
Contributor

blkid usage may help with coreos/fedora-coreos-tracker#1105 , as it returns proper LABEL when lsblk doesn't

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

One optional comment, but LGTM overall! For the commit message, can you add the rationale in the body, and for the commit title, use something like blockdev: use blkid instead of lsblk for querying all filesystems?

@nikita-dubrovskii nikita-dubrovskii force-pushed the issue-1105 branch 4 times, most recently from b8c7de7 to 17f9769 Compare April 22, 2022 10:53
@nikita-dubrovskii nikita-dubrovskii changed the title use blkid for getting all filesystems blockdev: use blkid instead of lsblk for querying all filesystems Apr 22, 2022
`blkid` can return the filesystem label when `lsblk` doesn't. This may
help with coreos/fedora-coreos-tracker#1105.
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

LGTM! I just added the same rationale in the first comment to the commit message.

);
}

#[test]
Copy link
Member

Choose a reason for hiding this comment

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

Awesome, thanks for adding these tests!

@jlebon jlebon enabled auto-merge April 22, 2022 13:06
@jlebon jlebon merged commit 9f1e2c4 into coreos:main Apr 22, 2022
@nikita-dubrovskii nikita-dubrovskii deleted the issue-1105 branch April 22, 2022 18:23
jlebon added a commit to jlebon/coreos-installer that referenced this pull request Jan 19, 2023
By default, `blkid` will return cached data, which we don't want because
it might be stale. Add `-p` to make sure we always bypass the cache. We
originally used `lsblk` here which uses the udev database, but this was
changed in coreos#813 because
it failed to return the filesystem label in some instances. It might be
worth revisiting this at some point and find out if we were just missing
a `udevadm settle` somewhere.

See also: coreos/fedora-coreos-config#2181 (comment)
jlebon added a commit to jlebon/coreos-installer that referenced this pull request Jan 19, 2023
By default, `blkid` will return cached data, which we don't want because
it might be stale. Add `-p` to make sure we always bypass the cache. We
originally used `lsblk` here which uses the udev database, but this was
changed in coreos#813 because
it failed to return the filesystem label in some instances. It might be
worth revisiting this at some point and find out if we were just missing
a `udevadm settle` somewhere.

See also: coreos/fedora-coreos-config#2181 (comment)
jlebon added a commit to jlebon/coreos-installer that referenced this pull request Jan 20, 2023
By default, `blkid` will return cached data, which we don't want because
it might be stale. We need to use`-p` to make sure it directly probes
the block devices and bypasses the cache.

With `-p`, `blkid` requires passing the devices directly. Call it once
to gather the list of devices (we trust the cache enough for this) and
then again with `-p`.

We originally used `lsblk` here which uses the udev database, but this
was changed in coreos#813
because it failed to return the filesystem label in some instances. It
might be worth revisiting this at some point and find out if we were
just missing a `udevadm settle` somewhere.

See also: coreos/fedora-coreos-config#2181 (comment)
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.

2 participants