Re: [Minios-devel] [UNIKRAFT PATCH 8/8] lib/uklibparam: Add documentation for parameters

[Florian Schmidt <Florian.Schmidt@xxxxxxxxx>]

Hi Sharan,

originally, I started to review your code from the beginning, but then I 
figured, maybe I should start with reading your documentation and try to 
use it without knowing all the ins and outs of the code yet. I have a 
few remarks below that would clarify things, I think:
On 3/15/19 6:06 PM, Sharan Santhanam wrote:
> This patch adds documentation for Unikraft library argument
> library.
>
> Signed-off-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> ---
>   doc/guides/developers-app.rst | 72 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 72 insertions(+)
>
> diff --git a/doc/guides/developers-app.rst b/doc/guides/developers-app.rst
> index 2a67b1f..fe779a4 100644
> --- a/doc/guides/developers-app.rst
> +++ b/doc/guides/developers-app.rst
> @@ -323,6 +323,78 @@ An example context of extra.ld: ::
>   
>   This will add the section .uk_fs_list after the .text
>   
> +==================================
> +Command line arguments in Unikraft
> +==================================
> +Unikraft command line arguments could either belong to an application or to a
> +library. These two classes of arguments are separated by **--**. The
> +arguments meant to a library precedes the **--**, followed by the application
> +arguments.
> +
> +Pass an argument to a library
> +-----------------------------
> +Unikraft provides support to pass arguments of the following data type:
> +
> +========  ========================
> +Type      Description
> +========  ========================
> +char      Single character value
I would point out here explicitly that "char" is an alias for "__s8". 
It's pretty obvious in the case of char...
> +__s8      Same as char
> +__u8      Single byte value
> +__s16     Short signed integer
> +__u16     Short unsigned integer
> +int       Integer
... but here, it might not necessarily be obvious that "int" is an alias 
for "__s32" in all situations.
> +__s32     Signed integer
> +__u32     Unsigned integer
> +__s64     Signed long integer
> +__u64     Unsigned long integer
> +charp     C strings.
> +========  ========================
> +
> +There are three interfaces through which a library registers with a variable
> +with Unikraft. These are:
> +
> +* UK_LIB_PARAM     - Pass a scalar value of the above type to a variable.
> +* UK_LIB_PARAM_STR - Pass a null terminated string to a variable.
> +* UK_LIB_PARAM_ARR - Pass space separated list of values of the above type.
This should maybe shortly explain how many arguments each macro takes 
and, in one or two words each, their meanings.
> +
> +Each library argument is identified by the following format ::
> +
> + [library name].[variable name]
> +
> + where,
> +     library name is the name registered with Unikraft build system.
> +     variable name is the name of the global or static variable in the program.
> +
> +Examples
> +--------
> +If the library requires command line argument support, it needs some
> +configuration to be performed while building the library. A Unikraft library 
> can
> +be specific to a particular platform or common across all the platform.
> +For the common library, one has to edit the Makefile.uk with ::
> +
> + $(eval $(call addlib_paramprefix,libukalloc,alloc))
> + where,
> +      libukalloc is the name of the library
> +      alloc is the alias for the library name.
> +
> +or for the platform library, edit the Makefile.uk with ::
> +
> + $(eval $(call addplatlib_paramprefix,linuxu,liblinuxuplat,linuxu))
> + where,
> +     linuxu is the Unikraft platform
> +     liblinuxuplat is the name of the library
> +     linuxu an alias for the library name.
> +
> +As the next step, we define a variable and register it with the ``uk_libparam``
> +library. The example below a simple code snippet ::
> +
> + static __u32 heap_size = CONFIG_LINUXU_DEFAULT_HEAPMB;
> + UK_LIB_PARAM(heap_size, __u32);
I think this might be confusing. At first, I thought I might actually 
need to define a CONFIG_* definition for every library parameter. So 
maybe a simpler example of "static __u16 myparam = 23" as a first 
example would be better, and then you can continue with "of course, we 
can also initialize the parameter with a compile-time configuration 
setting from menuconfig" or something like that.
Also, you don't mention that uk/libparam.h needs to be included.

> +
> +We can override the default value using the following command line ::
> +
> +  ./unikraft_linuxu-x86_64 linuxu.heap_size=10 --
>   
>   ============================
>   Make Targets
Those are the remarks I have so far. If I come up with more, I'll add 
comments later. :)
Cheers,
Florian


--
Dr. Florian Schmidt
フローリアン・シュミット
Research Scientist,
Systems and Machine Learning Group
NEC Laboratories Europe
Kurfürsten-Anlage 36, D-69115 Heidelberg
Tel.     +49 (0)6221 4342-265
Fax:     +49 (0)6221 4342-155
e-mail:  florian.schmidt@xxxxxxxxx
============================================================
Registered at Amtsgericht Mannheim, Germany, HRB728558

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel