Created attachment 406384 [details] skipcopy.patch to protect persistent overlay and home.img Description of problem: The --skipcopy option was implemented to aid testing and boot configuration file recovery (http://article.gmane.org/gmane.linux.redhat.fedora.livecd/2682). I've found that it will overwrite both an existing persistent overlay and home.img, if they exist. This is unintentional destructive behavior. While it may seem like one wouldn't ask for a new overlay or home folder AND skipcopy, during the testing of scripts this is a reasonable scenario. Version-Release number of selected component (if applicable): current tools_livecd-iso-to-disk.sh 3a4726dd3ffa046d56eae46d657213e635141024 How reproducible: Always. With a LiveOS installed device, /dev/sdb1, that contains a persistent overlay and home.img files, both 100 MB, Steps to Reproduce: 1. With a LiveOS installed device, /dev/sdb1, that contains a persistent overlay and home.img files, both 100 MB, 2. livecd-iso-to-disk --overlay-size-mb 50 --home-size-mb 50 --delete-home --unencrypted-home --skipcopy /path/to/isofile /dev/sdb1 Actual results: Notice that your /LiveOS/overlay- and home.img files are now 50 MB in size instead of the original 100 MB Expected results: No change to these files. Additional info: The attached skipcopy.patch adds an additional test for the skipcopy flag and moves the sediting of the BOOTCONFIG files outside of the test block.
Created attachment 407172 [details] 2ndskipcopy.patch to protect persistent overlay and home.img For the script testing use case, format and resetmbr also need to be prevented when skipcopy is requested.
This package has changed ownership in the Fedora Package Database. Reassigning to the new owner of this component.
From a review posted on the livecd list: I took a look at this and I think for it's purpose the intent of your changes seem reasonable. However, I think the man page entry should be expanded for --skipcopy to say precisely what the effects of using it are and a bit about what it is for. I looked through what you did change and for the most part it looked OK. I wouldn't be likely to notice an omition though, as I don't know the code well enough. One part that seemed odd was that part of the overlay code is being skipped now, but the test for the second part that you want done checks for -s $USBMNT/$LIVEOS/$OVERFILE instead of "$overlaysizemb" -gt 0. That seems odd to me. I would have expected you to have used -z "$skipcopy" as a nested if, if you wanted to skip only part of the stuff in that block. But maybe I missed something.
Created attachment 425374 [details] 3rdskipcopy.patch to protect persistent overlay and home.img (In reply to comment #3) > ... However, I think the man page entry should be expanded for > --skipcopy to say precisely what the effects of using it are and a bit about > what it is for. I've proposed a new help file in https://bugzilla.redhat.com/attachment.cgi?id=406158&action=edit and also on the wiki at https://fedoraproject.org/wiki/Livecd-iso-to-disk.pod for collaborative editing. > ... > > One part that seemed odd was that part of the overlay code is being skipped > now, but the test for the second part that you want done checks for > -s $USBMNT/$LIVEOS/$OVERFILE instead of "$overlaysizemb" -gt 0. That seems > odd to me. I would have expected you to have used -z "$skipcopy" as a nested > if, if you wanted to skip only part of the stuff in that block. But maybe > I missed something. You are right. These lines are out of context for this patch. (They crept in from the --copy-overlay option proposed in https://bugzilla.redhat.com/attachment.cgi?id=406157&action=edit .)
I already pushed out 032 because there were already a lot of changes that we should get better testing on, but it looks like there are also a fair amount of patches still queued up and we should get through those and get another rawhide release out.
Created attachment 431319 [details] 4thskipcopy.patch to protect overlay and home.img (allow reset-mbr) An updated patch to allow explicit --reset-mbr with --skipcopy. The --reset-mbr option seems to fit with the "repairing of boot configuration" use of --skipcopy, and needn't be called in the "testing" use.
Created attachment 431468 [details] update livecd-iso-to-disk.pod for --skipcopy patch Attaching a patch that updates the .pod file for the on-line manual.
Created attachment 431955 [details] 5thskipcopy.patch to protect overlay and home and restore boot configuration files I've reconsidered the removal of the special editing block for $BOOTCONFIG's that Bruno questioned in comment 3 and I agreed with at the bottom of comment 4: The 'boot config file repair' function of --skipcopy requires this. So this patch restores the test for an overlay on every invocation.
I don't think the review flag is used for requesting this kind of review. In any case I agreed to review this one more carefully and if it still seems controversial, I post a description of the change and get an ack for that (without necessarily another careful review) on the livecd list. My target is to get this done sometime over the weekend.
Created attachment 434357 [details] skipcopy.patch to protect overlay and home while testing or restoring boot configuration files The $OVERLAY variable must be set if the --skipcopy option is to be used with the if [ -s $USBMNT/$LIVEOS/$OVERFILE ]; then logic for editing the bootconfig files. Confusion on which test to use may be due to the dual use of --skipcopy: 1. Testing the livecd-iso-to-disk script (with variations on installation options, for example), or 2. Repairing boot configuration files on an already-installed LiveUSB image. For use #2, one would test for the existence of the overlay file on the target device, rather than the existence of a request for a new overlay (because $overlaysizemb is set only by the command line option --overlay-size-mb <size>, which is the way one requests a new, persistent overlay). So to include an existing overlay reference on an image needing a new boot configuration file, one could simply invoke the --skipcopy option---without needing also to include a, perhaps counterintuitive, request for a new, persistent overlay. This, of course, means that the variable $OVERFILE must be set without the new overlay request (which the attached patch provides). In the standard installation case, if an overlay was requested, the overlay file will have just been created with dd before the $OVERFILE test is performed.
This bug appears to have been reported against 'rawhide' during the Fedora 14 development cycle. Changing version to '14'. More information and reason for this action is here: http://fedoraproject.org/wiki/BugZappers/HouseKeeping
I committed a modified version of this. If the overlay size isn't given, nothing happens to any overlay. If you provide an overlay size and do a skipcopy, the boot config stuff will get modified, but reserving the space with dd will get skipped.
livecd-tools-034-1.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/livecd-tools-034-1.fc14
livecd-tools-034-2.fc14 has been pushed to the Fedora 14 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update livecd-tools'. You can provide feedback for this update here: https://admin.fedoraproject.org/updates/livecd-tools-034-2.fc14
livecd-tools-034-7.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/livecd-tools-034-7.fc14
livecd-tools-034-7.fc14 has been pushed to the Fedora 14 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update livecd-tools'. You can provide feedback for this update here: https://admin.fedoraproject.org/updates/livecd-tools-034-7.fc14
livecd-tools-034-7.fc14 has been pushed to the Fedora 14 stable repository. If problems still persist, please make note of it in this bug report.