Re: [PATCH] vl, qemu-config: remove -set
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
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
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
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
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
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 :|