All of lore.kernel.org
 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 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.