Fix TOML parsing, QEMU script issues, and SAVF manifest storage#15
Merged
Conversation
The extract_toml_value function now supports both dotted keys (package.id = "...") and section-based TOML format ([package] \n id = "..."), fixing PTF discovery that was returning empty results for valid generated packages.
Removed orphaned closing braces and duplicate poweroff block after EOF terminator in run_persistence_validation. The script now passes bash -n syntax check, allowing the QEMU install/persistence e2e test to run.
The savlib function was creating a tar.gz archive, then overwriting it with manifest text at line 147. Now stores manifest as user.l400.savf.manifest xattr using write_string_attr, preserving the tar archive for RSTLIB.
- Fix test_full_profile.sh: proper xattrs verification, loader mode check, cgroups v2 check - Fix install_linux400.sh: make setup_mega_io conditional for unattended installs - Fix test_e2e_install_qemu.sh: fix redirection issues and WRKSPLF persistence check - Fix backup.rs: verify MEGA mount in savobj(), fix SAVF overwrite (already fixed) - Fix db.rs: add missing DbError variants (CpfFileAlreadyExists, etc.) - Fix ptf.rs: create /var/log/l400 before audit logging, implement file installation in apply - Fix usrprf.rs: show linked Linux account UID instead of object file UID - Fix l400-loader: properly detect xattr support by testing write - Fix l400-ptf-create: record file destinations in manifest [files] section - Fix l400-ptf-server: serve actual PTF package files (tar.gz) - Fix ptf_maintenance.rs: fix extract_toml_value for [package] format, fix check_ptf_status uppercase mismatch, use APYPTF not aptptf - Fix implementation_plan.md: dont claim 100% PTF completion (update to 80%) - Fix libl400/examples: update DbError variant usage
The l400-loader was using xattr::set() but the crate wasn't in Cargo.toml dependencies, causing E0433 compilation error.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
This PR improves Linux/400 maintenance and backup flows by making PTF metadata parsing more flexible, serving/installing more complete PTF packages, and avoiding destructive SAVF “manifest overwrites” by moving manifest storage to xattrs. It also refines e2e scripts (full profile + QEMU install) and updates the implementation plan to reflect remaining Phase 3 work.
Changes:
- Store SAVF manifest metadata as an xattr instead of overwriting the archive file.
- Extend TOML extraction to support both dotted keys and section-based TOML in PTF-related code paths.
- Improve/clean up e2e scripts (xattr checks, QEMU persistence checks, shutdown flow) and adjust Phase 3 status/tasks.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/test/test_full_profile.sh | Updates “full profile” checks (loader full-mode presence, cgroups v2 detection, xattr verification). |
| scripts/test/test_e2e_install_qemu.sh | Tweaks persistence checks and removes duplicate shutdown block. |
| scripts/runtime/install_linux400.sh | Adds an env flag intended to skip mega.io setup for unattended installs. |
| os400-tui/src/screens/ptf_maintenance.rs | Switches to APYPTF command and enhances TOML parsing + audit log matching. |
| libl400/src/usrprf.rs | Reads linked UID from xattr for display and adjusts creation date retrieval. |
| libl400/src/ptf.rs | Enhances TOML parsing, installs PTF files from a [files] manifest section, and ensures audit log directory exists. |
| libl400/src/db.rs | Adjusts DB error variants/messages and fixes PF create “already exists” error. |
| libl400/src/backup.rs | Writes SAVF manifest to xattr and adds a mega.io mount precheck for SAVOBJ. |
| libl400/examples/objects_v1_demo.rs | Updates example to match new “already exists” DB error variant. |
| l400-ptf-server/src/main.rs | Serves full PTF directories as tar.gz (and supports serving tar.gz files directly). |
| l400-ptf-create/src/main.rs | Adds a [files] section to the generated manifest (intended to record destinations). |
| l400-loader/src/main.rs | Makes xattr support detection actually test setting an xattr. |
| l400-loader/Cargo.toml | Adds the xattr dependency used by the loader status probe. |
| docs/plan/implementation_plan.md | Marks Phase 3 as in-progress and notes remaining work for server/package/install details. |
| TEST_FILE=$(mktemp /tmp/l400-xattr-test.XXXXXX) | ||
| if touch "$TEST_FILE" 2>/dev/null && \ | ||
| setfattr -n "user.l400.test" -v "test_value" "$TEST_FILE" 2>/dev/null && \ | ||
| GET_VALUE=$(getfattr -n "user.l400.test" "$TEST_FILE" 2>/dev/null | grep -c "test_value"); then |
Comment on lines
625
to
+633
| setup_mega_io | ||
| configure_installed_system | ||
|
|
||
| # Setup mega.io only if enabled (set SETUP_MEGA_IO=0 for unattended installs) | ||
| if [ "${SETUP_MEGA_IO}" = "1" ]; then | ||
| setup_mega_io | ||
| else | ||
| info "Omitiendo configuración de mega.io (SETUP_MEGA_IO=${SETUP_MEGA_IO})" | ||
| fi |
Comment on lines
+300
to
+306
| linked_uid_str.parse::<u32>().unwrap_or_else(|_| { | ||
| // Fallback to file metadata if parse fails | ||
| let metadata = std::fs::metadata(&path).expect("Failed to get metadata"); | ||
| metadata.uid() | ||
| }) | ||
| } else { | ||
| let metadata = std::fs::metadata(&path).expect("Failed to get metadata"); |
Comment on lines
+205
to
+214
| let src_file = parts[0].trim().trim_matches('"'); | ||
| let dest_path = parts[1].trim().trim_matches('"').trim_matches('"'); | ||
|
|
||
| let src = files_dir.join(src_file); | ||
| if src.exists() { | ||
| let dest = Path::new(dest_path); | ||
| if let Some(parent) = dest.parent() { | ||
| fs::create_dir_all(parent).ok(); | ||
| } | ||
| fs::copy(&src, dest).ok(); |
Comment on lines
+144
to
+164
| // Add [files] section with destinations | ||
| if let Ok(entries) = fs::read_dir(&files_dir) { | ||
| let mut has_files = false; | ||
| for entry in entries.flatten() { | ||
| if entry.path().is_file() { | ||
| if !has_files { | ||
| manifest.push_str("\n[files]\n"); | ||
| has_files = true; | ||
| } | ||
| let file_name = entry | ||
| .path() | ||
| .file_name() | ||
| .unwrap() | ||
| .to_str() | ||
| .unwrap() | ||
| .to_string(); | ||
| // Default destination: /l400/QGPL/ if not specified | ||
| // In a real implementation, this would parse a mapping file | ||
| let dest = format!("/l400/QGPL/{}", file_name); | ||
| manifest.push_str(&format!("\"{}\" = \"{}\"\n", file_name, dest)); | ||
| } |
Comment on lines
+212
to
+214
| fs::create_dir_all(parent).ok(); | ||
| } | ||
| fs::copy(&src, dest).ok(); |
Comment on lines
+116
to
+130
| // Serve the entire directory as a tar archive | ||
| let output = std::process::Command::new("tar") | ||
| .args(["-czf", "-", "-C", ptf_path.to_str().unwrap(), "."]) | ||
| .output() | ||
| .map_err(std::io::Error::other)?; | ||
|
|
||
| if output.status.success() { | ||
| let response = format!( | ||
| "HTTP/1.1 200 OK\r\nContent-Type: application/gzip\r\nContent-Length: {}\r\n\r\n", | ||
| output.stdout.len() | ||
| ); | ||
| stream.write_all(response.as_bytes())?; | ||
| stream.write_all(&output.stdout)?; | ||
| return Ok(()); | ||
| } |
Comment on lines
+116
to
+120
| // Serve the entire directory as a tar archive | ||
| let output = std::process::Command::new("tar") | ||
| .args(["-czf", "-", "-C", ptf_path.to_str().unwrap(), "."]) | ||
| .output() | ||
| .map_err(std::io::Error::other)?; |
| pub const DEFAULT_PF_MEMBER: &str = "PF_MEMBER"; | ||
|
|
||
| #[derive(Error, Debug)] | ||
| #[repr(C)] |
| - `DSPPTF` y support report muestran historial de mantenimiento. | ||
| - Servidor PTF disponible via HTTP para opcion *SERVICE (sin soporte de tapes). | ||
| - Herramienta de compilacion genera paquetes PTF validos con destinos de archivos. | ||
| - Servidor PTF disponible via HTTP para opcion *SERVICE (sin soporte de tapes). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements and fixes across the codebase, focusing on more robust metadata handling for SAVF archives, enhanced TOML parsing for PTF packages, and cleanup of test scripts. The most significant changes are grouped below:
Backup and Metadata Handling:
libl400/src/backup.rs) [1] [2]PTF Package TOML Parsing:
extract_toml_valuefunction now supports extracting values from both dotted TOML keys (e.g.,package.id = "...") and section-based keys (e.g.,[package]\nid = "..."), improving compatibility with different TOML formats. (libl400/src/ptf.rs) [1] [2]Test Script Cleanup:
scripts/test/test_e2e_install_qemu.sh)