Re: [Minios-devel] [UNIKRAFT/CLICK PATCH v2 01/11] Initial public release: basic unikraft files

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

Great, thanks a lot.

On 05.06.19 13:16, Florian Schmidt wrote:
> Hi Simon,
>
> Felipe and I decided to split the work on those. I'll comment on what 
> I'll fix.
> On 6/5/19 1:13 PM, Simon Kuenzer wrote:
> > Hey all,
> >
> > I have actually some comments about this patch. See my comments inline.
> >
> > On 05.06.19 10:56, Costin Lupu wrote:
> > > Reviewed-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> > >
> > > On 6/5/19 10:35 AM, Florian Schmidt wrote:
> > > > --- /dev/null
> > > > +++ b/Config.uk
> > > > @@ -0,0 +1,86 @@
> > > > +### Invisible option for dependencies
> > > > +config APP_DEPENDENCIES
> > > > +    bool
> > > > +    default y
> > > > +
> > > > +menuconfig LIBCLICK
> > > > +    bool "The Click modular router"
> > > > +    depends on HAVE_LIBC
> > > > +    select LIBLWIP
> > > > +    default y
> > It is probably better if you select LIBNEWLIB instead of checking if a 
> > LIBC is there. If we add more libc's later, we anyway need to check if 
> > it compiles. newlib is the working configuration.
> OK, I'll select LIBNEWLIB. I also realized that click needs ukunistd, so 
> I'll also add select UKUNISTD.
>
> > > > --- /dev/null
> > > > +++ b/Makefile
> > > > @@ -0,0 +1,9 @@
> > > > +UK_ROOT ?= $(PWD)/../unikraft
> > > > +UK_LIBS ?= $(PWD)/..
> > > > +LIBS := $(UK_LIBS)/newlib:$(UK_LIBS)/lwip
> > > > +
> > So far, we had the following default definition for UK_ROOT and UK_LIBS:
> >   UK_ROOT ?= $(PWD)/../../unikraft
> >   UK_LIBS ?= $(PWD)/../../libs
> > For simplicity we should use the as we do with the applications 
> > `helloworld` and `httpreply` so that user can expect the same default 
> > directory structure.
>
> I'll change the paths accordingly.
>
> > > > +################################################################################ 
> > > >
> > > > +# Check requirements: Click requires netdev access, so LWIP must 
> > > > not attach them
> > > > +################################################################################ 
> > > >
> > > > +ifeq ($(CONFIG_LWIP_AUTOIFACE),y)
> > > > +$(error Click cannot be built with LWIP option "Automatically 
> > > > attach netifs"! Please disable)
> > > > +endif
> > Could you add a comment why you are doing this with an error?: KConfig 
> > unfortunately does not support to `unselect` an configuration option. 
> > I hope problems like this can be fixed by updating to a recent KConfig 
> > system.
>
> I'll add a comment.
>
> The rest is up to Felipe.
>
> Cheers,
> Florian
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel