I don't think this should be blocking for bug 885276:  For that bug, the fix
is just "pass the model name to the --board option".  That's not ideal as
a long-term solution, but it's good enough for beating the octopus deadline.

adurbin was concerned about too many xml symlinks, and this is an easy fix. I can have the code in today if that's helpful.

If we know both board and model, we don't need to create those symlinks. We could just search "servod_${MODEL}_overaly.xml" first. If it doesn't exists, then fall back to "servod_${BOARD}_overlay.xml".

Yes that's pretty much what I was going to implement.

> adurbin was concerned about too many xml symlinks, and this is an easy fix. I can have the code in today if that's helpful.

This fix alone won't help, because the new --model option would have to
be incorporated into the upstart job.  I'd also expect that Autotest would
be affected by this change as well.

Long term, if this provides a way to cut down on the proliferation of servo
overlay files, it's a good thing.  I'm a bit concerned that this kind of
change has a few too many moving parts to be certain that we can deliver it
on the octopus schedule.

Put more simply:  We should do what we know will work for octopus to meet
the deadline.  Then, we should go and clean up and make it better once we're
off the octopus critical path.

guocb@ in #c5
> If we know both board and model, we don't need to create
> those symlinks. We could just search "servod_${MODEL}_overaly.xml"
> first. If it doesn't exists, then fall back to "servod_${BOARD}_overlay.xml".

nsanders@ in #c7
> Yes that's pretty much what I was going to implement.

This'll work, but it still means that both "board" and "model" will have
to be supplied to the deployment script.

In crbug.com/885276#c12 there's this suggestion:
> If there were some trebuchet style offline db, that could map model
> to servo config in deployment scripts, chroot, autotest, wherever,
> that would be a good place to put it, and I thing that's a good long
> term goal.

This is nominally the stable, supportable solution we all want.  So,
some important questions:
  * Is that solution reasonably achievable in the near to medium term?
  * Assuming that the answer above is "yes", do we gain much by
    implementing the suggestion here in favor of going straight to that
    final solution?

With regard to time, I don't think we'll have any systems with a non-NPCX EC in the lab until roughly late November to December

Isn't cros_config_host effectively already that offline database?

> Isn't cros_config_host effectively already that offline database?

The problem is "on the DUT" is not "offline".  We need to be able to
query that content without having a working DUT.

cros_config_host doesn't run on the DUT.

> cros_config_host doesn't run on the DUT.

Ah.  Where does it come from, and how does it get built?

More broadly, how can we arrange for the executable program to be available
on any Google-issued laptop that might ask for it?

Partially answering my own questions:

(cros.base) jrbarnette@jrbarnette ~/trunk/src/scripts $ which cros_config_host
/usr/bin/cros_config_host
(cros.base) jrbarnette@jrbarnette ~/trunk/src/scripts $ equery b /usr/bin/cros_config_host
 * Searching for /usr/bin/cros_config_host ... 
chromeos-base/chromeos-config-host-0.0.1-r325 (/usr/bin/cros_config_host -> ../lib/python-exec/python-exec2)
(cros.base) jrbarnette@jrbarnette ~/trunk/src/scripts $ file $(which cros_config_host)
/usr/bin/cros_config_host: symbolic link to ../lib/python-exec/python-exec2

IIUC, this command is built in part from data contained in the private
overlays, and it's built inside the chroot.  So, making this command
available within the context of the chromeos lab tools involves some
new concepts.  Possible, but work is required.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/99c45da062fca46611ac6a28207253c4812b26a4

commit 99c45da062fca46611ac6a28207253c4812b26a4
Author: Nick Sanders <nsanders@chromium.org>
Date: Fri Sep 21 04:30:08 2018

servod: add -m model arg

This adds an optional argument '-m' which specifies
a model under a board. Starting with Octopus, not all
devices within a reference design/overlay share the
same servo header and component layout. We do currently
guarantee that model specifies a particular MLB, so
this is a sufficient and discoverable disambiguation.

BUG= chromium:886955 
TEST=the right files load for phaser and apel.

Change-Id: I5946b3ce3f87c4762d6650c7affb9121fe675d27
Reviewed-on: https://chromium-review.googlesource.com/1235334
Commit-Ready: Nick Sanders <nsanders@chromium.org>
Tested-by: Nick Sanders <nsanders@chromium.org>
Reviewed-by: Justin TerAvest <teravest@chromium.org>

