Skip to content

nixos/network-interfaces: Add support for multiple ipv4 / ipv6 addresses#3182

Merged
7c6f434c merged 4 commits intoNixOS:masterfrom
wkennington:master.ipv6
Aug 30, 2014
Merged

nixos/network-interfaces: Add support for multiple ipv4 / ipv6 addresses#3182
7c6f434c merged 4 commits intoNixOS:masterfrom
wkennington:master.ipv6

Conversation

@wkennington
Copy link
Contributor

No description provided.

@wkennington
Copy link
Contributor Author

TODO: Fix broken nixos tests
TODO: Add a stop script to the interface configuration to flush nixos configured addresses.

@wkennington
Copy link
Contributor Author

Stop script done and tested, TODO broken tests.

@wkennington
Copy link
Contributor Author

Should be done, testing would be appreciated but this works on all of my multi-interface, multi-ip machines.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this also filter out ipv6 interfaces?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it really only make sense in the case of ipv4 as you could conceivably want a static + dhcp ipv6 address. I think we should really force the user to choose if they want dhcp or not. Not try and figure it out for them.

@wkennington
Copy link
Contributor Author

@edolstra Could you look over this before I get too entrenched in using it and need to make changes?

@wkennington wkennington force-pushed the master.ipv6 branch 3 times, most recently from 7518138 to 27efdb7 Compare August 29, 2014 19:20
@7c6f434c
Copy link
Member

Is it necessary to make the old way defunct? Maybe it should complain using trace, but define a single address using the new syntax?

@robberer
Copy link
Contributor

Would be really great if this PR could be applied. I'm waiting for it since wkennington opened this PR.

@peti
Copy link
Member

peti commented Aug 30, 2014

Yes, this change looks quite useful.

@wkennington
Copy link
Contributor Author

I could fairly trivially support the old syntax with the exception of the subnetMask line which I assume isn't very common anymore. I just wanted to force a migration to the new syntax so users were more aware of the changes.

@aristidb
Copy link
Contributor

Yes compatibility please. If somebody needs multiple IPs he will find the
new syntax surely.
Am 30.08.2014 16:14 schrieb "William A. Kennington III" <
notifications@github.com>:

I could fairly trivially support the old syntax with the exception of the
subnetMask line which I assume isn't very common anymore. I just wanted to
force a migration to the new syntax so users were more aware of the changes.


Reply to this email directly or view it on GitHub
#3182 (comment).

@wkennington
Copy link
Contributor Author

I think that should do it.

@7c6f434c
Copy link
Member

Let's hope it works with real-world old-style configurations…

7c6f434c added a commit that referenced this pull request Aug 30, 2014
nixos/network-interfaces: Add support for multiple ipv4 / ipv6 addresses
@7c6f434c 7c6f434c merged commit b23fd65 into NixOS:master Aug 30, 2014
@robberer
Copy link
Contributor

There is an issue with this patch:

error: attribute `address' missing, at /etc/nixos/nixpkgs/nixos/modules/tasks/network-interfaces.nix:630:32

rbvermaa added a commit that referenced this pull request Aug 31, 2014
This reverts commit b23fd65, reversing
changes made to 43654cb.
@rbvermaa
Copy link
Member

I have reverted for now, because it seems to break even simple configurations. The feature sounds nice though.

@rbvermaa
Copy link
Member

Also, please in next PR also add the following (also reverted) commit : 704e91b

@rbvermaa
Copy link
Member

I suspect the issue might be an interaction with nixpkgs/nixos/modules/programs/virtualbox.nix

wkennington added a commit to wkennington/nixpkgs that referenced this pull request Aug 31, 2014
@wkennington wkennington deleted the master.ipv6 branch September 7, 2014 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0.kind: enhancement Add something new or improve an existing system.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants