> On July 17, 2017, 3:20 a.m., Qian Zhang wrote: > > m4/libnl3.m4 > > Lines 33-37 (patched) > > <https://reviews.apache.org/r/60902/diff/1/?file=1777351#file1777351line33> > > > > Mind to elaborate a bit about why introducing a new .m4 file? What > > about just introducing a new configure option (like `--enable-libnl3`) and > > make `--enable_port_mapping_isolator` depend on it? Just like the > > relationship between `--enable-ssl` and `--enable-libevent`: > > https://github.com/apache/mesos/blob/1.3.0/configure.ac#L1657:L1698 > > > > And for `--enable-libnl3`, I think we should not do much checks, just > > some very basic check should be OK, and on top of it, > > `--enable_port_mapping_isolator` can do some more advanced/specific check > > on libnl3 for itself. > > James Peach wrote: > > Mind to elaborate a bit about why introducing a new .m4 file? > > This is because a separate file makes it easier to maintain. We could > inline the macro into `configure.ac` but that's not typical autotool practice. > > > What about just introducing a new configure option (like > --enable-libnl3) > > I don't see how `--enable-libnl3` makes anything simpler, unless you want > to to use that option to enable all the features that depend on `libnl3`. In > that case, however, I'd just implement that based on the existing `--with-nl`. > > The semantics here are that we have `--enable-port-mapping-isolator`, > `--enable-network-ports-isolator` and `--with-nl` arguments. If either of the > first 2 options is enabled, we will attempt to use the `libnl3` from the > location of the 3rd option. > > > I think we should not do much checks > > The checks we are applying are all ones that we already had. I didn't > want to change the dependency requirements here. > > Qian Zhang wrote: > I was thinking to use `--enable-libnl3` to check basic libnl3 features, > and use `--enable_port_mapping_isolator` and > `--enable-network-ports-isolator` to check the advanced libnl3 features > needed by them respectively. > > So in your solution, there will be only 2 `--enable-xxx` flags (i.e., > `--enable_port_mapping_isolator`, `--enable-network-ports-isolator`) and > `--with-nl`, and `MESOS_HAVE_LIBNL3` is internal which is not exposed to the > user, right?
> I was thinking to use --enable-libnl3 to check basic libnl3 features, and use > --enable_port_mapping_isolator and --enable-network-ports-isolator to check > the advanced libnl3 features needed by them respectively. At this point, I'd prefer not to change the semantics of this. We have a pretty clear, documented requirement for libnl >= 3.2.6 and if we start accepting different versions, it becomes less clear. > So in your solution, there will be only 2 --enable-xxx flags (i.e., > --enable_port_mapping_isolator, --enable-network-ports-isolator) and > --with-nl, Yes, that's correct. > and MESOS_HAVE_LIBNL3 is internal which is not exposed to the user, right? `MESOS_HAVE_LIBNL3` is just the autoconf macro name, so it's not exposed either to the user or to the build. There is a new build conditional named `ENABLE_LINUX_ROUTING` which causes the `src/linux/routing` files to be included in the build - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60902/#review180653 ----------------------------------------------------------- On July 16, 2017, 11:43 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60902/ > ----------------------------------------------------------- > > (Updated July 16, 2017, 11:43 p.m.) > > > Review request for mesos and Qian Zhang. > > > Bugs: MESOS-7675 > https://issues.apache.org/jira/browse/MESOS-7675 > > > Repository: mesos > > > Description > ------- > > Since the `network/ports` isolator will depend on libnl3, move those > checks into a separate macro so that we can call it again when we > add a configure option to enable it. > > > Diffs > ----- > > configure.ac 4d7c4a4679e5c624ee750226d542e0d8c228507a > m4/libnl3.m4 PRE-CREATION > > > Diff: https://reviews.apache.org/r/60902/diff/2/ > > > Testing > ------- > > make check (Fedora 26), manual verification of configuration logs. > > > Thanks, > > James Peach > >
