cosa diff: add support for diffing metal images #4226
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature for diffing metal images by utilizing guestfs and multiprocessing to efficiently compare images. However, there are critical issues with process lifecycle management and cleanup that could lead to resource leaks or crashes. Addressing these issues is important for the stability of this new feature.
jbtrystram
force-pushed
the
diff-metal
branch
3 times, most recently
from
3d0140e to
1a0a833
Compare
This add two flags: `--metal` and `metal-part-table`. The former will mount the disk partitions using guestfs tools to the tmp-diff directory, then call git diff over the two mounted filesystems. It requires multithreading because the FUSE mount call is blocking so we mount the disks in two libreguest VMs that run in separate threads. A simpler approach would have been to copy all the content to the tmp-diff folder but that would copy a lot of data just for diffing. It may be faster though. The second flag `--metal-part-table` will simply show the partition table for the two images.
| for i, d in enumerate([mount_dir_from, mount_dir_to]): | ||
| p = p_from if i == 0 else p_to |
There was a problem hiding this comment.
Minor/optional: I think a cleaner way to do this is:
for (p, d) in [(p_from, mount_dir_from), (p_to, mount_dir_to)]:
| def diff_metal_partitions(diff_from, diff_to): | ||
| metal_from = get_metal_path(diff_from) | ||
| metal_to = get_metal_path(diff_to) | ||
| diff_cmd_outputs(['sgdisk', '-p'], metal_from, metal_to) |
There was a problem hiding this comment.
I think we should use sfdisk here instead.
| # Mount the disks in the guestfs VM | ||
| root = g.findfs_label("root") | ||
| g.mount_ro(root, "/") | ||
| boot = g.findfs_label("boot") | ||
| g.mount_ro(boot, "/boot") | ||
| efi = g.findfs_label("EFI-SYSTEM") | ||
| g.mount_ro(efi, "/boot/efi") |
There was a problem hiding this comment.
Totally fine to start, though this will need adjustments to make it usable on other arches.
And we should definitely check other arches as part of coreos/fedora-coreos-tracker#1827.
This add two flags:
--metalandmetal-part-table.The former will mount the disk partitions using guestfs tools to the tmp-diff directory, then call git diff over the two mounted filesystems. It requires multithreading because the FUSE mount call is blocking so we mount the disks in two libreguest VMs that run in separate threads.
A simpler approach would have been to copy all the content to the tmp-diff folder but that would copy a lot of data just for diffing. It may be faster though.
The second flag
--metal-part-tablewill simply show the partition table for the two images.