Making obmc-console_git.bb more generic (again)?

Making obmc-console_git.bb more generic (again)?

[Oskar Senft]

Hi everyone

I noticed that as of
https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/30369 (aka
https://github.com/openbmc/openbmc/commit/abf95efe7c3a34cc2e5d7424abb59710fb4a1d4d),
obmc-console_git.bb assumes that we always want to use ttyVUART0.

We used to have support for OBMC_CONSOLE_HOST_TTY and then create the
symlink /etc/obmc-console/server.${OBMC_CONSOLE_HOST_TTY}.conf as
needed.

From what I can tell, OBMC_CONSOLE_HOST_TTY is still used in quite a
few machine layers. Some of them (e.g. meta-amd and meta-facebook)
even went so far to replicate the previous behavior by deleting
/etc/obmc-console/server.VUART0.conf and then re-creating the correct
one.

Is this actually the expected behavior? Or was that just an oversight
in the commit?

I'd be happy to send a review request to make this generic again if
people agree. A bunch of follow-up commits could then remove the
duplicate code in individual machine layer overrides.

Thanks
Oskar.

Re: Making obmc-console_git.bb more generic (again)?

[Patrick Williams]

[-- Attachment #1: Type: text/plain, Size: 1735 bytes --]

On Wed, Nov 17, 2021 at 05:01:25PM -0500, Oskar Senft wrote:
> Hi everyone
> 
> I noticed that as of
> https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/30369 (aka
> https://github.com/openbmc/openbmc/commit/abf95efe7c3a34cc2e5d7424abb59710fb4a1d4d),
> obmc-console_git.bb assumes that we always want to use ttyVUART0.

There was a push to move service files outside of the openbmc/openbmc repository
and into the underlying repos.  Brad could comment on why as he was
asking for it.

> We used to have support for OBMC_CONSOLE_HOST_TTY and then create the
> symlink /etc/obmc-console/server.${OBMC_CONSOLE_HOST_TTY}.conf as
> needed.
> 
> From what I can tell, OBMC_CONSOLE_HOST_TTY is still used in quite a
> few machine layers. Some of them (e.g. meta-amd and meta-facebook)
> even went so far to replicate the previous behavior by deleting
> /etc/obmc-console/server.VUART0.conf and then re-creating the correct
> one.

Speaking for the Facebook machines, we have some machines which use a different
vTTY and we have other machines which manage multiple hosts and thus have
multiple vTTYs.  We probably should have contributed code to pass the desired
vTTY(s) as a meson-option.

> Is this actually the expected behavior? Or was that just an oversight
> in the commit?

I think it was the "put the default/typical config into the repo and let
everyone customize it otherwise if they need" approach.

> I'd be happy to send a review request to make this generic again if
> people agree. A bunch of follow-up commits could then remove the
> duplicate code in individual machine layer overrides.

I'd be thankful for this.  Please feel free to add me as a reviewer.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Making obmc-console_git.bb more generic (again)?

[Andrew Jeffery]

On Tue, 23 Nov 2021, at 03:25, Patrick Williams wrote:
> On Wed, Nov 17, 2021 at 05:01:25PM -0500, Oskar Senft wrote:
>> Hi everyone
>> 
>> I noticed that as of
>> https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/30369 (aka
>> https://github.com/openbmc/openbmc/commit/abf95efe7c3a34cc2e5d7424abb59710fb4a1d4d),
>> obmc-console_git.bb assumes that we always want to use ttyVUART0.
>
> There was a push to move service files outside of the openbmc/openbmc repository
> and into the underlying repos.  Brad could comment on why as he was
> asking for it.
>
>> We used to have support for OBMC_CONSOLE_HOST_TTY and then create the
>> symlink /etc/obmc-console/server.${OBMC_CONSOLE_HOST_TTY}.conf as
>> needed.
>> 
>> From what I can tell, OBMC_CONSOLE_HOST_TTY is still used in quite a
>> few machine layers. Some of them (e.g. meta-amd and meta-facebook)
>> even went so far to replicate the previous behavior by deleting
>> /etc/obmc-console/server.VUART0.conf and then re-creating the correct
>> one.
>
> Speaking for the Facebook machines, we have some machines which use a different
> vTTY and we have other machines which manage multiple hosts and thus have
> multiple vTTYs.  We probably should have contributed code to pass the desired
> vTTY(s) as a meson-option.
>
>> Is this actually the expected behavior? Or was that just an oversight
>> in the commit?
>
> I think it was the "put the default/typical config into the repo and let
> everyone customize it otherwise if they need" approach.
>
>> I'd be happy to send a review request to make this generic again if
>> people agree. A bunch of follow-up commits could then remove the
>> duplicate code in individual machine layer overrides.
>
> I'd be thankful for this.  Please feel free to add me as a reviewer.

Yeah I'm not sure what happened here, whether that was something I did 
or if someone's made changes to the recipe since. When I added support 
for exposing multiple devices from the one BMC there were some 
complications around not breaking everyone vs having to modify every 
in-tree consumer. I didn't have the bandwidth or ability to test fixing 
all the platforms at the time so I put some work-arounds in the install 
phase of the obmc-console recipe. Maybe that broke things?

Anyway, please also CC me on cleanups.

Cheers,

Andrew

Re: Making obmc-console_git.bb more generic (again)?

[Oskar Senft]

Hi everyone

I just sent https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/49082
for review.

It took me a while to test (mainly because Yocto is weird). From what
I can tell, this should be fully "backwards" compatible with existing
meta machine layers that do or do not use the OBMC_CONSOLE_HOST_TTY
variable.

Oskar.

On Mon, Nov 22, 2021 at 6:20 PM Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
>
> On Tue, 23 Nov 2021, at 03:25, Patrick Williams wrote:
> > On Wed, Nov 17, 2021 at 05:01:25PM -0500, Oskar Senft wrote:
> >> Hi everyone
> >>
> >> I noticed that as of
> >> https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/30369 (aka
> >> https://github.com/openbmc/openbmc/commit/abf95efe7c3a34cc2e5d7424abb59710fb4a1d4d),
> >> obmc-console_git.bb assumes that we always want to use ttyVUART0.
> >
> > There was a push to move service files outside of the openbmc/openbmc repository
> > and into the underlying repos.  Brad could comment on why as he was
> > asking for it.
> >
> >> We used to have support for OBMC_CONSOLE_HOST_TTY and then create the
> >> symlink /etc/obmc-console/server.${OBMC_CONSOLE_HOST_TTY}.conf as
> >> needed.
> >>
> >> From what I can tell, OBMC_CONSOLE_HOST_TTY is still used in quite a
> >> few machine layers. Some of them (e.g. meta-amd and meta-facebook)
> >> even went so far to replicate the previous behavior by deleting
> >> /etc/obmc-console/server.VUART0.conf and then re-creating the correct
> >> one.
> >
> > Speaking for the Facebook machines, we have some machines which use a different
> > vTTY and we have other machines which manage multiple hosts and thus have
> > multiple vTTYs.  We probably should have contributed code to pass the desired
> > vTTY(s) as a meson-option.
> >
> >> Is this actually the expected behavior? Or was that just an oversight
> >> in the commit?
> >
> > I think it was the "put the default/typical config into the repo and let
> > everyone customize it otherwise if they need" approach.
> >
> >> I'd be happy to send a review request to make this generic again if
> >> people agree. A bunch of follow-up commits could then remove the
> >> duplicate code in individual machine layer overrides.
> >
> > I'd be thankful for this.  Please feel free to add me as a reviewer.
>
> Yeah I'm not sure what happened here, whether that was something I did
> or if someone's made changes to the recipe since. When I added support
> for exposing multiple devices from the one BMC there were some
> complications around not breaking everyone vs having to modify every
> in-tree consumer. I didn't have the bandwidth or ability to test fixing
> all the platforms at the time so I put some work-arounds in the install
> phase of the obmc-console recipe. Maybe that broke things?
>
> Anyway, please also CC me on cleanups.
>
> Cheers,
>
> Andrew