cros_sdk: Switch use_image off by default
Switch from a default of --use-image to --nouse-image and add an
override when an existing chroot.img is present. This results in the
following rules:
1. New chroots will not have a chroot.img by default unless --use-image
is passed.
2. cros_sdk --replace will replace an image-backed chroot with a plain
chroot unless --use-image is passed.
3. Existing chroots without a chroot.img will continue to work unchanged
even if --use-image is passed (same as today).
4. Existing chroots with a chroot.img will continue to mount the
chroot.img as needed, even if --use-image is not passed (and even if
--nouse-image is passed).
The only rule that leads to potential surprises is #4, since there is
now no way to create a non-image chroot alongside an image-backed
chroot. Doing so already leaves the chroot partially broken, so the
change most likely won't affect any non-accidental uses.
BUG=chromium:1130049
TEST=cros_sdk with combinations of use_image and existing chroots
Change-Id: I9a2b918f58758da6ea7613ada72cbc4a52fb69d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2515959
Reviewed-by: Chris McDonald <cjmcdonald@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Tested-by: Benjamin Gordon <bmgordon@chromium.org>
Commit-Queue: Benjamin Gordon <bmgordon@chromium.org>
diff --git a/scripts/cros_sdk.py b/scripts/cros_sdk.py
index b2ca946..c900294 100644
--- a/scripts/cros_sdk.py
+++ b/scripts/cros_sdk.py
@@ -713,14 +713,14 @@
'--nouse-image',
dest='use_image',
action='store_false',
- default=True,
+ default=False,
help='Do not mount the chroot on a loopback image; '
'instead, create it directly in a directory.')
parser.add_argument(
'--use-image',
dest='use_image',
action='store_true',
- default=True,
+ default=False,
help='Mount the chroot on a loopback image '
'instead of creating it directly in a directory.')
@@ -969,11 +969,21 @@
if options.working_dir is not None and not os.path.isabs(options.working_dir):
options.working_dir = path_util.ToChrootPath(options.working_dir)
- # Discern if we need to create the chroot.
+ # If there is an existing chroot image and we're not removing it then force
+ # use_image on. This ensures that people don't have to remember to pass
+ # --use-image after a reboot to avoid losing access to their existing chroot.
chroot_exists = cros_sdk_lib.IsChrootReady(options.chroot)
+ img_path = _ImageFileForChroot(options.chroot)
+ if (not options.use_image and not chroot_exists and not options.delete and
+ not options.unmount and os.path.exists(img_path)):
+ logging.notice('Existing chroot image %s found. Forcing --use-image on.',
+ img_path)
+ options.use_image = True
+
+ # Discern if we need to create the chroot.
if (options.use_image and not chroot_exists and not options.delete and
not options.unmount and not missing_image_tools and
- os.path.exists(_ImageFileForChroot(options.chroot))):
+ os.path.exists(img_path)):
# Try to re-mount an existing image in case the user has rebooted.
with cgroups.SimpleContainChildren('cros_sdk'):
with locking.FileLock(lock_path, 'chroot lock') as lock:
@@ -982,8 +992,7 @@
cros_sdk_lib.MountChroot(options.chroot, create=False)
chroot_exists = cros_sdk_lib.IsChrootReady(options.chroot)
if chroot_exists:
- logging.notice('Mounted existing image %s on chroot',
- _ImageFileForChroot(options.chroot))
+ logging.notice('Mounted existing image %s on chroot', img_path)
# Finally, flip create if necessary.
if options.enter or options.snapshot_create: