Giuseppe Lettieri <giuseppe.letti...@unipi.it> writes:
> Il 13/09/19 15:04, Markus Armbruster ha scritto:
>>
>> What happens when I build with --enable-netmap=system on host A, then
>> run the resulting binary on some host B that doesn't have netmap
>> installed?
>>
>
> Qemu will fail at startup, complaining that /dev/netmap does not exists.
>
>
>>
>> Yes. We default to netmap=system, though. If you break things by
>> passing arcane arguments to configure, you get to keep the pieces :)
>>
>>> If the option is only useful for developers to check that some qemu
>>> change does not break anything, then probably it should be enabled in
>>> some other, less visible way. What do you think?
>>
>> I think an --enable-netmap patterned after --enable-capstone and
>> --enable-slirp has sufficiently low visibility as long as the default is
>> sane.
>>
>> We clearly want configure to pick netmap=system when the system provides
>> netmap.
>>
>> What shall configure pick when the system doesn't provide it? If you
>> think netmap=git is too dangerous for general audience, consider
>> disabling netmap then. Experts can still compile-test with
>> --enable-netmap=git. Our CI certainly should.
>>
>
> OK, sounds reasonable. The attached patch will select system if netmap
> is available, and git only if explicitly requested.
>
> Cheers,
> Giuseppe
>
> --
> Dr. Ing. Giuseppe Lettieri
> Dipartimento di Ingegneria della Informazione
> Universita' di Pisa
> Largo Lucio Lazzarino 1, 56122 Pisa - Italy
> Ph. : (+39) 050-2217.649 (direct) .599 (switch)
> Fax : (+39) 050-2217.600
> e-mail: g.letti...@iet.unipi.it
>
>>From 4e93b5cc3ad68d92bc3562df3745e1d10dc08fc0 Mon Sep 17 00:00:00 2001
> From: Giuseppe Lettieri <g.letti...@iet.unipi.it>
> Date: Mon, 2 Sep 2019 17:35:33 +0200
> Subject: [PATCH] netmap: support git-submodule build otption
>
> With this patch, netmap support can be enabled with
> the following options to the configure script:
>
> --enable-netmap[=system]
>
> Use the host system netmap installation.
> Fail if not found.
>
> --enable-netmap=git
>
> clone the official netmap repository on
> github (mostly useful for CI)
>
> system will also be automatically used if no option is
> passed and netmap is available in the host system.
>
> Signed-off-by: Giuseppe Lettieri <giuseppe.letti...@unipi.it>
> ---
> .gitmodules | 3 +++
> configure | 68 ++++++++++++++++++++++++++++++++++++++++++++---------
> netmap | 1 +
> 3 files changed, 61 insertions(+), 11 deletions(-)
> create mode 160000 netmap
>
> diff --git a/.gitmodules b/.gitmodules
> index c5c474169d..bf75dbc5e3 100644
> --- a/.gitmodules
> +++ b/.gitmodules
> @@ -58,3 +58,6 @@
> [submodule "roms/opensbi"]
> path = roms/opensbi
> url = https://git.qemu.org/git/opensbi.git
> +[submodule "netmap"]
> + path = netmap
> + url = https://github.com/luigirizzo/netmap.git
> diff --git a/configure b/configure
> index 30aad233d1..5cb924985c 100755
> --- a/configure
> +++ b/configure
> @@ -1133,6 +1133,10 @@ for opt do
> ;;
> --enable-netmap) netmap="yes"
> ;;
> + --enable-netmap=git) netmap="git"
> + ;;
> + --enable-netmap=system) netmap="system"
> + ;;
> --disable-xen) xen="no"
> ;;
> --enable-xen) xen="yes"
> @@ -3314,8 +3318,9 @@ fi
> # a minor/major version number. Minor new features will be marked with
> values up
> # to 15, and if something happens that requires a change to the backend we
> will
> # move above 15, submit the backend fixes and modify this two bounds.
> -if test "$netmap" != "no" ; then
> - cat > $TMPC << EOF
> +case "$netmap" in
> + "" | yes | system)
> + cat > $TMPC << EOF
> #include <inttypes.h>
> #include <net/if.h>
> #include <net/netmap.h>
> @@ -3325,15 +3330,56 @@ if test "$netmap" != "no" ; then
> #endif
> int main(void) { return 0; }
> EOF
> - if compile_prog "" "" ; then
> - netmap=yes
> - else
> - if test "$netmap" = "yes" ; then
> - feature_not_found "netmap"
> + if compile_prog "" "" ; then
> + netmap_system=yes
> + else
> + netmap_system=no
> + fi
> + ;;
> +esac
Is the indentation change intentional?
> +
> +case "$netmap" in
> + "" | yes)
> + if test "$netmap_system" = "yes"; then
> + netmap=system
> + elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
> + netmap=git
> + elif test -e "${source_path}/netmap/configure" ; then
> + netmap=internal
> + elif test -z "$netmap" ; then
> + netmap=no
> + else
> + feature_not_found "netmap" "Install netmap or git submodule"
> fi
> - netmap=no
> - fi
> -fi
> + ;;
> +
> + system)
> + if test "$netmap_system" = "no"; then
> + feature_not_found "netmap" "Install netmap"
> + fi
> + ;;
> +esac
> +
> +case "$netmap" in
> + git | internal)
> + if test "$netmap" = git; then
> + git_submodules="${git_submodules} netmap"
> + fi
> + mkdir -p netmap
> + QEMU_CFLAGS="$QEMU_CFLAGS -I\$(SRC_PATH)/netmap/sys"
> + ;;
> +
> + system)
> + ;;
> +
> + no)
> + ;;
> + *)
> + error_exit "Unknown state for netmap: $netmap"
> + ;;
> +esac
> +
> +##########################################
>
> ##########################################
> # libcap-ng library probe
> @@ -6582,7 +6628,7 @@ if test "$vde" = "yes" ; then
> echo "CONFIG_VDE=y" >> $config_host_mak
> echo "VDE_LIBS=$vde_libs" >> $config_host_mak
> fi
> -if test "$netmap" = "yes" ; then
> +if test "$netmap" != "no" ; then
> echo "CONFIG_NETMAP=y" >> $config_host_mak
> fi
> if test "$l2tpv3" = "yes" ; then
> diff --git a/netmap b/netmap
> new file mode 160000
> index 0000000000..137f537eae
> --- /dev/null
> +++ b/netmap
> @@ -0,0 +1 @@
> +Subproject commit 137f537eae513f02d5d6871d1f91c049e6345803
Looks reasonable to me. Please submit it as a patch.
