All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Doug Evans <dje@google.com>
Cc: "Jason Wang" <jasowang@redhat.com>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Samuel Thibault" <samuel.thibault@ens-lyon.org>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PATCH] net: Add "info neighbors" command
Date: Sat, 04 Sep 2021 08:26:11 +0200	[thread overview]
Message-ID: <87lf4crh4s.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <CADPb22TrHzujCONPAqdTcOYWNw2YtdKn+vPnitL7eU2hJJNDNg@mail.gmail.com> (Doug Evans's message of "Fri, 3 Sep 2021 09:53:02 -0700")

Doug Evans <dje@google.com> writes:

> On Fri, Sep 3, 2021 at 6:08 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Doug Evans <dje@google.com> writes:
>>
>> > This command dumps the ARP and NDP tables maintained within slirp.
>> > One use-case for it is showing the guest's IPv6 address(es).
>> >
>> > Signed-off-by: Doug Evans <dje@google.com>
>> > ---
>> >  hmp-commands-info.hx               | 15 +++++++
>> >  include/net/slirp.h                |  1 +
>> >  net/slirp.c                        | 15 +++++++
>> >  tests/acceptance/info_neighbors.py | 69 ++++++++++++++++++++++++++++++
>> >  4 files changed, 100 insertions(+)
>> >  create mode 100644 tests/acceptance/info_neighbors.py
>>
>> Standard request for new HMP commands without corresponding QMP
>> commands: please state in the commit message why the QMP command is not
>> worthwhile.
>>
>> HMP commands without a QMP equivalent are okay if their functionality
>> makes no sense in QMP, or is of use only for human users.
>>
>> Example for "makes no sense in QMP": setting the current CPU, because a
>> QMP monitor doesn't have a current CPU.
>>
>> Examples for "is of use only for human users": HMP command "help", the
>> integrated pocket calculator.
>>
>> Debugging commands are kind of borderline.  Debugging is commonly a
>> human activity, where HMP is just fine.  However, humans create tools to
>> assist with their activities, and then QMP is useful.  While I wouldn't
>> encourage HMP-only for the debugging use case, I wouldn't veto it.
>>
>
>
> Mostly I was following what I saw for "info usernet".
> I don't see a difference between "info neighbors" and "info usernet" so I
> went with that.
> Both draw their data from libslirp.

I see.

> I'm happy to add QMP support if necessary.
> Note that there is code that parses "info usernet" output, e.g.,
> get_info_usernet_hostfwd_port for python.

Demonstrates "is of use only for human users" is wrong.

> Presumably we don't want to print text in slirp only to parse it in qemu,
> right?

Yes, we'd prefer not to parse.

As long as libslirp can only give us text, we need to parse it
somewhere.

We can parse it right in QEMU, or punt the job to whatever uses QEMU.
The latter can get away with parsing just the part they need.  But we
may end up with multiple parsers.


> That'll change the qemu/slirp interface.
> OTOH, to what extent does libslirp want to export a more formal API for
> this, vs just text?

This is a question for Samuel or Marc-André.

If the answer is "no", then HMP only (so we don't have to parse in QEMU)
is fair, I think.  The commit message should explain this.



  reply	other threads:[~2021-09-04  6:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-02 21:20 [PATCH] net: Add "info neighbors" command Doug Evans
2021-09-02 22:08 ` Patrick Venture
2021-09-03  4:28 ` Markus Armbruster
2021-09-03 16:53   ` Doug Evans
2021-09-04  6:26     ` Markus Armbruster [this message]
2021-09-16 11:11       ` Marc-André Lureau

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=87lf4crh4s.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=dje@google.com \
    --cc=jasowang@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=philmd@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.