diff mbox series

docs: specify stability of hypfs path documentation

Message ID 20200713051639.26948-1-jgross@suse.com (mailing list archive)
State Superseded
Headers show
Series docs: specify stability of hypfs path documentation | expand

Commit Message

Jürgen Groß July 13, 2020, 5:16 a.m. UTC
In docs/misc/hypfs-paths.pandoc the supported paths in the hypervisor
file system are specified. Make it more clear that path availability
might change, e.g. due to scope widening or narrowing (e.g. being
limited to a specific architecture).

Signed-off-by: Juergen Gross <jgross@suse.com>
---
This might be a candidate for 4.14, as hypfs is new in 4.14 and the
documentation should be as clear as possible.
---
 docs/misc/hypfs-paths.pandoc | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Jan Beulich July 13, 2020, 7:56 a.m. UTC | #1
On 13.07.2020 07:16, Juergen Gross wrote:
> @@ -55,6 +58,11 @@ tags enclosed in square brackets.
>  * CONFIG_* -- Path is valid only in case the hypervisor was built with
>    the respective config option.
>  
> +Path availability is subject to change, e.g. a specific path specified
> +for a single architecture now might be made available for other architectures
> +in future, or it could be made conditional by an additional config option
> +of the hypervisor.

I agree this is worthwhile clarifying. To me, between the lines,
this then suggests that paths are entirely unreliable, which I
don't think is what we want. So perhaps some further clarification
could be added to clarify that we're not going to arbitrarily
change paths or their meaning? Or am I mistaken in understanding
that this interface is meant to act ABI-like?

Jan
Jürgen Groß July 13, 2020, 8:04 a.m. UTC | #2
On 13.07.20 09:56, Jan Beulich wrote:
> On 13.07.2020 07:16, Juergen Gross wrote:
>> @@ -55,6 +58,11 @@ tags enclosed in square brackets.
>>   * CONFIG_* -- Path is valid only in case the hypervisor was built with
>>     the respective config option.
>>   
>> +Path availability is subject to change, e.g. a specific path specified
>> +for a single architecture now might be made available for other architectures
>> +in future, or it could be made conditional by an additional config option
>> +of the hypervisor.
> 
> I agree this is worthwhile clarifying. To me, between the lines,
> this then suggests that paths are entirely unreliable, which I
> don't think is what we want. So perhaps some further clarification
> could be added to clarify that we're not going to arbitrarily
> change paths or their meaning? Or am I mistaken in understanding
> that this interface is meant to act ABI-like?

You are right. What about following rewording:

"In case a tag for a path indicates that this path is available in some
  case only, this availability might be extended or reduced in future by
  modification or removal of the tag."


Juergen
Jan Beulich July 13, 2020, 8:07 a.m. UTC | #3
On 13.07.2020 10:04, Jürgen Groß wrote:
> On 13.07.20 09:56, Jan Beulich wrote:
>> On 13.07.2020 07:16, Juergen Gross wrote:
>>> @@ -55,6 +58,11 @@ tags enclosed in square brackets.
>>>   * CONFIG_* -- Path is valid only in case the hypervisor was built with
>>>     the respective config option.
>>>   
>>> +Path availability is subject to change, e.g. a specific path specified
>>> +for a single architecture now might be made available for other architectures
>>> +in future, or it could be made conditional by an additional config option
>>> +of the hypervisor.
>>
>> I agree this is worthwhile clarifying. To me, between the lines,
>> this then suggests that paths are entirely unreliable, which I
>> don't think is what we want. So perhaps some further clarification
>> could be added to clarify that we're not going to arbitrarily
>> change paths or their meaning? Or am I mistaken in understanding
>> that this interface is meant to act ABI-like?
> 
> You are right. What about following rewording:
> 
> "In case a tag for a path indicates that this path is available in some
>   case only, this availability might be extended or reduced in future by
>   modification or removal of the tag."

Yes, yet I'd go even further and add "A path once assigned meaning
won't go away altogether or change its meaning, though" or some such.

Jan
Paul Durrant July 13, 2020, 8:13 a.m. UTC | #4
> -----Original Message-----
> From: Juergen Gross <jgross@suse.com>
> Sent: 13 July 2020 06:17
> To: xen-devel@lists.xenproject.org
> Cc: paul@xen.org; Juergen Gross <jgross@suse.com>; Andrew Cooper <andrew.cooper3@citrix.com>; George
> Dunlap <george.dunlap@citrix.com>; Ian Jackson <ian.jackson@eu.citrix.com>; Jan Beulich
> <jbeulich@suse.com>; Julien Grall <julien@xen.org>; Stefano Stabellini <sstabellini@kernel.org>; Wei
> Liu <wl@xen.org>
> Subject: [PATCH] docs: specify stability of hypfs path documentation
> 
> In docs/misc/hypfs-paths.pandoc the supported paths in the hypervisor
> file system are specified. Make it more clear that path availability
> might change, e.g. due to scope widening or narrowing (e.g. being
> limited to a specific architecture).
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> This might be a candidate for 4.14, as hypfs is new in 4.14 and the
> documentation should be as clear as possible.

Agreed. Since this a pure documentation change it carries no risk, so once the final wording is agreed then consider it...

Release-acked-by: Paul Durrant <paul@xen.org>

