[dpdk-dev,4/5] eal: add an error code to plugin init for the next step
Commit Message
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
---
lib/librte_eal/bsdapp/eal/eal.c | 3 ++-
lib/librte_eal/common/eal_common_options.c | 3 ++-
lib/librte_eal/common/eal_options.h | 2 +-
lib/librte_eal/linuxapp/eal/eal.c | 3 ++-
4 files changed, 7 insertions(+), 4 deletions(-)
Comments
On Fri, Oct 16, 2015 at 02:58:16PM +0300, Panu Matilainen wrote:
> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> ---
> lib/librte_eal/bsdapp/eal/eal.c | 3 ++-
> lib/librte_eal/common/eal_common_options.c | 3 ++-
> lib/librte_eal/common/eal_options.h | 2 +-
> lib/librte_eal/linuxapp/eal/eal.c | 3 ++-
> 4 files changed, 7 insertions(+), 4 deletions(-)
Again, another minor nit, but couldn't this be done when refactoring in previous
patches, rather than needed a whole separate commit ?
/Bruce
On 10/16/2015 03:59 PM, Bruce Richardson wrote:
> On Fri, Oct 16, 2015 at 02:58:16PM +0300, Panu Matilainen wrote:
>> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
>> ---
>> lib/librte_eal/bsdapp/eal/eal.c | 3 ++-
>> lib/librte_eal/common/eal_common_options.c | 3 ++-
>> lib/librte_eal/common/eal_options.h | 2 +-
>> lib/librte_eal/linuxapp/eal/eal.c | 3 ++-
>> 4 files changed, 7 insertions(+), 4 deletions(-)
>
> Again, another minor nit, but couldn't this be done when refactoring in previous
> patches, rather than needed a whole separate commit ?
Of course it'd be possible to do this earlier, I pondered about it too
but then went with this because
a) otherwise I would've had to rework the earlier patches again
b) not knowing which way people prefer it, I might've had to rework it
back to the original
c) didn't know we were saving commits
d) doing it like this maintains a certain symmetry to how stuff is
introduced
... yes, its all rather academic :)
- Panu -
On 10/16/2015 04:14 PM, Panu Matilainen wrote:
> On 10/16/2015 03:59 PM, Bruce Richardson wrote:
>> On Fri, Oct 16, 2015 at 02:58:16PM +0300, Panu Matilainen wrote:
>>> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
>>> ---
>>> lib/librte_eal/bsdapp/eal/eal.c | 3 ++-
>>> lib/librte_eal/common/eal_common_options.c | 3 ++-
>>> lib/librte_eal/common/eal_options.h | 2 +-
>>> lib/librte_eal/linuxapp/eal/eal.c | 3 ++-
>>> 4 files changed, 7 insertions(+), 4 deletions(-)
>>
>> Again, another minor nit, but couldn't this be done when refactoring
>> in previous
>> patches, rather than needed a whole separate commit ?
>
> Of course it'd be possible to do this earlier, I pondered about it too
> but then went with this because
> a) otherwise I would've had to rework the earlier patches again
> b) not knowing which way people prefer it, I might've had to rework it
> back to the original
> c) didn't know we were saving commits
> d) doing it like this maintains a certain symmetry to how stuff is
> introduced
In other words: I spent many years working with a codebase where the
policy was to never change code while moving it around otherwise. So
yeah, matter of policy, taste and all, I'm clearly still learning where
the fine line is in case of dpdk :)
The series can easily be shrunken into two logical steps if that's
preferred:
1) move the plugin handling code to common
2) add the plugin directory support
- Panu -
2015-10-16 16:38, Panu Matilainen:
> On 10/16/2015 04:14 PM, Panu Matilainen wrote:
> > On 10/16/2015 03:59 PM, Bruce Richardson wrote:
> >> On Fri, Oct 16, 2015 at 02:58:16PM +0300, Panu Matilainen wrote:
> >>> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> >>> ---
> >>> lib/librte_eal/bsdapp/eal/eal.c | 3 ++-
> >>> lib/librte_eal/common/eal_common_options.c | 3 ++-
> >>> lib/librte_eal/common/eal_options.h | 2 +-
> >>> lib/librte_eal/linuxapp/eal/eal.c | 3 ++-
> >>> 4 files changed, 7 insertions(+), 4 deletions(-)
> >>
> >> Again, another minor nit, but couldn't this be done when refactoring
> >> in previous
> >> patches, rather than needed a whole separate commit ?
> >
> > Of course it'd be possible to do this earlier, I pondered about it too
> > but then went with this because
> > a) otherwise I would've had to rework the earlier patches again
> > b) not knowing which way people prefer it, I might've had to rework it
> > back to the original
> > c) didn't know we were saving commits
> > d) doing it like this maintains a certain symmetry to how stuff is
> > introduced
>
> In other words: I spent many years working with a codebase where the
> policy was to never change code while moving it around otherwise. So
> yeah, matter of policy, taste and all, I'm clearly still learning where
> the fine line is in case of dpdk :)
>
> The series can easily be shrunken into two logical steps if that's
> preferred:
> 1) move the plugin handling code to common
> 2) add the plugin directory support
Yes, looks good. Thanks
@@ -543,7 +543,8 @@ rte_eal_init(int argc, char **argv)
rte_eal_mcfg_complete();
- eal_plugins_init();
+ if (eal_plugins_init() < 0)
+ rte_panic("Cannot init plugins\n");
eal_thread_init_master(rte_config.master_lcore);
@@ -167,7 +167,7 @@ eal_plugin_add(const char *path)
return 0;
}
-void
+int
eal_plugins_init(void)
{
struct shared_driver *solib = NULL;
@@ -178,6 +178,7 @@ eal_plugins_init(void)
if (solib->lib_handle == NULL)
RTE_LOG(WARNING, EAL, "%s\n", dlerror());
}
+ return 0;
}
/*
@@ -93,6 +93,6 @@ int eal_adjust_config(struct internal_config *internal_cfg);
int eal_check_common_options(struct internal_config *internal_cfg);
void eal_common_usage(void);
enum rte_proc_type_t eal_proc_type_detect(void);
-void eal_plugins_init(void);
+int eal_plugins_init(void);
#endif /* EAL_OPTIONS_H */
@@ -796,7 +796,8 @@ rte_eal_init(int argc, char **argv)
rte_eal_mcfg_complete();
- eal_plugins_init();
+ if (eal_plugins_init() < 0)
+ rte_panic("Cannot init plugins\n");
eal_thread_init_master(rte_config.master_lcore);