qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Marcel Apfelbaum <marcel@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Tony Krowiak" <akrowiak@linux.vnet.ibm.com>,
	qemu-devel@nongnu.org, "Andreas Färber" <afaerber@suse.de>
Subject: Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
Date: Wed, 01 Apr 2015 17:51:28 +0300	[thread overview]
Message-ID: <551C05F0.3090502@redhat.com> (raw)
In-Reply-To: <87oan8z1yw.fsf@blackfin.pond.sub.org>

On 04/01/2015 11:28 AM, Markus Armbruster wrote:
> Marcel Apfelbaum <marcel.apfelbaum@gmail.com> writes:
>
>> On 03/31/2015 05:21 PM, Tony Krowiak wrote:
>>> Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 removed the QemuOptDesc elements from the
>>> *desc* field of the *qemu_machine_opts *array defined in vl.c.  Since applying that patch to qemu
>>> on my system, I can not start a guest from libvirt when certain machine options are configured
>>> for the guest domain.  For example, if I configure the following for my guest domain:
>>>
>>>       <memoryBacking>
>>>           ...
>>>           <nosharepages>
>>>           ...
>>>       </memoryBacking>
>>>
>>> I get the following libvirt error when I try to start the guest:
>>>
>>>       error: unsupported configuration: disable shared memory is not available with this QEMU binary
>>>
>>> The *nosharepages *element generates the *-machine* option *mem-merge=off* on the QEMU command line.  The error is
>>> thrown by libvirt because the QMP *query-command-line-options* command does not return *mem-merge* in the machine
>>> options parameter list.  In fact, if I issue the *query-command-line-options* command via virsh as follows:
>>>
>>>       virsh qemu-monitor-command guest_c2aa '{ "execute": "query-command-line-options", "arguments": { "option": "machine" } }'
>>>
>> Hi Tony,
>> Thank you for finding this bug.
>
> Sounds like a regression.  If it is, we need to decide what to do about
> it urgently.
Hi Markus,
This is definitely a regression.