> ---
>  docs/misc/hypfs-paths.pandoc | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/docs/misc/hypfs-paths.pandoc b/docs/misc/hypfs-paths.pandoc
> index a111c6f25c..7ad4b7ba95 100644
> --- a/docs/misc/hypfs-paths.pandoc
> +++ b/docs/misc/hypfs-paths.pandoc
> @@ -5,6 +5,9 @@ in the Xen hypervisor file system (hypfs).
> 
>  The hypervisor file system can be accessed via the xenhypfs tool.
> 
> +The availability of the hypervisor file system depends on the hypervisor
> +config option CONFIG_HYPFS, which is on per default.
> +
>  ## Notation
> 
>  The hypervisor file system is similar to the Linux kernel's sysfs.
> @@ -55,6 +58,11 @@ tags enclosed in square brackets.
>  * CONFIG_* -- Path is valid only in case the hypervisor was built with
>    the respective config option.
> 
> +Path availability is subject to change, e.g. a specific path specified
> +for a single architecture now might be made available for other architectures
> +in future, or it could be made conditional by an additional config option
> +of the hypervisor.
> +
>  So an entry could look like this:
> 
>      /cpu-bugs/active-pv/xpti = ("No"|{"dom0", "domU", "PCID-on"}) [w,X86,PV]
> --
> 2.26.2
Jürgen Groß July 13, 2020, 8:14 a.m. UTC | #5
On 13.07.20 10:07, Jan Beulich wrote:
> On 13.07.2020 10:04, Jürgen Groß wrote:
>> On 13.07.20 09:56, Jan Beulich wrote:
>>> On 13.07.2020 07:16, Juergen Gross wrote:
>>>> @@ -55,6 +58,11 @@ tags enclosed in square brackets.
>>>>    * CONFIG_* -- Path is valid only in case the hypervisor was built with
>>>>      the respective config option.
>>>>    
>>>> +Path availability is subject to change, e.g. a specific path specified
>>>> +for a single architecture now might be made available for other architectures
>>>> +in future, or it could be made conditional by an additional config option
>>>> +of the hypervisor.
>>>
>>> I agree this is worthwhile clarifying. To me, between the lines,
>>> this then suggests that paths are entirely unreliable, which I
>>> don't think is what we want. So perhaps some further clarification
>>> could be added to clarify that we're not going to arbitrarily
>>> change paths or their meaning? Or am I mistaken in understanding
>>> that this interface is meant to act ABI-like?
>>
>> You are right. What about following rewording:
>>
>> "In case a tag for a path indicates that this path is available in some
>>    case only, this availability might be extended or reduced in future by
>>    modification or removal of the tag."
> 
> Yes, yet I'd go even further and add "A path once assigned meaning
> won't go away altogether or change its meaning, though" or some such.

Fine with me. Will send V2.


Juergen
Jürgen Groß July 13, 2020, 8:15 a.m. UTC | #6
On 13.07.20 10:13, Paul Durrant wrote:
>> -----Original Message-----
>> From: Juergen Gross <jgross@suse.com>
>> Sent: 13 July 2020 06:17
>> To: xen-devel@lists.xenproject.org
>> Cc: paul@xen.org; Juergen Gross <jgross@suse.com>; Andrew Cooper <andrew.cooper3@citrix.com>; George
>> Dunlap <george.dunlap@citrix.com>; Ian Jackson <ian.jackson@eu.citrix.com>; Jan Beulich
>> <jbeulich@suse.com>; Julien Grall <julien@xen.org>; Stefano Stabellini <sstabellini@kernel.org>; Wei
>> Liu <wl@xen.org>
>> Subject: [PATCH] docs: specify stability of hypfs path documentation
>>
>> In docs/misc/hypfs-paths.pandoc the supported paths in the hypervisor
>> file system are specified. Make it more clear that path availability
>> might change, e.g. due to scope widening or narrowing (e.g. being
>> limited to a specific architecture).
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> This might be a candidate for 4.14, as hypfs is new in 4.14 and the
>> documentation should be as clear as possible.
> 
> Agreed. Since this a pure documentation change it carries no risk, so once the final wording is agreed then consider it...
> 
> Release-acked-by: Paul Durrant <paul@xen.org>

Thanks!


Juergen
diff mbox series

Patch

diff --git a/docs/misc/hypfs-paths.pandoc b/docs/misc/hypfs-paths.pandoc
index a111c6f25c..7ad4b7ba95 100644
--- a/docs/misc/hypfs-paths.pandoc
+++ b/docs/misc/hypfs-paths.pandoc
@@ -5,6 +5,9 @@  in the Xen hypervisor file system (hypfs).
 
 The hypervisor file system can be accessed via the xenhypfs tool.
 
+The availability of the hypervisor file system depends on the hypervisor
+config option CONFIG_HYPFS, which is on per default.
+
 ## Notation
 
 The hypervisor file system is similar to the Linux kernel's sysfs.
@@ -55,6 +58,11 @@  tags enclosed in square brackets.
 * CONFIG_* -- Path is valid only in case the hypervisor was built with
   the respective config option.
 
+Path availability is subject to change, e.g. a specific path specified
+for a single architecture now might be made available for other architectures
+in future, or it could be made conditional by an additional config option
+of the hypervisor.
+
 So an entry could look like this:
 
     /cpu-bugs/active-pv/xpti = ("No"|{"dom0", "domU", "PCID-on"}) [w,X86,PV]