On 11/10/22 13:23, Thomas Huth wrote:
> On 10/11/2022 13.05, Claudio Fontana wrote:
>> On 11/10/22 12:42, Thomas Huth wrote:
>>> On 08/11/2022 10.49, Claudio Fontana wrote:
>>>> On 11/4/22 13:57, Thomas Huth wrote:
>>>>> Running QEMU with "-nic help" used to work in QEMU 5.2 and earlier
>>>>> versions
>>>>> (it showed the available netdev backends), but this feature got broken
>>>>> during
>>>>> some refactoring in version 6.0. Let's restore the old behavior, and while
>>>>> we're at it, let's also print the available NIC models here now since this
>>>>> option can be used to configure both, netdev backend and model in one go.
>>>>>
>>>>> Fixes: ad6f932fe8 ("net: do not exit on "netdev_add help" monitor
>>>>> command")
>>>>> Signed-off-by: Thomas Huth <th...@redhat.com>
>>>>> ---
>>>>> net/net.c | 14 ++++++++++++--
>>>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/net/net.c b/net/net.c
>>>>> index c0516a8067..b4b8f2a9cc 100644
>>>>> --- a/net/net.c
>>>>> +++ b/net/net.c
>>>>> @@ -1571,8 +1571,18 @@ static int net_param_nic(void *dummy, QemuOpts
>>>>> *opts, Error **errp)
>>>>> const char *type;
>>>>>
>>>>> type = qemu_opt_get(opts, "type");
>>>>> - if (type && g_str_equal(type, "none")) {
>>>>> - return 0; /* Nothing to do, default_net is cleared in vl.c */
>>>>> + if (type) {
>>>>> + if (g_str_equal(type, "none")) {
>>>>> + return 0; /* Nothing to do, default_net is cleared in
>>>>> vl.c */
>>>>> + }
>>>>> + if (is_help_option(type)) {
>>>>> + GPtrArray *nic_models = qemu_get_nic_models(TYPE_DEVICE);
>>>>> + show_netdevs();
>>>>> + printf("\n");
>>>>> + qemu_show_nic_models(type, (const char **)nic_models->pdata);
>>>>> + g_ptr_array_free(nic_models, true);
>>>>
>>>> nit: would not the order:
>>>>
>>>>> + GPtrArray *nic_models;
>>>>> + show_netdevs();
>>>>> + printf("\n");
>>>>> + nic_models = qemu_get_nic_models(TYPE_DEVICE);
>>>>> + qemu_show_nic_models(type, (const char **)nic_models->pdata);
>>>>> + g_ptr_array_free(nic_models, true);
>>>>
>>>> flow more logically?
>>>
>>> I think that's mostly a matter of taste ...
>>
>> To some extent, but for the reader it would make more sense not to intermix
>> unrelated code?
>
> I'm pretty sure that as soon as I change it, another reviewer
> shows up and asks me to put everything into one line again
> since they prefer more compact code ;-)
>
>> I'd say:
>>
>> - show_netdevs
>> _ get nic models
>> - show nic models
>>
>> instead of:
>>
>> - get nic models
>> - show netdevs
>> - show nic models
>
> I get your point, and I would immediately agree with you if we
> were allowed to do:
>
> show_netdevs();
> printf("\n");
> GPtrArray *nic_models = qemu_get_nic_models(TYPE_DEVICE);
> qemu_show_nic_models(type, (const char **)nic_models->pdata);
> g_ptr_array_free(nic_models, true);
>
> Although this is possible nowadays (since we're using
> not C89 anymore), it's against the QEMU coding style.
It seems you haven't seen the next message I sent. You can write exactly the
code above,
with valid C89 syntax,
which is:
show_netdevs();
printf("\n");
{
GPtrArray *nic_models = qemu_get_nic_models(TYPE_DEVICE);
qemu_show_nic_models(type, (const char **)nic_models->pdata);
g_ptr_array_free(nic_models, true);
}
>
> So it's a trade-off now - use two lines of code and have some more
> chronological code flow, or use one line of more compact code.
>
> I'm in favor of the more compact code. So please let's stop
> bike-shedding now.
>
> Thanks,
> Thomas
>
Ah, you don't like it, I see. Well.
Thanks,
Claudio
