New issue
Advanced search Search tips

Issue 848447 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

launch vsh in container via garcon

Project Member Reported by smbar...@chromium.org, May 31 2018

Issue description

VM version 10739.0.0 allows launching vsh via garcon directly inside a container, rather than using the run_container.sh script. CrostiniManager should invoke:

vsh --vm_name=termina --target_container=penguin --owner_id=<cryptohome id>

The LXD environment variables are no longer necessary. There's also no need to specify the user.
 
Owner: nverne@chromium.org
If we no longer specify the user, do we default to "chronos" ? 

We want our users to enter their as username@penguin, not chronos@penguin, right? 
In this mode vsh just runs as the same user as garcon. This would allow changing usernames after setup, too.
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e9753dafe05596d1bb5784df2cb99ed7c549302e

commit e9753dafe05596d1bb5784df2cb99ed7c549302e
Author: Nicholas Verne <nverne@chromium.org>
Date: Fri Jun 01 09:11:04 2018

Supports the new vsh-into-running-container syntax

It is no longer necessary to use the run_container.sh script every time a
container shell is needed. If the container is already running, vsh now allows
"direct access".

Bug:  848447 
Change-Id: Ia9aeb120befd016386816cdd16fb3336b45426ff
Reviewed-on: https://chromium-review.googlesource.com/1082055
Reviewed-by: Timothy Loh <timloh@chromium.org>
Commit-Queue: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563568}
[modify] https://crrev.com/e9753dafe05596d1bb5784df2cb99ed7c549302e/chrome/browser/chromeos/crostini/crostini_manager.cc

Status: Fixed (was: Untriaged)
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 4 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/531ca8c8d8e01d141888a7f8fd8204854bc7da41

commit 531ca8c8d8e01d141888a7f8fd8204854bc7da41
Author: Chirantan Ekbote <chirantan@chromium.org>
Date: Mon Jun 04 23:35:06 2018

Revert "Supports the new vsh-into-running-container syntax"

This reverts commit e9753dafe05596d1bb5784df2cb99ed7c549302e.

Reason for revert: Possible suspect for  crbug.com/849028 .  The run_container.sh script would always start the container if it wasn't running but the new code path will not work if the container is not already running.

Bug:  849028 

Original change's description:
> Supports the new vsh-into-running-container syntax
> 
> It is no longer necessary to use the run_container.sh script every time a
> container shell is needed. If the container is already running, vsh now allows
> "direct access".
> 
> Bug:  848447 
> Change-Id: Ia9aeb120befd016386816cdd16fb3336b45426ff
> Reviewed-on: https://chromium-review.googlesource.com/1082055
> Reviewed-by: Timothy Loh <timloh@chromium.org>
> Commit-Queue: Nicholas Verne <nverne@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#563568}

TBR=timloh@chromium.org,smbarber@chromium.org,nverne@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  848447 
Change-Id: I7af784e0d73814e1aa5aae5e70ea6d45f556c71f
Reviewed-on: https://chromium-review.googlesource.com/1086071
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564291}
[modify] https://crrev.com/531ca8c8d8e01d141888a7f8fd8204854bc7da41/chrome/browser/chromeos/crostini/crostini_manager.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/24a34b8e4dbe6cc339a91343c9268fd2532d54bb

commit 24a34b8e4dbe6cc339a91343c9268fd2532d54bb
Author: Nicholas Verne <nverne@chromium.org>
Date: Thu Jun 14 00:46:02 2018

Reland "Supports the new vsh-into-running-container syntax"

This is a reland of e9753dafe05596d1bb5784df2cb99ed7c549302e

Original change's description:
> Supports the new vsh-into-running-container syntax
> 
> It is no longer necessary to use the run_container.sh script every time a
> container shell is needed. If the container is already running, vsh now allows
> "direct access".
> 
> Bug:  848447 
> Change-Id: Ia9aeb120befd016386816cdd16fb3336b45426ff
> Reviewed-on: https://chromium-review.googlesource.com/1082055
> Reviewed-by: Timothy Loh <timloh@chromium.org>
> Commit-Queue: Nicholas Verne <nverne@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#563568}

Bug:  848447 
Change-Id: I17c73a37a65a8316e62154556ee94fbc37dd703d
Reviewed-on: https://chromium-review.googlesource.com/1096875
Reviewed-by: Timothy Loh <timloh@chromium.org>
Commit-Queue: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567068}
[modify] https://crrev.com/24a34b8e4dbe6cc339a91343c9268fd2532d54bb/chrome/browser/chromeos/crostini/crostini_manager.cc

After sync'd to this commit my terminal starts up and crashes after a few seconds. Some kind of an error message shows up on the terminal right before it crashes.
It is working after reboot.
Sorry about the flip flop but it's not working. Even when the container is up and some Crostini apps are running, the terminal just crashes.
I'll try to take a look next week. I might have to implement a fallback mode for vsh in case it fails to connect to the container.

Sign in to add a comment