Re: [PATCH] netmap: support git-submodule build otption

[Markus Armbruster]

Thomas Huth <address@hidden> writes:

> On 07/10/2019 14.35, Markus Armbruster wrote:
>> Peter Maydell <address@hidden> writes:
>> 
>>> On Mon, 7 Oct 2019 at 11:50, Markus Armbruster <address@hidden> wrote:
>>>> Peter Maydell <address@hidden> writes:
>>>>> Basically new submodules are a pain so we seek to minimize
>>>>> the use of them.
>>>>
>>>> I suggested making it a submodule upthread[*].  Let me try to distill
>>>> the conversation into a rationale.  Giuseppe, please correct mistakes.
>>>>
>>>> To make use of QEMU's netmap backend (CONFIG_NETMAP), you have to build
>>>> and install netmap software from sources[**].  Which pretty much ensures
>>>> developers compile with CONFIG_NETMAP off, and the code rots.
>>>>
>>>> For other dependencies that aren't readily available on common
>>>> development hosts (slirp, capstone), we use submodules to avoid such
>>>> rot.  If the system provides, we use that, and if it doesn't, we fall
>>>> back to the submodule.  This has served us well.
>>>
>>> I would put this differently. We don't use submodules to avoid
>>> code-rot. We use submodules where a dependency is needed for us
>>> to provide QEMU features that are sufficiently important that we
>>> want to provide them to users even if those users don't have the
>>> dependency available to them as a system library.
>>>
>>> There are lots of features of QEMU that only compile with sufficiently
>>> recent versions of dependencies, and we don't try to submodule-ize
>>> them because the features aren't really that important for the bulk
>>> of our users. For instance, we provided pixman as a submodule for
>>> a while because the features that require it (our graphics layer
>>> code) are important to almost all users. But we didn't provide
>>> spice as a module even when you pretty much needed to be
>>> running bleeding-edge redhat to satisfy the version dependency
>>> we had, because most users don't care about spice support.
>>> Shipping our dependencies as submodules imposes real costs
>>> on the project (for instance we then need to track the upstream
>>> to see when we should be updating, including checking whether
>>> we need to update to fix security issues). Submodules should be
>>> the exception, not the rule.
>>>
>>>> For netmap, falling back to the submodule when the host doesn't provide
>>>> tends not to be useful beyond compile-testing.  Because of that, we fall
>>>> back only when the user explicitly asks for it by passing
>>>> --enable-netmap=git to configure.  CI should do that.
>>>
>>> This sounds like netmap is in the same position as most of our
>>> dependencies: OK to compile if the system provides the library,
>>> but if the system doesn't then almost all users won't care
>>> that the feature isn't present. If CI of the QEMU code is useful,
>> 
>> If CI of QEMU code isn't useful, then I suspect the QEMU code isn't
>> useful, period.  Giuseppe assures us the netmap QEMU code *is* useful.
>> It followe we better make sure our CI covers it.
>> 
>> A submodule would make sure, but it looks like it won't fly.  So let's
>> try another tack:
>> 
>>> get the library supported by and shipped in distros. If you can't
>>> get anybody in a distro (Linux or BSD) to care enough to ship the
>>> library, this is a really niche feature, and up for consideration
>>> for deprecate-and-drop from QEMU, I think.
>> 
>> Giuseppe, you mentioned netmap is in FreeBSD, and getting it into Linux
>> is unlikely, so let's focus on FreeBSD.
>> 
>> We have a FreeBSD section in .patchew.yml, which makes me guess Patchew
>> CI tests FreeBSD.  Does it test with CONFIG_NETMAP out of the box?  If
>> not, how do we have to tweak its configuration to get CONFIG_NETMAP
>> enabled?  Who could help with this?
>
> I just tried this patch here:
>
> diff --git a/.cirrus.yml b/.cirrus.yml
> --- a/.cirrus.yml
> +++ b/.cirrus.yml
> @@ -8,7 +8,7 @@ freebsd_12_task:
>      memory: 8G
>    install_script: pkg install -y
>      bash bison curl cyrus-sasl git glib gmake gnutls gsed
> -    nettle perl5 pixman pkgconf png usbredir
> +    nettle perl5 pixman pkgconf png usbredir netmap
>    script:
>      - mkdir build
>      - cd build
>
> ... and looks like net/netmap.c now gets successfully compiled on
> FreeBSD in the Cirrus-CI:
>
>  https://api.cirrus-ci.com/v1/task/5669479475838976/logs/main.log

Awesome :)

> We can also add it to the vm-freebsd test:
>
> diff --git a/tests/vm/freebsd b/tests/vm/freebsd
> --- a/tests/vm/freebsd
> +++ b/tests/vm/freebsd
> @@ -54,6 +54,9 @@ class FreeBSDVM(basevm.BaseVM):
>          # libs: opengl
>          "libepoxy",
>          "mesa-libs",
> +
> +        # libs: network
> +        "netmap",
>      ]
>
>      BUILD_SCRIPT = """
>
> ... then it gets compiled succesfully during "make vm-build-freebsd".
>
> So does that sound like a good way to keep netmap.c from bitrotting? If
> so, I can send the above two diffs as a proper patch, if you like.

Yes, please!