>
>>> No machine option parameters are returned:
>>>
>>> {"return":[{"parameters":[],"option":"machine"}],"id":"libvirt-11"}
>> Indeed, we have a problem here.
>>
>> This is the first object for which QemuOps are defined per
>> sub-type and are not global (if you don't take "object" under consideration).
>> I saw others as well, like netdev, but I am not sure what happens there.
>>
>> Once the QemuOpts are parsed, the only place we can find those options
>> is the machine object itself (as QOM properties).
>>
>> I see a few options here:
>> 1. Add a feature to QemuOpts: "Look for options in QOM properties of this obj"
>
> QemuOpts is an overengineered, self-contained mess.  Let's not make it
> an overengineered mess with complex external dependencies.
>
>> 2. Add a callback to QEMU opts that supplies the options (have machine
>> supply the callback)
>
> Keeps QemuOpts and QOM more separated than 1, but still adds external
> dependencies.
>
>> 3. Have the machine object fill in the corresponding QemuOpts on init.
>
> Monkey-patching QemuOpts desc[] should be workable in principle.
>
> However, to monkey-patch qemu_machine_opts.desc[], we need the machine
> object, and to create the machine object, we need to parse machine
> options.  Thus, we'll first parse with an empty desc[], then make one up
> and monkey-patch it in just for introspection.  Nasty.

I noticed something weird. I cannot actually create an instance of machine
or get a reference to current_machine in order to query its properties!

It seems that util/qemu-config is used by qemu-img which obviously
does not have a current machine nor the means to create it.

So I have no way to create QOM objects for introspection :(.

>
> "Nasty" may well be what we need to fix the regression at this late
> hour.

I don't like it either but if 1. and 2. are worse, I posted a patch for 3. ish.

>
>> Any thoughts?
> [...]
>
> Yes, but you may not like them :)

Thanks  for the ideas!
Now I'll start reading...

>
> 4. Support tagged unions in QemuOpts
>
> QemuOpts supports a single list of typed parameters.  Good enough for
> many options.  Certain options, however, additionally take "variant"
> paramaters depending on the value of a discriminator parameter.
>
> Example: -tpmdev id=ID,type=T,...
>
>      type=T selects a TPM backend, which defines additional option
>      parameters.
>
>      Current solution: qemu_tpmdev_opts.desc[] is empty.  Option parsing
>      accepts arbitrary parameters unchecked in addition to the special
>      parameter id=ID.  configure_tpm() gets parameter "type", finds the
>      backend, then passes the backend's QemuOptsDesc[] to
>      qemu_opts_validate() to check parameters.
>
>      How configure_tpm() validates parameters is not visible to
>      query-command-line-options, naturally.
>
> Example: -device id=ID,driver=D,bus=B,...
>
>      driver=D selects a device model, which defines additional option
>      parameters.
>
>      Current solution: the device model defines QOM properties,
>      qemu_device_opts.desc[] is empty.  Option parsing accepts arbitrary
>      parameters unchecked in addition to the special parameter id=ID.
>      qdev_device_add() gets parameter "driver" and "bus", finds the
>      driver, then feeds the remaining option parameters to
>      object_property_parse() to check and set them.
>
>      How qdev_device_add() validates parameters is not visible to
>      query-command-line-options, naturally.  But libvirt knows what it
>      does, and finds the QOM properties elsewhere (QMP command
>      device-list-properties).
>
>      Related: QMP command device_add has not been QAPIfied.  We'll get
>      back to that in a jiffie.
>
> Example: -netdev id=ID,type=T,...
>
>      type=T selects a net backend, which defines additional option
>      parameters.
>
>      Current solution: qemu_netdev_opts.desc[] is empty.  Option parsing
>      accepts arbitrary parameters unchecked in addition to the special
>      parameter id=ID.  The QAPI schema defines type NetClientOptions as a
>      tagged union.  net_client_init() uses OptsVisitor to check
>      parameters and create a NetClientOptions object for them.
>
>      How net_client_init() validates parameters is not visible to
>      query-command-line-options, naturally.
>
>      We could do better in QMP, but we don't: netdev_add doesn't use
>      NetClientOptions, it uses the top type '**', which makes the QMP
>      core accept an arbitrary JSON value.  This is then converted to
>      QemuOpts and fed to the machinery described above.
>
>      Creating new infrastructure is exciting, converting the first 90% of
>      its users proves its worth, converting the other 90% is boring and
>      hard, so let's create something new and more exciting instead.
>
> The -netdev example shows that the QAPI schema already has what we need.
> QMP gets it for free, because it's based on QAPI (except the parts we
> can't be bothered to convert).
>
> We could do the same for command line options.  Would additionally get
> us other QAPI goodies, like a saner type system, and (soon)
> introspection.
>
> Big job, though.
You lost me... you are talking about QAPI that I have no knowledge about,
and I still don't see how I can create instances of QOM objects in the context
of qemu-config.

>
> We could of course hack up QemuOpts some more to make it support tagged
> unions all by itself, duplicating selected parts of QAPI.  Very
> traditional.
>
> 5. Introspect something else
>
> Remember the -device example?  There, query-command-line-options is of
> no help, so we find the information somewhere else.
-device is also looking into a static array, no introspection :(

>
> Adding an ad hoc "somewhere else" just for -machine would also be very
> traditional.
Thanks for the help!
Marcel

>
> [...]
>

  reply	other threads:[~2015-04-01 14:51 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-31 14:21 [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary Tony Krowiak
2015-04-01  6:54 ` Marcel Apfelbaum
2015-04-01  8:01   ` Paolo Bonzini
2015-04-01  8:06     ` Marcel Apfelbaum
2015-04-01  8:20       ` Paolo Bonzini
2015-04-01  8:42     ` Markus Armbruster
2015-04-01  9:07       ` Paolo Bonzini
2015-04-01  9:14         ` Marcel Apfelbaum
2015-04-01  9:23           ` Paolo Bonzini
2015-04-01  9:27             ` Marcel Apfelbaum
2015-04-01  8:28   ` Markus Armbruster
2015-04-01 14:51     ` Marcel Apfelbaum [this message]
2015-04-01 15:53       ` Markus Armbruster
2015-04-01 16:11         ` Marcel Apfelbaum
2015-04-01 16:20           ` Eric Blake
2015-04-01 16:31             ` Marcel Apfelbaum

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=551C05F0.3090502@redhat.com \
    --to=marcel@redhat.com \
    --cc=afaerber@suse.de \
    --cc=akrowiak@linux.vnet.ibm.com \
    --cc=armbru@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.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).