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

Allow to create label on vfat type #103

Merged
merged 1 commit into from Nov 7, 2023
Merged

Conversation

aka7
Copy link

@aka7 aka7 commented Nov 2, 2023

Fix label option for mkfs.vfat so that vfat type can also have label. mkfs.vfat does not have -L option, instead has -n.

fixes: #102

@aka7
Copy link
Author

aka7 commented Nov 2, 2023

open to any other suggestion on this.

@ned21
Copy link
Contributor

ned21 commented Nov 2, 2023

yeah, I can see why you have done it this way, a hash like TUNECMDS doesn't make much sense when only one FS type has a different value. The construct you really want is a switch with default option, which I guess should be supported in the version of perl on EL6? But constants can't be dynamic...

What I think you want to do here is replace the MKFSLABEL constant with a function which accepts a string of $fstype and then something like this:

for ($fstype) {
    $mkfs_label = '-n' when /^vfat$/;
    default { $mkfslabel = '-L' }
}
return $mkfs_label;

which can be called cleanly and as a near drop-in replacement for MKFSLABEL.
my @opts = exists $self->{label} ? (mkfs_label($self->{type}), $self->{label}):();

Fix label option for mkfs.vfat so that vfat type can also have label.
mkfs.vfat does not have -L option, instead has -n.

fixes: quattor#102
@aka7
Copy link
Author

aka7 commented Nov 2, 2023

yeah, I can see why you have done it this way, a hash like TUNECMDS doesn't make much sense when only one FS type has a different value. The construct you really want is a switch with default option, which I guess should be supported in the version of perl on EL6? But constants can't be dynamic...

What I think you want to do here is replace the MKFSLABEL constant with a function which accepts a string of $fstype and then something like this:

for ($fstype) {
    $mkfs_label = '-n' when /^vfat$/;
    default { $mkfslabel = '-L' }
}
return $mkfs_label;

which can be called cleanly and as a near drop-in replacement for MKFSLABEL. my @opts = exists $self->{label} ? (mkfs_label($self->{type}), $self->{label}):();

thanks, but I think what I've done now seems more better approach? Credit Trevor Carden.

@jrha jrha added this to the 23.10 milestone Nov 7, 2023
@jrha jrha merged commit 12f73d6 into quattor:master Nov 7, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

unable to give a label to vfat partitions.
4 participants