qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>,
	"Fam Zheng" <famz@redhat.com>
Cc: "Jan Kiszka" <jan.kiszka@siemens.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	qemu-devel@nongnu.org,
	"Samuel Thibault" <samuel.thibault@ens-lyon.org>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>
Subject: Re: [Qemu-devel] [PATCH v5 1/2] slirp: Add "query-usernet" QMP command
Date: Thu, 3 May 2018 09:53:18 -0500	[thread overview]
Message-ID: <38bb4fbe-481e-b859-c4bc-bcdb9dba0957@redhat.com> (raw)
In-Reply-To: <20180503091216.GD11382@redhat.com>

On 05/03/2018 04:12 AM, Daniel P. Berrangé wrote:
> On Thu, May 03, 2018 at 09:25:45AM +0800, Fam Zheng wrote:
>> HMP "info usernet" has been available but it isn't ideal for programmed
>> use cases. This closes the gap in QMP by adding a counterpart
>> "query-usernet" command. It is basically translated from
>> the HMP slirp_connection_info() loop, which now calls the QMP
>> implementation and prints the data, just like other HMP info_* commands.
>>
>> The TCPS_* macros are now defined as a QAPI enum.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
> 

>> +##
>> +{ 'enum': 'TCPS',
> 
> I'd suggest avoiding the abbreviation, IOW use  TCPState as
> the name. Yes that will require changnig the constants in
> the SLIRP code, but that's worthwhile to get this QAPI
> naming clearer IMHO.
> 
> I wonder if we should have a common prefix too eg UsernetTCPState
> as this is a global QAPI namespace after all .

You could also use

{ 'enum': 'UsernetTCPState', 'prefix': 'TCPS', ...

for minimizing code churn (if 'prefix' is present, that overrides what 
the generator would use for enum values); although I'm not sure if that 
is any better (we haven't used 'prefix' much, and Markus wasn't the 
biggest fan of it in the first place, as it is more of a hack).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

  parent reply	other threads:[~2018-05-03 14:53 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-03  1:25 [Qemu-devel] [PATCH v5 0/2] slirp: Add query-usernet QMP command Fam Zheng
2018-05-03  1:25 ` [Qemu-devel] [PATCH v5 1/2] slirp: Add "query-usernet" " Fam Zheng
2018-05-03  9:12   ` Daniel P. Berrangé
2018-05-03  9:27     ` Fam Zheng
2018-05-03 14:53     ` Eric Blake [this message]
2018-05-03 17:29       ` Markus Armbruster
2018-05-03  1:25 ` [Qemu-devel] [PATCH v5 2/2] tests: Use query-usernet instead of 'info usernet' Fam Zheng

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=38bb4fbe-481e-b859-c4bc-bcdb9dba0957@redhat.com \
    --to=eblake@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=famz@redhat.com \
    --cc=jan.kiszka@siemens.com \
    --cc=jasowang@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=samuel.thibault@ens-lyon.org \
    /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).