Re: [PATCH] rumpkernel dependencies

[Samuel Thibault]

Damien Zammit, le ven. 03 avril 2020 21:48:15 +1100, a ecrit:
> On 3/4/20 6:45 pm, Samuel Thibault wrote:
> > Isn't that ./buildrump.sh/src/sys/sys/ioccom.h ?
> 
> Oh yea, I have added that one instead to our tree (rump doesn't have it).

Well, rump does have it but doesn't install it (as well as dkio.h), but
it should, shouldn't it?  Otherwise rump_sys_ioctl is basically unusable
without it. It looks like makekernelheaders could be installing this?
Probably we don't want all of them? Probably discussion can happen with
upstream to know what they think about it? (I guess not being able to
use ioctl is a good argument).

> +ifeq ($(HAVE_LIBRUMP),1)
> +prog-subdirs += rumpdisk
> +endif

One usually use "yes" for these, not "1", see just below :)

>  AC_SUBST([HAVE_LIBZ])
>  
> +AC_ARG_WITH([librump],
> +  [AS_HELP_STRING([--without-librump], [disable librump])], , 
> [with_librump=yes])
> +
> +AC_DEFUN([LIBRUMP_FAIL], [
> +  AC_MSG_FAILURE([Please install required libraries or use 
> --without-librump.])
> +])
> +AS_IF([test "x$with_librump" != xno], [
> +  AC_CHECK_HEADER([rump/rump.h],
> +    [AC_DEFINE(HAVE_RUMP_RUMP_H)],
> +    [LIBRUMP_FAIL])
> +  AC_CHECK_LIB(rump, rump_init, [HAVE_LIBRUMP=1], [LIBRUMP_FAIL])
> +])
> +AC_SUBST([HAVE_LIBRUMP])

Please make it automatically optionnal as I mentioned: either rump is
detected and it is used, or it is not detected, and then anything that
needs it is disabled.

> +++ b/rumpdisk/Makefile

> +rumpdisk.static: main.o block-rump.o

I don't think this is needed? SRCS already provides the list of files to
include.

> +++ b/rumpdisk/block-rump.c

> +/* BSD name of whole disk device is /dev/wdXd
> + * but we will receive /dev/wdX as the name */
> +static void
> +translate_name (char *name, char *output)
> +{
> +  snprintf (output, DISK_NAME_LEN, "%sd", name);
> +}

Make the length a parameter, to avoid the magic assumption that it's
DISK_NAME_LEN.

> +/* FIXME:
> + * Call rump_sys_aio_read/write and return MIG_NO_REPLY from

Do mention only that long-term plan. Also mention rump_sys_pread/pwrite
plan for multiple thread support. That's what we'll probably use short-
and middle-term wise.

Samuel