Re: [pve-devel] [PATCH installer 4/4] fetch-answer: use partition label from fetch config instead of hardcoded

[Aaron Lauterer <a.lauterer@proxmox.com>]

On  2024-11-08  10:28, Christoph Heiss wrote:
> Thanks for the review!
> 
> On Thu, Nov 07, 2024 at 04:28:25PM +0100, Aaron Lauterer wrote:
>> only small nits inline
>>
>> Tested-By: Aaron Lauterer <a.lauterer@proxmox.com>
>> Reviewed-By: Aaron Lauterer <a.lauterer@proxmox.com>
>>
>> On  2024-10-18  13:59, Christoph Heiss wrote:
>>> [..]
>>> diff --git a/proxmox-fetch-answer/src/fetch_plugins/partition.rs b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
>>> index cbfe2d5..c68dc59 100644
>>> --- a/proxmox-fetch-answer/src/fetch_plugins/partition.rs
>>> +++ b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
>>> @@ -9,17 +9,16 @@ use std::{
>>>    static ANSWER_FILE: &str = "answer.toml";
>>>    static ANSWER_MP: &str = "/mnt/answer";
>>>    // FAT can only handle 11 characters, so shorten Automated Installer Source to AIS
>>
>> this comment is now dangling. we could move that to the definition of the
>> default value for the new parameter
> 
> Yep, good catch!
> 
> Also, while reading this, seems nowhere is checked whether the
> user-supplied value is at max 11 characters long -- I'll add that for v2
> too.

Too be honest, do we need to do that? For the default it is important, 
but users can use other FS that might allow for longer labels and if the 
kernel of the install live system can handle it, all is good.
> 
>>> [..]
>>> @@ -74,14 +73,14 @@ fn scan_partlabels(partlabel: &str, search_path: &str) -> Result<PathBuf> {
>>>        bail!("Could not find partition for label '{partlabel}'");
>>>    }
>>> -/// Will search and mount a partition/FS labeled PARTLABEL (proxmox-ais) in lower or uppercase
>>> -/// to ANSWER_MP
>>> -fn mount_proxmoxinst_part() -> Result<String> {
>>> +/// Will search and mount a partition/FS labeled labeled `part_label` in lower or uppercase to
>>
>> one `labeled` too much :)
> 
> Thx!



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel