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 <thuth@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);
+ exit(0);
+ }
}
idx = nic_get_free_idx();
--
2.31.1On 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 <thuth@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?
Ciao
C
> + exit(0);
> + }
> }
>
> idx = nic_get_free_idx();
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 <thuth@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 ... and as long as the declaration
of the variable has to stay at the top of the block (according to QEMU's
coding style), I think I'd also prefer to keep the initialization there.
Thomas
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 <thuth@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'd say:
- show_netdevs
_ get nic models
- show nic models
instead of:
- get nic models
- show netdevs
- show nic models
> and as long as the declaration
> of the variable has to stay at the top of the block (according to QEMU's
> coding style), I think I'd also prefer to keep the initialization there.
>
This conflict can easily be solved by putting the nic_models on its own line,
as I have shown in my hunk above.
Ciao,
Claudio
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 <thuth@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.
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
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 <thuth@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
On 11/10/22 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 <thuth@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'd say:
>
> - show_netdevs
> _ get nic models
> - show nic models
>
> instead of:
>
> - get nic models
> - show netdevs
> - show nic models
>
>
> > and as long as the declaration
>> of the variable has to stay at the top of the block (according to QEMU's
>> coding style), I think I'd also prefer to keep the initialization there.
>>
>
> This conflict can easily be solved by putting the nic_models on its own line,
> as I have shown in my hunk above.
>
And another option (not used enough in QEMU code imo), is to open a block specifically for the nic models.
Ie you can say:
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);
}
this even has the additional benefit of making the allocation/free of the nic_models clearer as it corresponds to the block.
C
should -net and -netdev be adapted too?
For audio, we now have support for help options in both -audiodev and -audio..
Thanks,
Claudio
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 <thuth@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);
> + exit(0);
> + }
> }
>
> idx = nic_get_free_idx();
On 07/11/2022 13.27, Claudio Fontana wrote: > should -net and -netdev be adapted too? "-netdev help" already works just fine ... and "-net" should IMHO rather be removed than improved ;-) Thomas
On 11/8/22 09:42, Thomas Huth wrote: > On 07/11/2022 13.27, Claudio Fontana wrote: >> should -net and -netdev be adapted too? > > "-netdev help" already works just fine ... and "-net" should IMHO rather be > removed than improved ;-) > > Thomas > I wonder if it could be done once for all, in net_init_clients, instead of repeating the is_help_option in net_init_netdev and net_param_nic (and that would take care of net_init_client too, so we'd get "net" for free).. Ciao, C
On 08/11/2022 09.52, Claudio Fontana wrote: > On 11/8/22 09:42, Thomas Huth wrote: >> On 07/11/2022 13.27, Claudio Fontana wrote: >>> should -net and -netdev be adapted too? >> >> "-netdev help" already works just fine ... and "-net" should IMHO rather be >> removed than improved ;-) >> >> Thomas >> > > I wonder if it could be done once for all, in net_init_clients, > instead of repeating the is_help_option in net_init_netdev and net_param_nic > (and that would take care of net_init_client too, so we'd get "net" for free).. I don't think that it makes too much sense to have one option for all - since all three CLI options are slightly different anyway. E.g. "-net nic" only exists for "-net", "hubport" cannot be used with "-net", "-nic" can also be used to configure the NIC model, etc. Thomas
On 11/8/22 09:59, Thomas Huth wrote: > On 08/11/2022 09.52, Claudio Fontana wrote: >> On 11/8/22 09:42, Thomas Huth wrote: >>> On 07/11/2022 13.27, Claudio Fontana wrote: >>>> should -net and -netdev be adapted too? >>> >>> "-netdev help" already works just fine ... and "-net" should IMHO rather be >>> removed than improved ;-) >>> >>> Thomas >>> >> >> I wonder if it could be done once for all, in net_init_clients, >> instead of repeating the is_help_option in net_init_netdev and net_param_nic >> (and that would take care of net_init_client too, so we'd get "net" for free).. > > I don't think that it makes too much sense to have one option for all - > since all three CLI options are slightly different anyway. E.g. "-net nic" > only exists for "-net", "hubport" cannot be used with "-net", "-nic" can > also be used to configure the NIC model, etc. > > Thomas > Hi Thomas, I would not suggest to merge everything together, I was considering whether it would make sense to just check the "type" id for is_help_option once, since all the options "net", "netdev", "nic" have a "type" implied_opt_name, and so it should be possible to make a list of structs that signify what to do for "net", "netdev", "nic", and loop on that and check for that help string once, and then split off the codepath into the "net", "netdev", "nic" - specific code as it is now, either manually or by storing the function that is now in the foreach as a function pointer in the struct, ie moving the is_help_option check one level up. However, it might not be worth it since it seems that for "nic" the nic models need to also be printed, so it might make things needlessly verbose. Not sure, have not tried to write the code for it. Ciao, Claudio
On 08/11/2022 10.43, Claudio Fontana wrote: > On 11/8/22 09:59, Thomas Huth wrote: >> On 08/11/2022 09.52, Claudio Fontana wrote: >>> On 11/8/22 09:42, Thomas Huth wrote: >>>> On 07/11/2022 13.27, Claudio Fontana wrote: >>>>> should -net and -netdev be adapted too? >>>> >>>> "-netdev help" already works just fine ... and "-net" should IMHO rather be >>>> removed than improved ;-) >>>> >>>> Thomas >>>> >>> >>> I wonder if it could be done once for all, in net_init_clients, >>> instead of repeating the is_help_option in net_init_netdev and net_param_nic >>> (and that would take care of net_init_client too, so we'd get "net" for free).. >> >> I don't think that it makes too much sense to have one option for all - >> since all three CLI options are slightly different anyway. E.g. "-net nic" >> only exists for "-net", "hubport" cannot be used with "-net", "-nic" can >> also be used to configure the NIC model, etc. >> >> Thomas >> > > Hi Thomas, I would not suggest to merge everything together, > > I was considering whether it would make sense to just check the "type" id for is_help_option > once, since all the options "net", "netdev", "nic" have a "type" implied_opt_name, > > and so it should be possible to make a list of structs that signify what to do for "net", "netdev", "nic", and > loop on that and check for that help string once, > > and then split off the codepath into the "net", "netdev", "nic" - specific code as it is now, > either manually or by storing the function that is now in the foreach as a function pointer in the struct, ie moving the is_help_option check one level up. > > However, it might not be worth it since it seems that for "nic" the nic models need to also be printed, so it might make things needlessly verbose. > > Not sure, have not tried to write the code for it. Sorry, I currently fail to see how that should really work out nicely, so I'll continue with my current patch. But feel free to write some patches, maybe it's clearer to me if I see the code. Thomas
On 11/10/22 12:35, Thomas Huth wrote: > On 08/11/2022 10.43, Claudio Fontana wrote: >> On 11/8/22 09:59, Thomas Huth wrote: >>> On 08/11/2022 09.52, Claudio Fontana wrote: >>>> On 11/8/22 09:42, Thomas Huth wrote: >>>>> On 07/11/2022 13.27, Claudio Fontana wrote: >>>>>> should -net and -netdev be adapted too? >>>>> >>>>> "-netdev help" already works just fine ... and "-net" should IMHO rather be >>>>> removed than improved ;-) >>>>> >>>>> Thomas >>>>> >>>> >>>> I wonder if it could be done once for all, in net_init_clients, >>>> instead of repeating the is_help_option in net_init_netdev and net_param_nic >>>> (and that would take care of net_init_client too, so we'd get "net" for free).. >>> >>> I don't think that it makes too much sense to have one option for all - >>> since all three CLI options are slightly different anyway. E.g. "-net nic" >>> only exists for "-net", "hubport" cannot be used with "-net", "-nic" can >>> also be used to configure the NIC model, etc. >>> >>> Thomas >>> >> >> Hi Thomas, I would not suggest to merge everything together, >> >> I was considering whether it would make sense to just check the "type" id for is_help_option >> once, since all the options "net", "netdev", "nic" have a "type" implied_opt_name, >> >> and so it should be possible to make a list of structs that signify what to do for "net", "netdev", "nic", and >> loop on that and check for that help string once, >> >> and then split off the codepath into the "net", "netdev", "nic" - specific code as it is now, >> either manually or by storing the function that is now in the foreach as a function pointer in the struct, ie moving the is_help_option check one level up. >> >> However, it might not be worth it since it seems that for "nic" the nic models need to also be printed, so it might make things needlessly verbose. >> >> Not sure, have not tried to write the code for it. > > Sorry, I currently fail to see how that should really work out nicely, so > I'll continue with my current patch. But feel free to write some patches, > maybe it's clearer to me if I see the code. > > Thomas > No worries, maybe it's just too complex and not worth it. Ciao, C
© 2016 - 2026 Red Hat, Inc.