Use fewer modprobes by daym · Pull Request #38930 · moby/moby

daym · 2019-03-23T19:52:38Z

It's not customary for user space programs to modprobe stuff in general. Modern distributions will autoload modules (if it is indeed a module and not built-in) on access anyway, so it's also not necessary to manually modprobe stuff.

Therefore, this replaces modprobe by actual accesses of the devices.

GordonTheTurtle · 2019-03-23T19:52:41Z

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "fewer-modprobes" git@github.com:daym/moby.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

justincormack · 2019-03-25T17:59:54Z

Agree that removing modprobes is a good idea.

thaJeztah

Looks like there's some linting failures;

13:33:55 daemon/graphdriver/devmapper/deviceset.go:546:9:warning: if block ends with a return statement, so drop this else and outdent its block (golint)
13:33:55 daemon/graphdriver/overlay/overlay.go:208:9:warning: if block ends with a return statement, so drop this else and outdent its block (golint)
13:33:55 daemon/graphdriver/overlay2/overlay.go:280:9:warning: if block ends with a return statement, so drop this else and outdent its block (golint)

vendor/github.com/docker/libnetwork/iptables/iptables.go

full diff: moby/libnetwork@83d30db...09cdcc8 changes included: - moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling - moby/libnetwork#2411 Macvlan network handles netlabel.Internal wrong - fixes moby/libnetwork#2410 Macvlan network handles netlabel.Internal wrong - moby/libnetwork#2414 Allow network with --config-from to be --internal - fixes moby/libnetwork#2413 Network with --config-from does not honor --internal - moby/libnetwork#2351 Use fewer modprobes - relates to moby#38930 Use fewer modprobes - moby/libnetwork#2415 Support dockerd and system restarts for ipvlan and macvlan networks - carry of moby/libnetwork#2295 phantom ip/mac vlan network after a powercycle - fixes moby/libnetwork#1743 Phantom docker network Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

kolyshkin · 2019-07-30T23:47:50Z

@daym can you please remove the chunk that modifies the code under /vendor?

thaJeztah · 2019-07-31T01:03:05Z

@kolyshkin I guess it can be rebased once #39633 is merged

codecov · 2019-07-31T11:04:43Z

Codecov Report

❗ No coverage uploaded for pull request base (master@264e16c). Click here to learn what that means.
The diff coverage is 52.94%.

@@            Coverage Diff            @@
##             master   #38930   +/-   ##
=========================================
  Coverage          ?    36.9%           
=========================================
  Files             ?      614           
  Lines             ?    45416           
  Branches          ?        0           
=========================================
  Hits              ?    16760           
  Misses            ?    26363           
  Partials          ?     2293

full diff: moby/libnetwork@83d30db...09cdcc8 changes included: - moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling - moby/libnetwork#2411 Macvlan network handles netlabel.Internal wrong - fixes moby/libnetwork#2410 Macvlan network handles netlabel.Internal wrong - moby/libnetwork#2414 Allow network with --config-from to be --internal - fixes moby/libnetwork#2413 Network with --config-from does not honor --internal - moby/libnetwork#2351 Use fewer modprobes - relates to moby/moby#38930 Use fewer modprobes - moby/libnetwork#2415 Support dockerd and system restarts for ipvlan and macvlan networks - carry of moby/libnetwork#2295 phantom ip/mac vlan network after a powercycle - fixes moby/libnetwork#1743 Phantom docker network Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: 6f234db9fef23c591d8376f96db062e7107b658f Component: engine

daemon/graphdriver/devmapper/deviceset.go

thaJeztah · 2019-09-16T16:23:08Z

@AkihiroSuda @kolyshkin looks like the PR was updated to get rid of the vendor changes; PTAL

full diff: moby/libnetwork@83d30db...09cdcc8 changes included: - moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling - moby/libnetwork#2411 Macvlan network handles netlabel.Internal wrong - fixes moby/libnetwork#2410 Macvlan network handles netlabel.Internal wrong - moby/libnetwork#2414 Allow network with --config-from to be --internal - fixes moby/libnetwork#2413 Network with --config-from does not honor --internal - moby/libnetwork#2351 Use fewer modprobes - relates to moby#38930 Use fewer modprobes - moby/libnetwork#2415 Support dockerd and system restarts for ipvlan and macvlan networks - carry of moby/libnetwork#2295 phantom ip/mac vlan network after a powercycle - fixes moby/libnetwork#1743 Phantom docker network Signed-off-by: Sebastiaan van Stijn <github@gone.nl> (cherry picked from commit 6f234db) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

full diff: moby/libnetwork@83d30db...09cdcc8 changes included: - moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling - moby/libnetwork#2411 Macvlan network handles netlabel.Internal wrong - fixes moby/libnetwork#2410 Macvlan network handles netlabel.Internal wrong - moby/libnetwork#2414 Allow network with --config-from to be --internal - fixes moby/libnetwork#2413 Network with --config-from does not honor --internal - moby/libnetwork#2351 Use fewer modprobes - relates to moby/moby#38930 Use fewer modprobes - moby/libnetwork#2415 Support dockerd and system restarts for ipvlan and macvlan networks - carry of moby/libnetwork#2295 phantom ip/mac vlan network after a powercycle - fixes moby/libnetwork#1743 Phantom docker network Signed-off-by: Sebastiaan van Stijn <github@gone.nl> (cherry picked from commit 6f234db9fef23c591d8376f96db062e7107b658f) Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: b6190c2713623ab455d29da4771b684e4eafc63f Component: engine

