Re: [PATCH v2 2/3] xl/libxl: Add ability to specify SMBIOS strings

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Fri, Sep 9, 2022 at 6:19 AM Anthony PERARD <anthony.perard@xxxxxxxxxx> wrote:
>
> On Thu, Sep 08, 2022 at 03:51:12PM -0400, Jason Andryuk wrote:
> > hvm_xs_strings.h specifies xenstore entries which can be used to set or
> > override smbios strings.  hvmloader has support for reading them, but
> > xl/libxl support is not wired up.
> >
> > Allow specifying the strings with the new xl.cfg option:
> > smbios=["bios_vendor=Xen Project","system_version=1.0"]
>
> I've tried something like:
>     smbios=['oem=foo=bar']
> And only "foo" is added to the smbios string. Shouldn't we allow users
> to have '=' in their smbios strings?

Yes, you are correct.  split_string_into_pair() isn't suitable since
it terminates at the second '='.  Also, we probably don't want to trim
the strings to keep them unchanged, especially for any values.  If a
space-padded option name is provided, it'll show up in "xl: unknown
smbios type '%s'" and the whitespace would be evident.

hvmloader iterates through HVM_XS_OEM_STRINGS entries and stops
processing when an entry is not found, or it is zero-length.  So I
think we should disallow zero-length values like "oem=" to avoid
surprises.  For example, smbios=["oem=A","oem=","oem=B"] would result
in only oem-1 "A" making its way through to the guest.

Your other comments sound good.

Thanks,
Jason