[pve-devel] [PATCH 1/2] assistant: keep prepared iso bootable on uefi with flash drives

[pve-devel] [PATCH 1/2] assistant: keep prepared iso bootable on uefi with flash drives

[Aaron Lauterer]

By mapping files into the ISO, the UUID for the partitions change as
they depend on the timestamp. The result is, that grub cannot find its
partition anymore and the user ends up on the grub shell.

This only happens when booting from a blockdev in UEFI mode. E.g. a USB
flash drive. Alternatively one can `dd` the ISO to a small (2GiB) VM
disk and mark it as the first boot device.

Booting in legacy mode or via CDROM (e.g. pass through via IPMI), it
worked.

Xorriso can report the commands needed to recreate the source ISO. The
'-volume_date uuid' is the one needed to override the same UUIDs. We
therefore read it first from the source iso and pass it as parameter
whenever we inject a file into the iso.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 proxmox-auto-install-assistant/src/main.rs | 44 ++++++++++++++++++++--
 1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index 0debd29..e9213f7 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -276,6 +276,7 @@ fn show_system_info(_args: &CommandSystemInfo) -> Result<()> {
 
 fn prepare_iso(args: &CommandPrepareISO) -> Result<()> {
     check_prepare_requirements(args)?;
+    let uuid = get_iso_uuid(&args.input)?;
 
     if args.fetch_from == FetchAnswerFrom::Iso && args.answer_file.is_none() {
         bail!("Missing path to the answer file required for the fetch-from 'iso' mode.");
@@ -331,10 +332,15 @@ fn prepare_iso(args: &CommandPrepareISO) -> Result<()> {
     instmode_file_tmp.push("auto-installer-mode.toml");
     fs::write(&instmode_file_tmp, toml::to_string_pretty(&config)?)?;
 
-    inject_file_to_iso(&tmp_iso, &instmode_file_tmp, "/auto-installer-mode.toml")?;
+    inject_file_to_iso(
+        &tmp_iso,
+        &instmode_file_tmp,
+        "/auto-installer-mode.toml",
+        &uuid,
+    )?;
 
     if let Some(answer_file) = &args.answer_file {
-        inject_file_to_iso(&tmp_iso, answer_file, "/answer.toml")?;
+        inject_file_to_iso(&tmp_iso, answer_file, "/answer.toml", &uuid)?;
     }
 
     println!("Moving prepared ISO to target location...");
@@ -371,11 +377,14 @@ fn final_iso_location(args: &CommandPrepareISO) -> PathBuf {
     target.to_path_buf()
 }
 
-fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str) -> Result<()> {
+fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str, uuid: &String) -> Result<()> {
     let result = Command::new("xorriso")
         .arg("--boot_image")
         .arg("any")
         .arg("keep")
+        .arg("-volume_date")
+        .arg("uuid")
+        .arg(uuid)
         .arg("-dev")
         .arg(iso)
         .arg("-map")
@@ -391,6 +400,35 @@ fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str) -> Result<(
     Ok(())
 }
 
+fn get_iso_uuid(iso: &PathBuf) -> Result<String> {
+    let result = Command::new("xorriso")
+        .arg("-dev")
+        .arg(iso)
+        .arg("-report_system_area")
+        .arg("cmd")
+        .output()?;
+    if !result.status.success() {
+        bail!(
+            "Error determining the UUID of the source ISO: {}",
+            String::from_utf8_lossy(&result.stderr)
+        );
+    }
+    let mut uuid = String::new();
+    for line in String::from_utf8(result.stdout)?.lines() {
+        if line.starts_with("-volume_date uuid") {
+            uuid = line
+                .split(' ')
+                .last()
+                .unwrap()
+                .replace('\'', "")
+                .trim()
+                .into();
+            break;
+        }
+    }
+    Ok(uuid)
+}
+
 fn get_disks() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
     let unwantend_block_devs = vec![
         "ram[0-9]*",
-- 
2.39.2



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

[pve-devel] [PATCH 2/2] assistant: use single dash for xorriso parameter

[Aaron Lauterer]

while it works with two, one is what is shown in the man page and what
we already use for the other paramters.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 proxmox-auto-install-assistant/src/main.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index e9213f7..afbac85 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -379,7 +379,7 @@ fn final_iso_location(args: &CommandPrepareISO) -> PathBuf {
 
 fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str, uuid: &String) -> Result<()> {
     let result = Command::new("xorriso")
-        .arg("--boot_image")
+        .arg("-boot_image")
         .arg("any")
         .arg("keep")
         .arg("-volume_date")
-- 
2.39.2



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

Re: [pve-devel] [PATCH 1/2] assistant: keep prepared iso bootable on uefi with flash drives

[Stefan Sterz]

On Tue Apr 30, 2024 at 10:54 AM CEST, Aaron Lauterer wrote:
> By mapping files into the ISO, the UUID for the partitions change as
> they depend on the timestamp. The result is, that grub cannot find its
> partition anymore and the user ends up on the grub shell.
>
> This only happens when booting from a blockdev in UEFI mode. E.g. a USB
> flash drive. Alternatively one can `dd` the ISO to a small (2GiB) VM
> disk and mark it as the first boot device.
>
> Booting in legacy mode or via CDROM (e.g. pass through via IPMI), it
> worked.
>
> Xorriso can report the commands needed to recreate the source ISO. The
> '-volume_date uuid' is the one needed to override the same UUIDs. We
> therefore read it first from the source iso and pass it as parameter
> whenever we inject a file into the iso.
>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  proxmox-auto-install-assistant/src/main.rs | 44 ++++++++++++++++++++--
>  1 file changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
> index 0debd29..e9213f7 100644
> --- a/proxmox-auto-install-assistant/src/main.rs
> +++ b/proxmox-auto-install-assistant/src/main.rs
> @@ -276,6 +276,7 @@ fn show_system_info(_args: &CommandSystemInfo) -> Result<()> {
>
>  fn prepare_iso(args: &CommandPrepareISO) -> Result<()> {
>      check_prepare_requirements(args)?;
> +    let uuid = get_iso_uuid(&args.input)?;
>
>      if args.fetch_from == FetchAnswerFrom::Iso && args.answer_file.is_none() {
>          bail!("Missing path to the answer file required for the fetch-from 'iso' mode.");
> @@ -331,10 +332,15 @@ fn prepare_iso(args: &CommandPrepareISO) -> Result<()> {
>      instmode_file_tmp.push("auto-installer-mode.toml");
>      fs::write(&instmode_file_tmp, toml::to_string_pretty(&config)?)?;
>
> -    inject_file_to_iso(&tmp_iso, &instmode_file_tmp, "/auto-installer-mode.toml")?;
> +    inject_file_to_iso(
> +        &tmp_iso,
> +        &instmode_file_tmp,
> +        "/auto-installer-mode.toml",
> +        &uuid,
> +    )?;
>
>      if let Some(answer_file) = &args.answer_file {
> -        inject_file_to_iso(&tmp_iso, answer_file, "/answer.toml")?;
> +        inject_file_to_iso(&tmp_iso, answer_file, "/answer.toml", &uuid)?;
>      }
>
>      println!("Moving prepared ISO to target location...");
> @@ -371,11 +377,14 @@ fn final_iso_location(args: &CommandPrepareISO) -> PathBuf {
>      target.to_path_buf()
>  }
>
> -fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str) -> Result<()> {
> +fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str, uuid: &String) -> Result<()> {
>      let result = Command::new("xorriso")
>          .arg("--boot_image")
>          .arg("any")
>          .arg("keep")
> +        .arg("-volume_date")
> +        .arg("uuid")
> +        .arg(uuid)
>          .arg("-dev")
>          .arg(iso)
>          .arg("-map")
> @@ -391,6 +400,35 @@ fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str) -> Result<(
>      Ok(())
>  }
>
> +fn get_iso_uuid(iso: &PathBuf) -> Result<String> {
> +    let result = Command::new("xorriso")
> +        .arg("-dev")
> +        .arg(iso)
> +        .arg("-report_system_area")
> +        .arg("cmd")
> +        .output()?;
> +    if !result.status.success() {
> +        bail!(
> +            "Error determining the UUID of the source ISO: {}",
> +            String::from_utf8_lossy(&result.stderr)
> +        );
> +    }
> +    let mut uuid = String::new();
> +    for line in String::from_utf8(result.stdout)?.lines() {
> +        if line.starts_with("-volume_date uuid") {
> +            uuid = line
> +                .split(' ')
> +                .last()
> +                .unwrap()

nit: while this probably won't happen, if xorriso ever starts returning
nothing to the above command, this unwrap may panic. it might be nice to
do a `ok_or_else(|| bail!("xorisso command behaved unexpectedly"))?` or
similar here.

> +                .replace('\'', "")
> +                .trim()
> +                .into();
> +            break;
> +        }
> +    }
> +    Ok(uuid)
> +}
> +
>  fn get_disks() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
>      let unwantend_block_devs = vec![
>          "ram[0-9]*",



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

Re: [pve-devel] [PATCH 1/2] assistant: keep prepared iso bootable on uefi with flash drives

[Stoiko Ivanov]

gave this and the next patch a spin on 2 test-servers and one VM, where I
could reproduce the issue yesterday - 
* execsnoop-bpfcc says the patch does what it says on the tin
* automated installs worked.

tiny nit: subject prefix and a cover-letter would have helped

the following goes for both patches:
Reviewed-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Tested-by: Stoiko Ivanov <s.ivanov@proxmox.com>

On Tue, 30 Apr 2024 10:54:33 +0200
Aaron Lauterer <a.lauterer@proxmox.com> wrote:

> By mapping files into the ISO, the UUID for the partitions change as
> they depend on the timestamp. The result is, that grub cannot find its
> partition anymore and the user ends up on the grub shell.
> 
> This only happens when booting from a blockdev in UEFI mode. E.g. a USB
> flash drive. Alternatively one can `dd` the ISO to a small (2GiB) VM
> disk and mark it as the first boot device.
> 
> Booting in legacy mode or via CDROM (e.g. pass through via IPMI), it
> worked.
> 
> Xorriso can report the commands needed to recreate the source ISO. The
> '-volume_date uuid' is the one needed to override the same UUIDs. We
> therefore read it first from the source iso and pass it as parameter
> whenever we inject a file into the iso.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  proxmox-auto-install-assistant/src/main.rs | 44 ++++++++++++++++++++--
>  1 file changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
> index 0debd29..e9213f7 100644
> --- a/proxmox-auto-install-assistant/src/main.rs
> +++ b/proxmox-auto-install-assistant/src/main.rs
> @@ -276,6 +276,7 @@ fn show_system_info(_args: &CommandSystemInfo) -> Result<()> {
>  
>  fn prepare_iso(args: &CommandPrepareISO) -> Result<()> {
>      check_prepare_requirements(args)?;
> +    let uuid = get_iso_uuid(&args.input)?;
>  
>      if args.fetch_from == FetchAnswerFrom::Iso && args.answer_file.is_none() {
>          bail!("Missing path to the answer file required for the fetch-from 'iso' mode.");
> @@ -331,10 +332,15 @@ fn prepare_iso(args: &CommandPrepareISO) -> Result<()> {
>      instmode_file_tmp.push("auto-installer-mode.toml");
>      fs::write(&instmode_file_tmp, toml::to_string_pretty(&config)?)?;
>  
> -    inject_file_to_iso(&tmp_iso, &instmode_file_tmp, "/auto-installer-mode.toml")?;
> +    inject_file_to_iso(
> +        &tmp_iso,
> +        &instmode_file_tmp,
> +        "/auto-installer-mode.toml",
> +        &uuid,
> +    )?;
>  
>      if let Some(answer_file) = &args.answer_file {
> -        inject_file_to_iso(&tmp_iso, answer_file, "/answer.toml")?;
> +        inject_file_to_iso(&tmp_iso, answer_file, "/answer.toml", &uuid)?;
>      }
>  
>      println!("Moving prepared ISO to target location...");
> @@ -371,11 +377,14 @@ fn final_iso_location(args: &CommandPrepareISO) -> PathBuf {
>      target.to_path_buf()
>  }
>  
> -fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str) -> Result<()> {
> +fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str, uuid: &String) -> Result<()> {
>      let result = Command::new("xorriso")
>          .arg("--boot_image")
>          .arg("any")
>          .arg("keep")
> +        .arg("-volume_date")
> +        .arg("uuid")
> +        .arg(uuid)
>          .arg("-dev")
>          .arg(iso)
>          .arg("-map")
> @@ -391,6 +400,35 @@ fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str) -> Result<(
>      Ok(())
>  }
>  
> +fn get_iso_uuid(iso: &PathBuf) -> Result<String> {
> +    let result = Command::new("xorriso")
> +        .arg("-dev")
> +        .arg(iso)
> +        .arg("-report_system_area")
> +        .arg("cmd")
> +        .output()?;
> +    if !result.status.success() {
> +        bail!(
> +            "Error determining the UUID of the source ISO: {}",
> +            String::from_utf8_lossy(&result.stderr)
> +        );
> +    }
> +    let mut uuid = String::new();
> +    for line in String::from_utf8(result.stdout)?.lines() {
> +        if line.starts_with("-volume_date uuid") {
> +            uuid = line
> +                .split(' ')
> +                .last()
> +                .unwrap()
> +                .replace('\'', "")
> +                .trim()
> +                .into();
> +            break;
> +        }
> +    }
> +    Ok(uuid)
> +}
> +
>  fn get_disks() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
>      let unwantend_block_devs = vec![
>          "ram[0-9]*",



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

Re: [pve-devel] [PATCH 1/2] assistant: keep prepared iso bootable on uefi with flash drives

[Aaron Lauterer]

On  2024-04-30  11:41, Stefan Sterz wrote:
> On Tue Apr 30, 2024 at 10:54 AM CEST, Aaron Lauterer wrote:
>> By mapping files into the ISO, the UUID for the partitions change as
>> they depend on the timestamp. The result is, that grub cannot find its
>> partition anymore and the user ends up on the grub shell.
>>
>> This only happens when booting from a blockdev in UEFI mode. E.g. a USB
>> flash drive. Alternatively one can `dd` the ISO to a small (2GiB) VM
>> disk and mark it as the first boot device.
>>
>> Booting in legacy mode or via CDROM (e.g. pass through via IPMI), it
>> worked.
>>
>> Xorriso can report the commands needed to recreate the source ISO. The
>> '-volume_date uuid' is the one needed to override the same UUIDs. We
>> therefore read it first from the source iso and pass it as parameter
>> whenever we inject a file into the iso.
>>
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> ---
>>   proxmox-auto-install-assistant/src/main.rs | 44 ++++++++++++++++++++--
>>   1 file changed, 41 insertions(+), 3 deletions(-)
>>
>>
>> +fn get_iso_uuid(iso: &PathBuf) -> Result<String> {
>> +    let result = Command::new("xorriso")
>> +        .arg("-dev")
>> +        .arg(iso)
>> +        .arg("-report_system_area")
>> +        .arg("cmd")
>> +        .output()?;
>> +    if !result.status.success() {
>> +        bail!(
>> +            "Error determining the UUID of the source ISO: {}",
>> +            String::from_utf8_lossy(&result.stderr)
>> +        );
>> +    }
>> +    let mut uuid = String::new();
>> +    for line in String::from_utf8(result.stdout)?.lines() {
>> +        if line.starts_with("-volume_date uuid") {
>> +            uuid = line
>> +                .split(' ')
>> +                .last()
>> +                .unwrap()
> 
> nit: while this probably won't happen, if xorriso ever starts returning
> nothing to the above command, this unwrap may panic. it might be nice to
> do a `ok_or_else(|| bail!("xorisso command behaved unexpectedly"))?` or
> similar here.

thanks. will send a v2
> 
>> +                .replace('\'', "")
>> +                .trim()
>> +                .into();
>> +            break;
>> +        }
>> +    }
>> +    Ok(uuid)
>> +}
>> +
>>   fn get_disks() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
>>       let unwantend_block_devs = vec![
>>           "ram[0-9]*",
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 


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