thaJeztah

SGTM

@cpuguy83 were your questions answered?

kolyshkin · 2019-09-20T21:23:01Z

daemon/graphdriver/overlay/overlay.go

The actual error need to be logged here as well, i.e. add .WithError(err).

Also, we have a convention to

start error messages with a lowercased letter

do not end error messages with .

kolyshkin

A couple of nitpicks concerning error messages

Signed-off-by: Danny Milosavljevic <dannym@scratchpost.org>

full diff: moby/libnetwork@83d30db...09cdcc8 changes included: - moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling - moby/libnetwork#2411 Macvlan network handles netlabel.Internal wrong - fixes moby/libnetwork#2410 Macvlan network handles netlabel.Internal wrong - moby/libnetwork#2414 Allow network with --config-from to be --internal - fixes moby/libnetwork#2413 Network with --config-from does not honor --internal - moby/libnetwork#2351 Use fewer modprobes - relates to moby#38930 Use fewer modprobes - moby/libnetwork#2415 Support dockerd and system restarts for ipvlan and macvlan networks - carry of moby/libnetwork#2295 phantom ip/mac vlan network after a powercycle - fixes moby/libnetwork#1743 Phantom docker network Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Signed-off-by: zach <Zachary.Joyner@linux.com>

daym requested a review from dmcgowan as a code owner March 23, 2019 19:52

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Mar 23, 2019

GordonTheTurtle added status/0-triage and removed dco/no Automatically set by a bot when one of the commits lacks proper signature labels Mar 23, 2019

justincormack approved these changes Mar 25, 2019

View reviewed changes

thaJeztah requested changes Mar 26, 2019

View reviewed changes

vendor/github.com/docker/libnetwork/iptables/iptables.go Outdated Show resolved Hide resolved

thaJeztah added status/2-code-review status/needs-vendoring and removed status/0-triage labels Mar 26, 2019

daym mentioned this pull request Mar 26, 2019

Use fewer modprobes moby/libnetwork#2351

Merged

GordonTheTurtle assigned vieux Apr 7, 2019

thaJeztah mentioned this pull request Jul 30, 2019

bump libnetwork to 09cdcc8c0eab3946c2d70e8f6225b05baf1e90d1 #39633

Merged

thaJeztah mentioned this pull request Aug 22, 2019

[19.03 backport] bump libnetwork to 92d1fbe1eb0883cf11d283cea8e658275146411d docker-archive/engine#330

Merged

AkihiroSuda reviewed Sep 3, 2019

View reviewed changes

daemon/graphdriver/devmapper/deviceset.go Outdated Show resolved Hide resolved

thaJeztah removed the status/needs-vendoring label Sep 16, 2019

AkihiroSuda approved these changes Sep 16, 2019

View reviewed changes

thaJeztah approved these changes Sep 20, 2019

View reviewed changes

kolyshkin reviewed Sep 20, 2019

View reviewed changes

kolyshkin requested changes Sep 20, 2019

View reviewed changes

Use fewer modprobes

074eca1

Signed-off-by: Danny Milosavljevic <dannym@scratchpost.org>

thaJeztah added status/4-merge and removed status/2-code-review labels Sep 21, 2019

AkihiroSuda merged commit 610551e into moby:master Sep 23, 2019

daym deleted the fewer-modprobes branch October 2, 2019 21:20

AkihiroSuda mentioned this pull request Feb 6, 2020

[19.03 backport] overlay[2]: rm extra checks in init #40462

Merged

thaJeztah added the process/cherry-picked label Feb 6, 2020

thaJeztah added this to the 20.03.0 milestone Apr 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use fewer modprobes#38930

Use fewer modprobes#38930
AkihiroSuda merged 1 commit intomoby:masterfrom
daym:fewer-modprobes

daym commented Mar 23, 2019 •

edited

Loading

Uh oh!

GordonTheTurtle commented Mar 23, 2019

Uh oh!

justincormack commented Mar 25, 2019

Uh oh!

thaJeztah left a comment

Uh oh!

Uh oh!

kolyshkin commented Jul 30, 2019

Uh oh!

thaJeztah commented Jul 31, 2019

Uh oh!

codecov bot commented Jul 31, 2019 •

edited

Loading

Uh oh!

Uh oh!

thaJeztah commented Sep 16, 2019

Uh oh!

thaJeztah left a comment

Uh oh!

kolyshkin Sep 20, 2019

Uh oh!

kolyshkin Sep 20, 2019

Uh oh!

kolyshkin left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

daym commented Mar 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GordonTheTurtle commented Mar 23, 2019

Uh oh!

justincormack commented Mar 25, 2019

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kolyshkin commented Jul 30, 2019

Uh oh!

thaJeztah commented Jul 31, 2019

Uh oh!

codecov bot commented Jul 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

thaJeztah commented Sep 16, 2019

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

kolyshkin Sep 20, 2019

Choose a reason for hiding this comment

Uh oh!

kolyshkin Sep 20, 2019

Choose a reason for hiding this comment

Uh oh!

kolyshkin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

daym commented Mar 23, 2019 •

edited

Loading

codecov bot commented Jul 31, 2019 •

edited

Loading