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

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Sep 09, 2022 at 08:59:09AM -0400, Jason Andryuk wrote:
> 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.

Both suggestion sounds good.

Cheers,

-- 
Anthony PERARD