Re: [PATCH 4/5] SUPPORT.md: Move descriptions up before Status info

[Lars Kurth <lars.kurth@citrix.com>]

On 17/04/2018, 14:22, "Ian Jackson" <ian.jackson@citrix.com> wrote:

    Lars Kurth writes ("Re: [PATCH 4/5] SUPPORT.md: Move descriptions up before Status info"):
    > There were a couple of minor text changes for grammar reasons, which I noticed and highlighted.
    
    Thanks.
    
    > I also checked the code motions. There are some things which need to be pointed out, but they should not prevent this series from being checked in.
    > 
    > However, a couple were missed
    > * ### PV Console (frontend) => missed moving the note (which is a definition)
    
    That's one, not a couple.  I have fixed it.
    
    > I also spotted a few other inconsistencies, which we probably should fix, but these need backporting
    > * ARM: 16K and 64K page granularity in guests
    > * ARM: Guest Device Tree support
    > * ARM: Guest ACPI support
    > In all the other section headers we use x86/ or ARM/
    
    I think "x86:" and "ARM:" are more natural so I would prefer that
    bikeshed purple rather than blue.  I think the "/" came from the
    example of the guest types, which are indeed in some sense "x86/HVM"
    rather than "x86: HVM".
    
    I think we should treat that as a separate issue from this series.

Agreed
    
    >     -### x86/PVH
    >     -
    >          Status, domU: Supported
    >     -    Status, dom0: Experimental
    >     +
    >     +### x86/PVH
    >      
    >      PVH is a next-generation paravirtualized mode
    >      designed to take advantage of hardware virtualization support when possible.
    > 
    > Looks correct from a mere refactoring perspective, but generates some odd behaviour in https://xenbits.xen.org/people/iwj/2018/support-matrix-example-B-v1/t.html
    > 
    > The underlying reason is that we had some headline re-names between 4.10 and 4.11. e.g.
    > ARM guest => ARM
    > 
    > And some support statement changes, e.g. in x86/HVM guest
    > Status: Supported => Status, domU: Supported
    
    The rendering is indeed not ideal.  Our options are:
    
     (a) Live with it and document it.
    
     (b) Make it our practice to go always back and backport a name
         change for a feature to all versions.  I'm not sure this is worth
         the effort.
    
     (c) Invent some new equivalency metadata to put into SUPPORT.md or
         even into some other file in-tree.  Urgh, I don't want to do
         that.
    
    I chose (a). You will see a paragraph about this at the top of the
    html page:
    
      Sometimes the same feature, or a similar feature, is named
      differently in the documentation for different releases.  In such
      cases the table will show it as two separate features, with a
      discontinuity in support, even though support may have been
      continuous.

I can live with this approach
    
    > We probably need to go through some of these in 4.10 and fix them
    > But for 4.11 this is correct
    
    I think the 4.10 documentation is not wrong, just differently
    expressed.
    
    > The implication is that we need to minimize unnecessary changes to 
    > a) headings
    > b) clarifications to status before the colon
    > or backport them to older versions of SUPPORT.md. Otherwise the generated table will become confusing
    
    See above.  If you want to backport the heading changes, I'll ack your
    patches :-).

Happy to pick this up
    
    >      ### Direct-boot kernel image format
    >      
    >     +Format which the toolstack accepts for direct-boot kernels
    >     +
    >          Supported, x86: bzImage, ELF
    >          Supported, ARM32: zImage
    >          Supported, ARM64: Image
    >      
    >     -Format which the toolstack accepts for direct-boot kernels
    >     -
    > 
    > Note: the format here is wrong in both 4.10 and 4.11, this should be something like
    > 
    >          Status, zImage (ARM32): Supported
    > 
    > Lars will submit a separate patch
    
    This is not a blocker because I added parsing code for this format.
    If you fix it, we can drop that, too, once the change is backported.
    
    >      ## Scalability
    >      
    >      ### Super page support
    >      
    >     -    Status, x86 HVM/PVH, HAP: Supported
    >     -    Status, x86 HVM/PVH, Shadow, 2MiB: Supported
    >     -    Status, ARM: Supported
    >     -
    >      NB that this refers to the ability of guests
    > 
    > The beginning of this sentence should probably be changed to
    > "This feature refers to the ability of guests ..."
    
    Or even just "The ability of guests ..." since we don't normally lead
    each thing with "this is".  I think this is not very important.  If
    you want to improve it I will ack your patch.

Sure, I can roll this up with the other changes on my list
    
    >      ## Virtual Hardware, QEMU
    >      
    >     -These are devices available in HVM mode using a qemu devicemodel (the default).
    >     +This section describes supported devices available in HVM mode using a
    >     +qemu devicemodel (the default).
    >     +
    >     +    Status: Support scope restricted 
    >     +
    >      Note that other devices are available but not security supported.
    > 
    > This is causing a rendering issue: the footnote is not generated in the right place. It is added to " stgvga". Presumably a corner case in the table generation tool
    
    Yes.  It is generating semantically invalid html which renders very
    oddly, too.  I will fix it.
    
Thanks
Lars    

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel