Re: [PATCH] vl, qemu-config: remove -set

2020-11-12 Thread Paolo Bonzini

On 12/11/20 12:26, Gerd Hoffmann wrote:

Yes, that is quite useful for setting device properties which are not
(yet) supported by libvirt, like this:

   
 
 
   

Grepping through my libvirt domain config files I see 90% is
indeed "-set device".  But I've also found netdev (set tftp+bootfile
for "").


Hmm...  Looks like I will move -set from config-file to vl.c and handle 
both QemuOpts and keyval-based options.


Paolo




Re: [PATCH] vl, qemu-config: remove -set

2020-11-12 Thread Gerd Hoffmann
  Hi,

> IOW, it looks like it is valid to use -set, even if you're not using
> -readconfig.

Yes, that is quite useful for setting device properties which are not
(yet) supported by libvirt, like this:

  


  

Grepping through my libvirt domain config files I see 90% is
indeed "-set device".  But I've also found netdev (set tftp+bootfile
for "").

take care,
  Gerd




Re: [PATCH] vl, qemu-config: remove -set

2020-11-11 Thread Paolo Bonzini
Il gio 12 nov 2020, 07:55 Markus Armbruster  ha scritto:

> Immediate removal of -set / rejection of -set for some option groups
> needs more justification than just "I think we can get away with it":
> there has to be a tangible benefit.


The benefit is being able to switch parsers (for everything but -device to
begin with) away from QemuOpts. Non-scalar priorities have a use for
machine, accel and object, but for now not for device. Alternatively I can:

1) keep QemuOpts as the parser, and turn the options into qdict, instead of
using keyval

2) move -set out of config-file.c and into vl.c, so that I can special case
the monkey patching.

What would immediate removal buy us
> over the orthodox "deprecate, wait for grace period to expire, remove"?
>

The problem is that, at least for -device, I don't think we can reasonably
remove it without a replacement.

Paolo


>


Re: [PATCH] vl, qemu-config: remove -set

2020-11-11 Thread Markus Armbruster
Paolo Bonzini  writes:

> On 11/11/20 16:03, Daniel P. Berrangé wrote:
>> On Wed, Nov 11, 2020 at 08:57:16AM -0500, Paolo Bonzini wrote:
>>> -set as far as I can see has basically no use.  It was intended as an 
>>> override
>>> mechanism for configuration files, but even configuration files themselves
>>> are hardly used.  Drop it with prejudice.
>>> 
>>> Signed-off-by: Paolo Bonzini 
>>> ---
>>>  docs/system/deprecated.rst |  6 ++
>>>  include/qemu/config-file.h |  1 -
>>>  qemu-options.hx|  9 -
>>>  softmmu/vl.c   |  4 
>>>  util/qemu-config.c | 33 -
>>>  5 files changed, 6 insertions(+), 47 deletions(-)
>>
>> iotest 068 uses -set and qtest vhost-user-text.c also does
>> IOW, it looks like it is valid to use -set, even if you're not using
>> -readconfig.

Of course that's valid.

>> Libvirt doesn't use -set, but we've had users who make use of
>> libvirt
>> command line passthrough for QEMU with -set.
>
> Hmm, indeed:
>
> https://patchwork.kernel.org/project/qemu-devel/patch/20181218041625.24969-16-...@redhat.com/

Such monkey-patching may not be wise, but unwise != invalid.

>> IOW, I'm not convinced real world usage is near zero as suggested.

Guessing the gamut of usage out there in the real world correctly is
always a tall order :)

> Yes, perhaps it's not. :)  Though for both tests you pointed out it's
> even cleaner not to use it, there seems to be real world usage at
> least with "device".

I have common test configurations files for -readconfig.  I've used -set
for quick monkey-patching once in a great while.  Now, such ad hoc use
is a *weak* argument against ditching the feature.  But it does
undermine the "basically no use" proposition.

> It is probably more viable to deprecate or even forbid usage of "-set"
> with anything but "device".  vhost-user-test.c would still be
> affected, but it's a relatively small patch.

Deprecating only some uses buys us next to nothing, I think.  If we want
to deprecate it, just deprecate it.

Immediate removal of -set / rejection of -set for some option groups
needs more justification than just "I think we can get away with it":
there has to be a tangible benefit.  What would immediate removal buy us
over the orthodox "deprecate, wait for grace period to expire, remove"?




Re: [PATCH] vl, qemu-config: remove -set

2020-11-11 Thread Paolo Bonzini

On 11/11/20 16:03, Daniel P. Berrangé wrote:

iotest 068 uses -set and qtest vhost-user-text.c also does

IOW, it looks like it is valid to use -set, even if you're not using
-readconfig.

Libvirt doesn't use -set, but we've had users who make use of libvirt
command line passthrough for QEMU with -set.


Hmm, indeed:

https://patchwork.kernel.org/project/qemu-devel/patch/20181218041625.24969-16-...@redhat.com/


IOW, I'm not convinced real world usage is near zero as suggested.


Yes, perhaps it's not. :)  Though for both tests you pointed out it's 
even cleaner not to use it, there seems to be real world usage at least 
with "device".


It is probably more viable to deprecate or even forbid usage of "-set" 
with anything but "device".  vhost-user-test.c would still be affected, 
but it's a relatively small patch.


Paolo




Re: [PATCH] vl, qemu-config: remove -set

2020-11-11 Thread Daniel P . Berrangé
On Wed, Nov 11, 2020 at 08:57:16AM -0500, Paolo Bonzini wrote:
> -set as far as I can see has basically no use.  It was intended as an override
> mechanism for configuration files, but even configuration files themselves
> are hardly used.  Drop it with prejudice.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  docs/system/deprecated.rst |  6 ++
>  include/qemu/config-file.h |  1 -
>  qemu-options.hx|  9 -
>  softmmu/vl.c   |  4 
>  util/qemu-config.c | 33 -
>  5 files changed, 6 insertions(+), 47 deletions(-)

iotest 068 uses -set and qtest vhost-user-text.c also does

IOW, it looks like it is valid to use -set, even if you're not using
-readconfig.

Libvirt doesn't use -set, but we've had users who make use of libvirt
command line passthrough for QEMU with -set.

IOW, I'm not convinced real world usage is near zero as suggested.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|