[add] https://crrev.com/99c45da062fca46611ac6a28207253c4812b26a4/servo/data/servo_octopus_apel_overlay.xml
[modify] https://crrev.com/99c45da062fca46611ac6a28207253c4812b26a4/servo/multiservo.py
[modify] https://crrev.com/99c45da062fca46611ac6a28207253c4812b26a4/servo/servod.py
[add] https://crrev.com/99c45da062fca46611ac6a28207253c4812b26a4/servo/data/servo_octopus_ampton_overlay.xml

I want to slightly repurpose this bug:  bug 885276 comes
in three parts:
 1) Change the deploy script to apply the model label at host creation.
 2) Change servo repair to pass the model name instead of the board
    name to the servo upstart job
 3) Change the servod upstart job to accept model names for selecting
    overlays.

I want to make bug 885276 about problems 1) and 2), and make this bug
about problem 3).

Some notes:
  * The change in #c15 isn't complete:  we must also change the upstart
    job, and take care that the change is compatible.
  * The change in #c15 is awkward in that the interface becomes not
    "use option X" but rather "use option X or Y, whichever happens to
    exist."  That's more complicated, and the complication isn't inherently
    necessary.
  * The actual requirement is (effectively) merely that "model names can
    uniquely identify an XML overlay."  Since we've decided that we don't
    want individual XML files for each model name, this means what we want
    is merely a mapping of "model -> XML overlay file".

So, I think these are our options:
 1) Continue with the -m option as currently constituted, and enhance the
    upstart job(s) to add an optional "MODEL" parameter that supplements
    the "BOARD" parameter.
 2) Alter the meaning of the -m to be more like "look up the model name
    in a model -> file name map."  Then change the upstart job to match
    that usage.

My vote is for 2).  That means changes along these lines:
  * Create the "model -> file name" map.  That can be a simple hard-coded
    python dict, although a YAML file has also been suggested.  N.B. The
    map should provide for a default mapping of "servo_%s_overlay.xml",
    to match the behavior of the --board option.
  * Change the -m option to look up overlays in the model map.  The option
    should be mutually exclusive with the -b option.
  * Change the upstart job to accept either "BOARD" or "MODEL".  The job
    will start servod either with "--model $BOARD" [sic] or "--model $MODEL",
    depending on the provided parameter.

Once that's done and pushed to all the servo hosts, we can update Autotest
to pass "MODEL" in place of "BOARD".  After that we can go back and remove
support for the "BOARD" parameter at leisure.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/hdctools/+/4a9e3899ba3c3267c241a22e687cff2b24e28b0c

commit 4a9e3899ba3c3267c241a22e687cff2b24e28b0c
Author: Nick Sanders <nsanders@chromium.org>
Date: Fri Oct 19 19:19:45 2018

servod.conf: add support for model

This adds an optional config MODEL= to servod.conf
Having this override allows differentiated servo
config based on model. This is cached, and passed
through as the --model arg to servod.

BUG= chromium:886955 
TEST=startup works with and without arg. If present it's passed to servod.

Change-Id: I565382a91d25809ac70cd65f2ad6c132665e878b
Reviewed-on: https://chromium-review.googlesource.com/1288997
Commit-Ready: Nick Sanders <nsanders@chromium.org>
Tested-by: Nick Sanders <nsanders@chromium.org>
Reviewed-by: Justin TerAvest <teravest@chromium.org>

[modify] https://crrev.com/4a9e3899ba3c3267c241a22e687cff2b24e28b0c/chromeos/servod.conf

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/2f3c985085a788673f8eb8302d1baa8d8cb4ab36

commit 2f3c985085a788673f8eb8302d1baa8d8cb4ab36
Author: Nick Sanders <nsanders@chromium.org>
Date: Thu Nov 01 04:59:35 2018

autotest: allow servo repair with model

Add MODEL=model arg to servod restart. This allows
octopus devices with ite ECs to be repaired in the
lab.

BUG=chromium:885276, chromium:886955 
TEST=will run in staging lab

Change-Id: I190caf73792a8e3409547ec479e579c5a87827ec
Reviewed-on: https://chromium-review.googlesource.com/1298355
Commit-Ready: Alex Zamorzaev <zamorzaev@chromium.org>
Tested-by: Nick Sanders <nsanders@chromium.org>
Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>

[modify] https://crrev.com/2f3c985085a788673f8eb8302d1baa8d8cb4ab36/server/hosts/cros_host.py
[modify] https://crrev.com/2f3c985085a788673f8eb8302d1baa8d8cb4ab36/server/hosts/servo_host.py
[modify] https://crrev.com/2f3c985085a788673f8eb8302d1baa8d8cb4ab36/server/hosts/servo_repair.py