Re: [PATCH v7 08/12] xen: add /buildinfo/config entry to hypervisor filesystem

[Jürgen Groß <jgross@xxxxxxxx>]

On 03.04.20 16:31, Jan Beulich wrote:
> On 02.04.2020 17:46, Juergen Gross wrote:
> > --- a/xen/common/Kconfig
> > +++ b/xen/common/Kconfig
> > @@ -353,6 +353,16 @@ config DOM0_MEM
> >   
> >   	  Leave empty if you are not sure what to specify.
> >   
> > +config HYPFS_CONFIG
> > +       bool "Provide hypervisor .config via hypfs entry"
> > +       default y
> My initial reaction was to ask for "depends on HYPFS", but then
> I noticed the earlier patch doesn't introduce such. Am I
> mis-remembering that it was agreed to make the whole thing
> possible to disable at least in EXPERT mode?
No, I don't remember that agreement.

And TBH I'm not sure this is a good idea, as that would at once make the
plan to replace at least some sysctl and/or domctl interfaces impossible
(like e.g. the last 3 patches of the series are doing already), or at
least would tie the related functionality to CONFIG_HYPFS.

> > --- a/xen/common/kernel.c
> > +++ b/xen/common/kernel.c
> > @@ -389,6 +389,16 @@ static HYPFS_STRING_INIT(compile_date, "compile_date");
> >   static HYPFS_STRING_INIT(compile_domain, "compile_domain");
> >   static HYPFS_STRING_INIT(extra, "extra");
> >   
> > +#ifdef CONFIG_HYPFS_CONFIG
> > +static struct hypfs_entry_leaf config = {
> > +    .e.type = XEN_HYPFS_TYPE_STRING,
> > +    .e.encoding = XEN_HYPFS_ENC_GZIP,
> > +    .e.name = "config",
> > +    .e.read = hypfs_read_leaf,
> > +    .content = &xen_config_data
> > +};
> > +#endif
> Would be really good if no open-coded instantiations like this
> one would ever have to appear, i.e. if suitable macros were
> available. What's preventing use of one of the available ones
> here?
Right now it is the only case for a non plain encoding. The alternative
would have been to either specify the encoding explicitly at all other
node definitions, or to have a macro for this single instance.


Juergen