-
Notifications
You must be signed in to change notification settings - Fork 198
Set partition UUID from VolumeSerialNumber #412
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
Conversation
baier233
commented
Apr 22, 2025
Signed-off-by: Baier <68586144+baier233@users.noreply.github.com>
Name: strings.TrimSpace(*logicaldisk.Caption), | ||
Label: strings.TrimSpace(*logicaldisk.Caption), | ||
SizeBytes: *logicaldisk.Size, | ||
MountPoint: *logicaldisk.DeviceID, | ||
Type: *diskpartition.Type, | ||
IsReadOnly: toReadOnly(*diskpartition.Access), | ||
UUID: "", | ||
UUID: *logicaldisk.VolumeSerialNumber, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@baier233 I think the UUID
field should only contain UUIDs, so I'm not sure that VolumeSerialNumber is appropriate for that field of Partition. What about either adding a new SerialNumber field to the Partition struct or using the existing FilesystemLabel
field instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fully agree that Volume Serial Numbers are not RFC-compliant UUIDs.
I noticed that the Linux implementation of ghw
obtains partition UUIDs through the ID_PART_ENTRY_UUID
attribute from udev
. This means that for non-GPT partitions, the UUID field will not be RFC-compliant (see systemd PR #3293).
As documented in Debian’s Part-UUID:
"FAT and NTFS filesystems have shorter names (e.g., sda6 and sdb1), but they are still listed by UUID."
Therefore, I don't believe the UUID field must strictly contain RFC-compliant value.
To maintain backward compatibility with existing applications that expect partition identifiers in the UUID
field, I believe it would be appropriate to use VolumeSerialNumber
in this field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@baier233 OK, makes sense to me :) Could you add a note to the Partition.UUID field's docstring that mentions this may not be a UUID format for Windows partitions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@baier233 OK, makes sense to me :) Could you add a note to the Partition.UUID field's docstring that mentions this may not be a UUID format for Windows partitions?
Sure,I've also updated the README description for ghw.Partition.UUID
to reflect the Windows support:
Updated documentation to clarify that the UUID field may contain non-standard UUID formats on Windows systems. Signed-off-by: Baier <68586144+baier233@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome, thank you @baier233 ! :)