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

[Thomas Huth]

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

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.

 Thomas