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

Get partition type for unmounted partitions #305

Merged
merged 2 commits into from
Mar 24, 2022
Merged

Get partition type for unmounted partitions #305

merged 2 commits into from
Mar 24, 2022

Conversation

Itxaka
Copy link
Contributor

@Itxaka Itxaka commented Mar 18, 2022

If a partition is unmounted our current way of getting the partition
type wont work, as it only checks for mounted partitions.

This patch makes it check in udev if the partition type is empty

This requires #304 as that brings the udevInfoPartition

Signed-off-by: Itxaka igarcia@suse.com

@Itxaka Itxaka changed the title Get partition labels from udev Get partition type for unmounted partitions Mar 18, 2022
@Itxaka
Copy link
Contributor Author

Itxaka commented Mar 18, 2022

will rebase if #304 gets in :)

@Itxaka
Copy link
Contributor Author

Itxaka commented Mar 18, 2022

rebased!

@Itxaka
Copy link
Contributor Author

Itxaka commented Mar 24, 2022

@fromanirh any updates on this? thanks!

Copy link
Owner

@jaypipes jaypipes left a comment

Choose a reason for hiding this comment

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

@Itxaka awesome stuff, thank you so much! Just a few little nits inline to address, otherwise looks great :)

// Check empty fs if not found
fs = diskPartTypeUdev(paths, "sda", "sda2")
if fs != util.UNKNOWN {
t.Fatalf("Got label %s, but expected %s label", fs, util.UNKNOWN)
Copy link
Owner

Choose a reason for hiding this comment

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

s/label/partition type/g :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

@@ -237,6 +240,18 @@ func diskPartLabel(paths *linuxpath.Paths, disk string, partition string) string
return util.UNKNOWN
}

func diskPartTypeUdev(paths *linuxpath.Paths, disk string, partition string) string {
Copy link
Owner

Choose a reason for hiding this comment

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

Please add a quick docstring indicating this function is used as a fallback when partitionInfo() fails to determine a partition type, which happens when the partition is not mounted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

_ = os.Mkdir(filepath.Join(paths.SysBlock, "sda", "sda1"), 0755)
_ = ioutil.WriteFile(filepath.Join(paths.SysBlock, "sda", "sda1", "dev"), []byte("259:0\n"), 0644)
_ = ioutil.WriteFile(filepath.Join(paths.RunUdevData, "b259:0"), []byte(fmt.Sprintf("E:ID_FS_TYPE=%s\n", fsType)), 0644)
fs := diskPartTypeUdev(paths, "sda", "sda1")
Copy link
Owner

Choose a reason for hiding this comment

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

consider renaming fsType to expectedPartType and fs variables to pt to align with the "partition type" terminology (and yes, I realize udev uses "FS_TYPE" term :) )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

@ffromani
Copy link
Collaborator

I see @jaypipes is already on it; I will have a look myself today/tomorrow as well

Copy link
Owner

@jaypipes jaypipes left a comment

Choose a reason for hiding this comment

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

rock on :) thanks @Itxaka!

Itxaka added 2 commits March 24, 2022 14:58
If a partition is unmounted our current way of getting the partition
type wont work, as it only checks for mounted partitions.

This patch makes it check in udev if the partition type is empty

Signed-off-by: Itxaka <igarcia@suse.com>
Signed-off-by: Itxaka <igarcia@suse.com>
@jaypipes jaypipes merged commit 95585aa into jaypipes:main Mar 24, 2022
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