qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Yang Zhong <yang.zhong@intel.com>
Cc: pbonzini@redhat.com, qemu-devel@nongnu.org
Subject: Re: [PATCH v2] qapi: Cleanup SGX related comments and restore @section-size
Date: Thu, 20 Jan 2022 09:44:34 +0000	[thread overview]
Message-ID: <YekvAuBUzcfSaHiT@redhat.com> (raw)
In-Reply-To: <20220120091601.GA27922@yangzhon-Virtual>

On Thu, Jan 20, 2022 at 05:16:01PM +0800, Yang Zhong wrote:
> On Thu, Jan 20, 2022 at 09:10:34AM +0000, Daniel P. Berrangé wrote:
> > On Wed, Jan 19, 2022 at 06:57:20PM -0500, Yang Zhong wrote:
> > > The SGX NUMA patches were merged into Qemu 7.0 release, we need
> > > clarify detailed version history information and also change
> > > some related comments, which make SGX related comments clearer.
> > > 
> > > The QMP command schema promises backwards compatibility as standard.
> > > We temporarily restore "@section-size", which can avoid incompatible
> > > API breakage. The "@section-size" will be deprecated in 7.2 version.
> > > 
> > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > > Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > >  qapi/machine.json     |  4 ++--
> > >  qapi/misc-target.json | 17 ++++++++++++-----
> > >  hw/i386/sgx.c         | 11 +++++++++--
> > >  3 files changed, 23 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/qapi/machine.json b/qapi/machine.json
> > > index b6a37e17c4..cf47cb63a9 100644
> > > --- a/qapi/machine.json
> > > +++ b/qapi/machine.json
> > > @@ -1207,7 +1207,7 @@
> > >  #
> > >  # @memdev: memory backend linked with device
> > >  #
> > > -# @node: the numa node
> > > +# @node: the numa node (Since: 7.0)
> > >  #
> > >  # Since: 6.2
> > >  ##
> > > @@ -1288,7 +1288,7 @@
> > >  #
> > >  # @memdev: memory backend linked with device
> > >  #
> > > -# @node: the numa node
> > > +# @node: the numa node (Since: 7.0)
> > >  #
> > >  # Since: 6.2
> > >  ##
> > > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > > index 1022aa0184..a87358ea44 100644
> > > --- a/qapi/misc-target.json
> > > +++ b/qapi/misc-target.json
> > > @@ -344,9 +344,9 @@
> > >  #
> > >  # @node: the numa node
> > >  #
> > > -# @size: the size of epc section
> > > +# @size: the size of EPC section
> > >  #
> > > -# Since: 6.2
> > > +# Since: 7.0
> > >  ##
> > >  { 'struct': 'SGXEPCSection',
> > >    'data': { 'node': 'int',
> > > @@ -365,7 +365,9 @@
> > >  #
> > >  # @flc: true if FLC is supported
> > >  #
> > > -# @sections: The EPC sections info for guest
> > > +# @section-size: The EPC section size for guest (Will be deprecated in 7.2)
> > 
> > I expected deprecation would start now (7.0, and it would be removed
> > in 7.2.
> > 
> > Also needs to be documented in docs/about/deprecated.rst
> > 
> > 
>   
>    Thanks Daniel, Please check if below comments are okay or not? If no
>    problem, I will send v3 today, thanks! 
>  
>    diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>    index e21e07478f..810542427f 100644
>    --- a/docs/about/deprecated.rst
>    +++ b/docs/about/deprecated.rst
>    @@ -441,3 +441,13 @@ nanoMIPS ISA
> 
>    The ``nanoMIPS`` ISA has never been upstreamed to any compiler toolchain.
>    As it is hard to generate binaries for it, declare it deprecated.
>    +
>    +
>    +SGX backwards compatibility
>    +---------------------------
>    +
>    +@section-size (Since 7.0)
>    +''''''''''''''''''''''''
>    +
>    +The ``@section-size`` will be replaced with ``@sections`` struct and declare
>    +it deprecated.

This needs to be higher up in the file - look at the section
with the heading 'QEMU Machine Protocol (QMP) commands' for
how we list QMP features we're removing.


>    diff --git a/qapi/misc-target.json b/qapi/misc-target.json
>    index a87358ea44..c88fd0c2a2 100644
>    --- a/qapi/misc-target.json
>    +++ b/qapi/misc-target.json
>    @@ -365,7 +365,7 @@
>    #
>    # @flc: true if FLC is supported
>    #
>   -# @section-size: The EPC section size for guest (Will be deprecated in 7.2)
>   +# @section-size: The EPC section size for guest (7.0, and it would be removed in 7.2)

We don't need this comment - see the other reply in this thread
about using an '@deprecated' tag instead, which gets turned into
a comment in the auto-generated documentation. 


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  reply	other threads:[~2022-01-20 11:12 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-19 23:57 [PATCH v2] qapi: Cleanup SGX related comments and restore @section-size Yang Zhong
2022-01-20  0:24 ` Philippe Mathieu-Daudé via
2022-01-20  9:10 ` Daniel P. Berrangé
2022-01-20  9:16   ` Yang Zhong
2022-01-20  9:44     ` Daniel P. Berrangé [this message]
2022-01-20 13:17       ` Yang Zhong
2022-01-20 15:31   ` Philippe Mathieu-Daudé via
2022-01-20 15:40     ` Daniel P. Berrangé
2022-01-20 15:58       ` Philippe Mathieu-Daudé via

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YekvAuBUzcfSaHiT@redhat.com \
    --to=berrange@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=yang.zhong@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).