Re: [Qemu-devel] [PATCH 1/3] qemu: add capability for vhost-net busy polling

Re: [Qemu-devel] [PATCH 1/3] qemu: add capability for vhost-net busy polling

[Eduardo Habkost]

(CCing qemu-devel and the QAPI maintainers.  I have a question
about introspection below.)

On Sat, Jun 17, 2017 at 11:48:10AM -0400, Laine Stump wrote:
[...]
> > +    /* Support for busy polling on vhost-net devices ("-netdev
> > +     * tap,...,poll-us=n") */
> > +    if (qemuCaps->version >= 2007000)
> > +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_VHOST_NET_POLL_US);
> > +
> 
> Enabling a capability based purely on the qemu binary version is
> something that's only done as a last resort if it can't be determined by
> QMP probes. This is because the version number is an inaccurate
> indicator - it often happens that a new capability is added to upstream
> in qemu version "Y", but then that feature is backported to qemu version
> "X" in some downstream distro package; if we based the capability on
> version number, then libvirt wouldn't take advantage of the feature even
> though qemu had it.
> 
> 
> What *should* get us the info we need is to add a table entry to
> virQEMUCapsCommandLine to check whether or not the poll-us option exists
> in the current qemu binary. Something like this:
> 
> static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = {
>     ...
>     { "netdev", "poll-us", QEMU_CAPS_NETDEV_POLL_US },
> 
> 
> Unfortunately when I add that and regenerate the .replies file for qemu
> 2.9.0 (with the command "tests/qemucapsprobe
> /usr/bin/qemu-system-x86_64"), this is the info it finds for netdev:
> 
>     {
>       "parameters": [
>       ],
>       "option": "netdev"
>     },
> 
> My understanding was that the possible parameters to the netdev
> commandline option should be listed, but instead the parameters list is
> empty :-/.
> 
> Maybe Eduardo can help me understand why the parameters are empty, and
> (more importantly) what needs to be done to get the supported parameters
> for -netdev - basically we need to know what are the available
> parameters to "-netdev tap,...". I hope we don't end up needing to base
> the check purely on qemu version.

query-command-line-options is incomplete and useless on many
cases (including but not limited to -device, -object, and
-netdev).  It is not capable to model cases where the set of
accepted options depends on a discriminator option (e.g. a
"driver" or "type" option).

Fortunately, netdev options are modelled in the QAPI schema as
union Netdev.  However, 'query-qmp-schema' doesn't seem to
include union Netdev because it is not referenced by any QMP
command or event.

Markus, Eric, Michael: is there any way libvirt can query the
definition of union Netdev from the schema with current QEMU?

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/3] qemu: add capability for vhost-net busy polling

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1157 bytes --]

On 06/17/2017 09:15 PM, Eduardo Habkost wrote:
> (CCing qemu-devel and the QAPI maintainers.  I have a question
> about introspection below.)
> 

> 
> Fortunately, netdev options are modelled in the QAPI schema as
> union Netdev.  However, 'query-qmp-schema' doesn't seem to
> include union Netdev because it is not referenced by any QMP
> command or event.

I've posted patches in the past (qemu 2.6 timeframe, if I recall) that
changed netdev_add into a fully-advertised interface, but we didn't take
it then because we weren't sure how to handle the fact that netdev_add
can currently accept both an integer (1) and a string ("1") as
identical, and we didn't want to risk breaking clients that passed a
string when the promoted command would only accept integers.  I guess we
should revive that.

> 
> Markus, Eric, Michael: is there any way libvirt can query the
> definition of union Netdev from the schema with current QEMU?

Really, we need to promote netdev_add to a full-fledged QMP command.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 1/3] qemu: add capability for vhost-net busy polling

[Eduardo Habkost]

On Mon, Jun 26, 2017 at 07:23:50AM -0600, Eric Blake wrote:
> On 06/17/2017 09:15 PM, Eduardo Habkost wrote:
> > (CCing qemu-devel and the QAPI maintainers.  I have a question
> > about introspection below.)
> > 
> 
> > 
> > Fortunately, netdev options are modelled in the QAPI schema as
> > union Netdev.  However, 'query-qmp-schema' doesn't seem to
> > include union Netdev because it is not referenced by any QMP
> > command or event.
> 
> I've posted patches in the past (qemu 2.6 timeframe, if I recall) that
> changed netdev_add into a fully-advertised interface, but we didn't take
> it then because we weren't sure how to handle the fact that netdev_add
> can currently accept both an integer (1) and a string ("1") as
> identical, and we didn't want to risk breaking clients that passed a
> string when the promoted command would only accept integers.  I guess we
> should revive that.
> 
> > 
> > Markus, Eric, Michael: is there any way libvirt can query the
> > definition of union Netdev from the schema with current QEMU?
> 
> Really, we need to promote netdev_add to a full-fledged QMP command.

This may be enough for -netdev, but do we want to make command-line
option introspection always depend on having the same struct being used
in a QMP command to work?  What if the arguments for the command-line
option don't match precisely the input of a QMP command?

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/3] qemu: add capability for vhost-net busy polling

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2178 bytes --]

On 06/26/2017 09:24 PM, Eduardo Habkost wrote:
>> I've posted patches in the past (qemu 2.6 timeframe, if I recall) that
>> changed netdev_add into a fully-advertised interface, but we didn't take
>> it then because we weren't sure how to handle the fact that netdev_add
>> can currently accept both an integer (1) and a string ("1") as
>> identical, and we didn't want to risk breaking clients that passed a
>> string when the promoted command would only accept integers.  I guess we
>> should revive that.
>>
>>>
>>> Markus, Eric, Michael: is there any way libvirt can query the
>>> definition of union Netdev from the schema with current QEMU?
>>
>> Really, we need to promote netdev_add to a full-fledged QMP command.

Remember, netdev_add and object_add are the only two outlier QMP
commands that have NOT been converted to use QAPI.  We've already done
most of the work at making QMP netdev_add and command line -netdev be
compatible, it is just the last bit of switching on full QAPI support
that has some interface ramifications that we were not comfortable
making during a release freeze, and haven't had time to revisit since.
object_add is much further away from being introspectible.

> 
> This may be enough for -netdev, but do we want to make command-line
> option introspection always depend on having the same struct being used
> in a QMP command to work?  What if the arguments for the command-line
> option don't match precisely the input of a QMP command?

Then that's arguably a poor user interface, and only something we should
do for back-compat reasons, and not for new design. We WANT to make the
command-line introspectible, and the easiest way to do that is to make
the command-line and QMP share common guts (see the recent -blockdev
addition in qemu 2.9, along with Dan and Markus' work at tweaking
visitors so that the command line directly feeds into the QObject
representation used by QMP commands rather than the hackish QemuOpts
that is already too bloated with workarounds).

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]