Use crossystem inside_vm

Use crossystem inside_vm

BUG=chromium:609001
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/a88f0dee52f5ae9e84c3922ebcefc58ec0c39e65

[achuithb, 2016-08-17 19:45:59]

The CQ bit was checked by achuith@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-17 19:46:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[achuithb, 2016-08-17 19:46:29]

achuith@chromium.org changed reviewers:
+ norvez@chromium.org

[achuithb, 2016-08-17 19:46:29]

Nicolas, PTAL

[commit-bot: I haz the power, 2016-08-17 20:55:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-17 20:55:01]

Dry run: This issue passed the CQ dry run.

[norvez, 2016-08-18 11:28:27]

https://codereview.chromium.org/2252863004/diff/20001/telemetry/telemetry/cor...
File telemetry/telemetry/core/cros_interface.py (right):

https://codereview.chromium.org/2252863004/diff/20001/telemetry/telemetry/cor...
telemetry/telemetry/core/cros_interface.py:506: return
self.RunCmdOnDevice(['crossystem', 'inside_vm'])[0] != '0'
Can you rather check that stdout == '1'? Otherwise errors (e.g. older crossystem
binary or broken system) will be false positives.

[achuithb, 2016-08-18 18:36:02]

https://codereview.chromium.org/2252863004/diff/20001/telemetry/telemetry/cor...
File telemetry/telemetry/core/cros_interface.py (right):

https://codereview.chromium.org/2252863004/diff/20001/telemetry/telemetry/cor...
telemetry/telemetry/core/cros_interface.py:506: return
self.RunCmdOnDevice(['crossystem', 'inside_vm'])[0] != '0'
On 2016/08/18 11:28:27, norvez wrote:
> Can you rather check that stdout == '1'? Otherwise errors (e.g. older
crossystem
> binary or broken system) will be false positives.

This is intentional - we prefer false positives, and a false positive would
retain old behavior.

https://codereview.chromium.org/2252863004/diff/20001/telemetry/telemetry/int...
File telemetry/telemetry/internal/platform/cros_platform_backend.py (right):

https://codereview.chromium.org/2252863004/diff/20001/telemetry/telemetry/int...
telemetry/telemetry/internal/platform/cros_platform_backend.py:176: return not
self.cri.IsRunningOnVM()
We prefer to default IsRunningOnVM to True, so CanTakeScreenshot will default to
False. We will then not attempt to take a screenshot on a platform where it
isn't possible to take a screenshot.

This is also the old default behavior (return False).

[norvez, 2016-08-18 22:58:00]

lgtm

https://codereview.chromium.org/2252863004/diff/20001/telemetry/telemetry/cor...
File telemetry/telemetry/core/cros_interface.py (right):

https://codereview.chromium.org/2252863004/diff/20001/telemetry/telemetry/cor...
telemetry/telemetry/core/cros_interface.py:506: return
self.RunCmdOnDevice(['crossystem', 'inside_vm'])[0] != '0'
On 2016/08/18 18:36:02, achuithb wrote:
> On 2016/08/18 11:28:27, norvez wrote:
> > Can you rather check that stdout == '1'? Otherwise errors (e.g. older
> crossystem
> > binary or broken system) will be false positives.
> 
> This is intentional - we prefer false positives, and a false positive would
> retain old behavior.

ok

[achuithb, 2016-08-18 23:02:50]

On 2016/08/18 22:58:00, norvez wrote:
> lgtm
> 
>
https://codereview.chromium.org/2252863004/diff/20001/telemetry/telemetry/cor...
> File telemetry/telemetry/core/cros_interface.py (right):
> 
>
https://codereview.chromium.org/2252863004/diff/20001/telemetry/telemetry/cor...
> telemetry/telemetry/core/cros_interface.py:506: return
> self.RunCmdOnDevice(['crossystem', 'inside_vm'])[0] != '0'
> On 2016/08/18 18:36:02, achuithb wrote:
> > On 2016/08/18 11:28:27, norvez wrote:
> > > Can you rather check that stdout == '1'? Otherwise errors (e.g. older
> > crossystem
> > > binary or broken system) will be false positives.
> > 
> > This is intentional - we prefer false positives, and a false positive would
> > retain old behavior.
> 
> ok

ty

[achuithb, 2016-08-18 23:02:55]

The CQ bit was checked by achuith@chromium.org

[commit-bot: I haz the power, 2016-08-18 23:02:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[achuithb, 2016-08-18 23:03:18]

achuith@chromium.org changed reviewers:
+ nednguyen@google.com

[achuithb, 2016-08-18 23:03:18]

Ned, could you please rubberstamp?

[nednguyen, 2016-08-18 23:25:18]

lgtm

[commit-bot: I haz the power, 2016-08-18 23:36:55]

Description was changed from

==========
Use crossystem inside_vm

BUG=chromium:609001
==========

to

==========
Use crossystem inside_vm

BUG=chromium:609001

Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-08-18 23:36:56]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...