* [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
@ 2015-03-31 14:21 Tony Krowiak
2015-04-01 6:54 ` Marcel Apfelbaum
0 siblings, 1 reply; 16+ messages in thread
From: Tony Krowiak @ 2015-03-31 14:21 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel.apfelbaum
[-- Attachment #1: Type: text/plain, Size: 1874 bytes --]
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" } }'
No machine option parameters are returned:
{"return":[{"parameters":[],"option":"machine"}],"id":"libvirt-11"}
I took a look at the *qmp_query_command_line_options* function in
*util/qemu-config.c*. The function derives the
option parameters to return with the the query response from the
QemuOptDesc elements contained in the
*desc* field of the *qemu_machine_opts *array defined in vl.c. It
appears that removing the
QemuOptDesc elements broke the *qmp_query_command_line_options*
function. If I restore the QemuOptDesc
elements removed by commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6, I
can start the guest with
*nosharepages* configured.
It would appear that a bug was introduced with commit
49d2e648e8087d154d8bf8b91f27c8e05e79d5a6,
what say you?
[-- Attachment #2: Type: text/html, Size: 2764 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
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:28 ` Markus Armbruster
0 siblings, 2 replies; 16+ messages in thread
From: Marcel Apfelbaum @ 2015-04-01 6:54 UTC (permalink / raw)
To: Tony Krowiak, qemu-devel; +Cc: Paolo Bonzini, Andreas Färber
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.
> 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"
2. Add a callback to QEMU opts that supplies the options (have machine supply the callback)
3. Have the machine object fill in the corresponding QemuOpts on init.
Any thoughts?
Thanks,
Marcel
>
>
> I took a look at the *qmp_query_command_line_options* function in *util/qemu-config.c*. The function derives the
> option parameters to return with the the query response from the QemuOptDesc elements contained in the
> *desc* field of the *qemu_machine_opts *array defined in vl.c. It appears that removing the
> QemuOptDesc elements broke the *qmp_query_command_line_options* function. If I restore the QemuOptDesc
> elements removed by commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6, I can start the guest with
> *nosharepages* configured.
>
> It would appear that a bug was introduced with commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6,
> what say you?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
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:42 ` Markus Armbruster
2015-04-01 8:28 ` Markus Armbruster
1 sibling, 2 replies; 16+ messages in thread
From: Paolo Bonzini @ 2015-04-01 8:01 UTC (permalink / raw)
To: marcel, Tony Krowiak, qemu-devel; +Cc: Andreas Färber
On 01/04/2015 08:54, Marcel Apfelbaum wrote:
> 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).
We can return the same QemuOpts that were included before.
Per-machine-type options are new and need not be covered by
query-command-line-options.
Paolo
> 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"
> 2. Add a callback to QEMU opts that supplies the options (have machine
> supply the callback)
> 3. Have the machine object fill in the corresponding QemuOpts on init.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
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
1 sibling, 1 reply; 16+ messages in thread
From: Marcel Apfelbaum @ 2015-04-01 8:06 UTC (permalink / raw)
To: Paolo Bonzini, Tony Krowiak, qemu-devel; +Cc: Andreas Färber
On 04/01/2015 11:01 AM, Paolo Bonzini wrote:
>
>
> On 01/04/2015 08:54, Marcel Apfelbaum wrote:
>> 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).
>
> We can return the same QemuOpts that were included before.
> Per-machine-type options are new and need not be covered by
> query-command-line-options.
OK, we have them under hw/core/machine.c as "base" machine properties.
We still need a way to fill them back into QemuOpts right?
Maybe return them to the static global list? It seems like a step
back, but if there is no better way...
Thanks,
Marcel
>
> Paolo
>
>> 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"
>> 2. Add a callback to QEMU opts that supplies the options (have machine
>> supply the callback)
>> 3. Have the machine object fill in the corresponding QemuOpts on init.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
2015-04-01 8:06 ` Marcel Apfelbaum
@ 2015-04-01 8:20 ` Paolo Bonzini
0 siblings, 0 replies; 16+ messages in thread
From: Paolo Bonzini @ 2015-04-01 8:20 UTC (permalink / raw)
To: Marcel Apfelbaum, Tony Krowiak, qemu-devel; +Cc: Andreas Färber
On 01/04/2015 10:06, Marcel Apfelbaum wrote:
>>
>> We can return the same QemuOpts that were included before.
>> Per-machine-type options are new and need not be covered by
>> query-command-line-options.
>
> OK, we have them under hw/core/machine.c as "base" machine properties.
> We still need a way to fill them back into QemuOpts right?
> Maybe return them to the static global list? It seems like a step
> back, but if there is no better way...
Yeah, QemuOpts needs to be enhanced to stop doing checks on -machine.
Paolo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
2015-04-01 6:54 ` Marcel Apfelbaum
2015-04-01 8:01 ` Paolo Bonzini
@ 2015-04-01 8:28 ` Markus Armbruster
2015-04-01 14:51 ` Marcel Apfelbaum
1 sibling, 1 reply; 16+ messages in thread
From: Markus Armbruster @ 2015-04-01 8:28 UTC (permalink / raw)
To: marcel; +Cc: Paolo Bonzini, Tony Krowiak, qemu-devel, Andreas Färber
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.
>> 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.
"Nasty" may well be what we need to fix the regression at this late
hour.
> Any thoughts?
[...]
Yes, but you may not like them :)
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.
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.
Adding an ad hoc "somewhere else" just for -machine would also be very
traditional.
[...]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
2015-04-01 8:01 ` Paolo Bonzini
2015-04-01 8:06 ` Marcel Apfelbaum
@ 2015-04-01 8:42 ` Markus Armbruster
2015-04-01 9:07 ` Paolo Bonzini
1 sibling, 1 reply; 16+ messages in thread
From: Markus Armbruster @ 2015-04-01 8:42 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: marcel, Tony Krowiak, qemu-devel, Andreas Färber
Paolo Bonzini <pbonzini@redhat.com> writes:
> On 01/04/2015 08:54, Marcel Apfelbaum wrote:
>> 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).
>
> We can return the same QemuOpts that were included before.
> Per-machine-type options are new and need not be covered by
> query-command-line-options.
The obvious way to return them is to put them right back in
qemu_machine_opts.desc[]. But then -machine rejects machine-specific
parameters.
Hack: monkey-patch them in after we're done parsing.
Cleaner: "empty desc[] means accept anything" has always been overly
restrictive. Have a flag "accept additional parameters".
We may have to do the former for 2.3, but that's no excuse not to
replace it by something less gross in 2.4.
[...]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
2015-04-01 8:42 ` Markus Armbruster
@ 2015-04-01 9:07 ` Paolo Bonzini
2015-04-01 9:14 ` Marcel Apfelbaum
0 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2015-04-01 9:07 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcel, Tony Krowiak, qemu-devel, Andreas Färber
On 01/04/2015 10:42, Markus Armbruster wrote:
> The obvious way to return them is to put them right back in
> qemu_machine_opts.desc[]. But then -machine rejects machine-specific
> parameters.
>
> Hack: monkey-patch them in after we're done parsing.
>
> Cleaner: "empty desc[] means accept anything" has always been overly
> restrictive. Have a flag "accept additional parameters".
>
> We may have to do the former for 2.3, but that's no excuse not to
> replace it by something less gross in 2.4.
The latter sounds less intrusive, actually. Could it be as easy as
static bool opts_accepts_any(const QemuOpts *opts)
{
- return opts->list->desc[0].name == NULL;
+ return opts->list->desc[0].name == NULL || opts->list->accept_any;
}
?
Paolo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
2015-04-01 9:07 ` Paolo Bonzini
@ 2015-04-01 9:14 ` Marcel Apfelbaum
2015-04-01 9:23 ` Paolo Bonzini
0 siblings, 1 reply; 16+ messages in thread
From: Marcel Apfelbaum @ 2015-04-01 9:14 UTC (permalink / raw)
To: Paolo Bonzini, Markus Armbruster
Cc: Tony Krowiak, qemu-devel, Andreas Färber
On 04/01/2015 12:07 PM, Paolo Bonzini wrote:
>
>
> On 01/04/2015 10:42, Markus Armbruster wrote:
>> The obvious way to return them is to put them right back in
>> qemu_machine_opts.desc[]. But then -machine rejects machine-specific
>> parameters.
>>
>> Hack: monkey-patch them in after we're done parsing.
>>
>> Cleaner: "empty desc[] means accept anything" has always been overly
>> restrictive. Have a flag "accept additional parameters".
>>
>> We may have to do the former for 2.3, but that's no excuse not to
>> replace it by something less gross in 2.4.
>
> The latter sounds less intrusive, actually. Could it be as easy as
>
> static bool opts_accepts_any(const QemuOpts *opts)
> {
> - return opts->list->desc[0].name == NULL;
> + return opts->list->desc[0].name == NULL || opts->list->accept_any;
> }
>
> ?
This + 'monkey-patch' may be a feasible solution for 2.4
Thanks, I'll give it a try and see how ugly will be
Marcel
>
> Paolo
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
2015-04-01 9:14 ` Marcel Apfelbaum
@ 2015-04-01 9:23 ` Paolo Bonzini
2015-04-01 9:27 ` Marcel Apfelbaum
0 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2015-04-01 9:23 UTC (permalink / raw)
To: Marcel Apfelbaum, Markus Armbruster
Cc: Tony Krowiak, qemu-devel, Andreas Färber
On 01/04/2015 11:14, Marcel Apfelbaum wrote:
> This + 'monkey-patch' may be a feasible solution for 2.4
Why monkey-patch and not just revert?
Paolo
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
2015-04-01 9:23 ` Paolo Bonzini
@ 2015-04-01 9:27 ` Marcel Apfelbaum
0 siblings, 0 replies; 16+ messages in thread
From: Marcel Apfelbaum @ 2015-04-01 9:27 UTC (permalink / raw)
To: Paolo Bonzini, Markus Armbruster
Cc: Tony Krowiak, qemu-devel, Andreas Färber
On 04/01/2015 12:23 PM, Paolo Bonzini wrote:
>
>
> On 01/04/2015 11:14, Marcel Apfelbaum wrote:
>> This + 'monkey-patch' may be a feasible solution for 2.4
>
> Why monkey-patch and not just revert?
There are already several machine sub-types that have
their own options, some of the code I think
it was added after the 'dynamic options' patch, so revert will
not be enough.
We will need to go over the new options and add them to the global
list.
At this point, adding the machine options to the global list during
machine init seems to be a less risky and
will also keep options per machine.
Maybe I'll try it and see how it looks.
Thanks,
Marcel
>
> Paolo
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
2015-04-01 8:28 ` Markus Armbruster
@ 2015-04-01 14:51 ` Marcel Apfelbaum
2015-04-01 15:53 ` Markus Armbruster
0 siblings, 1 reply; 16+ messages in thread
From: Marcel Apfelbaum @ 2015-04-01 14:51 UTC (permalink / raw)
To: Markus Armbruster
Cc: Paolo Bonzini, Tony Krowiak, qemu-devel, Andreas Färber
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
>
> [...]
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
2015-04-01 14:51 ` Marcel Apfelbaum
@ 2015-04-01 15:53 ` Markus Armbruster
2015-04-01 16:11 ` Marcel Apfelbaum
0 siblings, 1 reply; 16+ messages in thread
From: Markus Armbruster @ 2015-04-01 15:53 UTC (permalink / raw)
To: Marcel Apfelbaum
Cc: Paolo Bonzini, Tony Krowiak, qemu-devel, Andreas Färber
Marcel Apfelbaum <marcel@redhat.com> writes:
> 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 :(.
You'd have to do something like
desc[] = generic entries + the machine's entries
where the latter is empty outside qemu proper.
For 2.3, I recommend to do *only* generic entries. Specifically,
*exactly* the entries we had before we cleared out
qemu_machine_opts.desc[].
>> "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.
Very high level summary:
0. We use QemuOpts to define our command line. The definition is
*incomplete*. The missing parts are left to code.
query-command-line-options can't see them.
1. You have a QemuOpts problem that is actually pretty common: how to
accept a few fixed parameters plus a bunch of parameters that are
specific to the value of one of the fixed parameters (the
discriminator, in your case "type").
2. We've solved this in several different ways, and all of them make
query-command-line-options useless.
3. Same problem exists in QMP, and we have a decent solution there,
based on QAPI.
4. We'll soon have QMP/QAPI introspection.
5. If we used a QAPI schema to define our command line, we could do a
more complete job (because it's more expressive), and we'd get
introspection basically for free.
6. #5 would be a big job, though.
Less confused now?
>> 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 :(
Let me rephrase:
query-command-line-options comes up empty for -device.
Libvirt knows that -device takes parameters "id", "driver" and
driver-specific ones. It further knows how to get the driver-specific
ones: with QMP command device-list-properties. Example:
{ "execute": "device-list-properties", "arguments": { "typename": "usb-mouse" } }
{"return": [{"name": "msos-desc", "description": "on/off", "type": "bool"}, {"name": "full-path", "description": "on/off", "type": "bool"}, {"name": "serial", "type": "str"}, {"name": "port", "type": "str"}, {"name": "usb_version", "type": "uint32"}]}
>> Adding an ad hoc "somewhere else" just for -machine would also be very
>> traditional.
> Thanks for the help!
> Marcel
>
>>
>> [...]
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
2015-04-01 15:53 ` Markus Armbruster
@ 2015-04-01 16:11 ` Marcel Apfelbaum
2015-04-01 16:20 ` Eric Blake
0 siblings, 1 reply; 16+ messages in thread
From: Marcel Apfelbaum @ 2015-04-01 16:11 UTC (permalink / raw)
To: Markus Armbruster
Cc: Paolo Bonzini, Tony Krowiak, qemu-devel, Andreas Färber
On 04/01/2015 06:53 PM, Markus Armbruster wrote:
> Marcel Apfelbaum <marcel@redhat.com> writes:
[...]
>> 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 :(.
>
> You'd have to do something like
>
> desc[] = generic entries + the machine's entries
>
> where the latter is empty outside qemu proper.
Hmm! So I will loose with some dignity.
I'll keep the properties of the base "machine" on a static array
and *only* per-machine properties dynamic and I loose them.
>
> For 2.3, I recommend to do *only* generic entries. Specifically,
> *exactly* the entries we had before we cleared out
> qemu_machine_opts.desc[].
I submitted:
[PATCH for-2.3] util/qemu-config" fix regression of qmp_query_command_line_options
which includes both base-machine/per-machine properties.
Is it that bad? qmp can query it and even the new options will work
if qmp decides to set them. Can you have a look?
>
[...]
>>> 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.
>
> Very high level summary:
>
> 0. We use QemuOpts to define our command line. The definition is
> *incomplete*. The missing parts are left to code.
> query-command-line-options can't see them.
OK
>
> 1. You have a QemuOpts problem that is actually pretty common: how to
> accept a few fixed parameters plus a bunch of parameters that are
> specific to the value of one of the fixed parameters (the
> discriminator, in your case "type").
Yes, but is more than that:
per-type properties are not static, you cannot find them before creating
an actual QOM object, and that is not possible.
We could have a per-machine static options array that will be loaded
at init time into object properties... ugly.
>
> 2. We've solved this in several different ways, and all of them make
> query-command-line-options useless.
Got it
>
> 3. Same problem exists in QMP, and we have a decent solution there,
> based on QAPI.
OK
>
> 4. We'll soon have QMP/QAPI introspection.
And then we can query a 'living' object's properties
>
> 5. If we used a QAPI schema to define our command line, we could do a
> more complete job (because it's more expressive), and we'd get
> introspection basically for free.
If the QAPI schema will include *all* properties *per* machine type, sure.
>
> 6. #5 would be a big job, though.
>
> Less confused now?
Much better, thanks!
Marcel
> [...]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
2015-04-01 16:11 ` Marcel Apfelbaum
@ 2015-04-01 16:20 ` Eric Blake
2015-04-01 16:31 ` Marcel Apfelbaum
0 siblings, 1 reply; 16+ messages in thread
From: Eric Blake @ 2015-04-01 16:20 UTC (permalink / raw)
To: Marcel Apfelbaum, Markus Armbruster
Cc: Paolo Bonzini, Tony Krowiak, qemu-devel, Andreas Färber
[-- Attachment #1: Type: text/plain, Size: 2738 bytes --]
On 04/01/2015 10:11 AM, Marcel Apfelbaum wrote:
> On 04/01/2015 06:53 PM, Markus Armbruster wrote:
>> Marcel Apfelbaum <marcel@redhat.com> writes:
> [...]
>>> 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 :(.
>>
>> You'd have to do something like
>>
>> desc[] = generic entries + the machine's entries
>>
>> where the latter is empty outside qemu proper.
> Hmm! So I will loose with some dignity.
> I'll keep the properties of the base "machine" on a static array
> and *only* per-machine properties dynamic and I loose them.
>
>>
>> For 2.3, I recommend to do *only* generic entries. Specifically,
>> *exactly* the entries we had before we cleared out
>> qemu_machine_opts.desc[].
> I submitted:
> [PATCH for-2.3] util/qemu-config" fix regression of
> qmp_query_command_line_options
> which includes both base-machine/per-machine properties.
> Is it that bad? qmp can query it and even the new options will work
> if qmp decides to set them. Can you have a look?
The problem is that the per-machine properties are ALSO advertised even
on machines where they do not work, which means you could be lying to
libvirt if it needs to know if a specific per-machine option is present.
It would indeed be more conservative for 2.3 to advertise ONLY the
generic options, so even though I already reviewed your patch, you may
want to respin to incorporate the more conservative approach by dropping
the advertising of any machine-specific option (as that is no worse than
what we had before - better to not advertise a feature than to advertise
something we don't actually support).
>> 1. You have a QemuOpts problem that is actually pretty common: how to
>> accept a few fixed parameters plus a bunch of parameters that are
>> specific to the value of one of the fixed parameters (the
>> discriminator, in your case "type").
> Yes, but is more than that:
> per-type properties are not static, you cannot find them before creating
> an actual QOM object, and that is not possible.
>
> We could have a per-machine static options array that will be loaded
> at init time into object properties... ugly.
But we can avoid worrying about the ugliness or alternatives for solving
that until 2.4. For 2.3, all we need to focus on is avoiding the
regression.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [qemu devel] disable shared memory is not available with this QEMU binary
2015-04-01 16:20 ` Eric Blake
@ 2015-04-01 16:31 ` Marcel Apfelbaum
0 siblings, 0 replies; 16+ messages in thread
From: Marcel Apfelbaum @ 2015-04-01 16:31 UTC (permalink / raw)
To: Eric Blake, Markus Armbruster
Cc: Paolo Bonzini, Tony Krowiak, qemu-devel, Andreas Färber
On 04/01/2015 07:20 PM, Eric Blake wrote:
> On 04/01/2015 10:11 AM, Marcel Apfelbaum wrote:
>> On 04/01/2015 06:53 PM, Markus Armbruster wrote:
>>> Marcel Apfelbaum <marcel@redhat.com> writes:
>> [...]
>>>> 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 :(.
>>>
>>> You'd have to do something like
>>>
>>> desc[] = generic entries + the machine's entries
>>>
>>> where the latter is empty outside qemu proper.
>> Hmm! So I will loose with some dignity.
>> I'll keep the properties of the base "machine" on a static array
>> and *only* per-machine properties dynamic and I loose them.
>>
>>>
>>> For 2.3, I recommend to do *only* generic entries. Specifically,
>>> *exactly* the entries we had before we cleared out
>>> qemu_machine_opts.desc[].
>> I submitted:
>> [PATCH for-2.3] util/qemu-config" fix regression of
>> qmp_query_command_line_options
>> which includes both base-machine/per-machine properties.
>> Is it that bad? qmp can query it and even the new options will work
>> if qmp decides to set them. Can you have a look?
>
> The problem is that the per-machine properties are ALSO advertised even
> on machines where they do not work, which means you could be lying to
> libvirt if it needs to know if a specific per-machine option is present.
> It would indeed be more conservative for 2.3 to advertise ONLY the
> generic options, so even though I already reviewed your patch, you may
> want to respin to incorporate the more conservative approach by dropping
> the advertising of any machine-specific option (as that is no worse than
> what we had before - better to not advertise a feature than to advertise
> something we don't actually support).
OK I'll send it shortly
Thanks,
Marcel
>
>
>>> 1. You have a QemuOpts problem that is actually pretty common: how to
>>> accept a few fixed parameters plus a bunch of parameters that are
>>> specific to the value of one of the fixed parameters (the
>>> discriminator, in your case "type").
>> Yes, but is more than that:
>> per-type properties are not static, you cannot find them before creating
>> an actual QOM object, and that is not possible.
>>
>> We could have a per-machine static options array that will be loaded
>> at init time into object properties... ugly.
>
> But we can avoid worrying about the ugliness or alternatives for solving
> that until 2.4. For 2.3, all we need to focus on is avoiding the
> regression.
>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-04-01 16:31 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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
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).