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

["Jürgen Groß" <jgross@suse.com>]

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