Bug 582051 - --skipcopy option in tools_livecd-iso-to-disk destructively overwrites persistent overlay and home.img
Summary: --skipcopy option in tools_livecd-iso-to-disk destructively overwrites persis...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: livecd-tools
Version: 14
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Bruno Wolff III
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-04-14 00:01 UTC by Frederick Grose
Modified: 2010-09-16 03:48 UTC (History)
4 users (show)

Fixed In Version: livecd-tools-034-7.fc14
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-09-16 03:48:31 UTC


Attachments (Terms of Use)
skipcopy.patch to protect persistent overlay and home.img (1.47 KB, patch)
2010-04-14 00:01 UTC, Frederick Grose
no flags Details | Diff
2ndskipcopy.patch to protect persistent overlay and home.img (1.98 KB, patch)
2010-04-16 19:04 UTC, Frederick Grose
no flags Details | Diff
3rdskipcopy.patch to protect persistent overlay and home.img (1.63 KB, patch)
2010-06-19 19:23 UTC, Frederick Grose
no flags Details | Diff
4thskipcopy.patch to protect overlay and home.img (allow reset-mbr) (1.35 KB, patch)
2010-07-13 01:44 UTC, Frederick Grose
no flags Details | Diff
update livecd-iso-to-disk.pod for --skipcopy patch (660 bytes, patch)
2010-07-13 13:47 UTC, Frederick Grose
no flags Details | Diff
5thskipcopy.patch to protect overlay and home and restore boot configuration files (1.66 KB, patch)
2010-07-15 00:42 UTC, Frederick Grose
no flags Details | Diff
skipcopy.patch to protect overlay and home while testing or restoring boot configuration files (1.82 KB, patch)
2010-07-26 08:00 UTC, Frederick Grose
no flags Details | Diff

Description Frederick Grose 2010-04-14 00:01:10 UTC
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.

Comment 1 Frederick Grose 2010-04-16 19:04:36 UTC
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.

Comment 2 Fedora Admin XMLRPC Client 2010-05-07 15:40:53 UTC
This package has changed ownership in the Fedora Package Database.  Reassigning to the new owner of this component.

Comment 3 Bruno Wolff III 2010-06-19 16:36:01 UTC
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.

Comment 4 Frederick Grose 2010-06-19 19:23:45 UTC
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 .)

Comment 5 Bruno Wolff III 2010-06-19 19:39:17 UTC
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.

Comment 6 Frederick Grose 2010-07-13 01:44:19 UTC
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.

Comment 7 Frederick Grose 2010-07-13 13:47:03 UTC
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.

Comment 8 Frederick Grose 2010-07-15 00:42:12 UTC
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.

Comment 9 Bruno Wolff III 2010-07-15 02:25:08 UTC
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.

Comment 10 Frederick Grose 2010-07-26 08:00:53 UTC
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.

Comment 11 Bug Zapper 2010-07-30 11:20:11 UTC
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

Comment 12 Bruno Wolff III 2010-09-11 22:01:54 UTC
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.

Comment 13 Fedora Update System 2010-09-12 04:20:04 UTC
livecd-tools-034-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/livecd-tools-034-1.fc14

Comment 14 Fedora Update System 2010-09-13 18:09:54 UTC
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

Comment 15 Fedora Update System 2010-09-15 03:22:50 UTC
livecd-tools-034-7.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/livecd-tools-034-7.fc14

Comment 16 Fedora Update System 2010-09-15 06:52:09 UTC
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

Comment 17 Fedora Update System 2010-09-16 03:46:51 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.