Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ipv6-only routed networks #1440

Merged

Conversation

rubenk
Copy link
Contributor

@rubenk rubenk commented Jan 22, 2022

Trying to create a routed network with just ipv6 fails with:
Error occurred while creating new network: Call to virNetworkDefineXML failed: XML error: Missing required address attribute in network 'test'.

Since an ipv4 address should be optional for this scenario, make it so.

This makes the following configuration work:

config.vm.network :private_network,
:libvirt__network_name => "test",
:libvirt__forward_mode => "route",
:libvirt__dhcp_enabled => false,
:libvirt__guest_ipv6 => "yes",
:libvirt__ipv6_address => "fc00::",
:libvirt__ipv6_prefix => "64"

Trying to create a routed network with just ipv6 fails with:
Error occurred while creating new network: Call to virNetworkDefineXML failed: XML error: Missing required address attribute in network 'test'.

Since an ipv4 address should be optional for this scenario, make it so.

This makes the following configuration work:

  config.vm.network :private_network,
    :libvirt__network_name => "test",
    :libvirt__forward_mode => "route",
    :libvirt__dhcp_enabled => false,
    :libvirt__guest_ipv6 => "yes",
    :libvirt__ipv6_address => "fc00::",
    :libvirt__ipv6_prefix => "64"
@electrofelix
Copy link
Contributor

Currently I don't quite have enough time (newborn duties) to dig into the various scenarios around networking, as I think I need to review the different configs that should work as I'm not sure whether this aligns well with what the config should look like for the similar ipv4 network config in vagrant.

If you have a chance to put together the corresponding config for the corollary ipv4 config to help me along in ensuring the config behaviour is reasonably consistent and intuitive. E.g. if the ipv4 only approach uses some of the standard options then should support using those for Ipv6 by detecting whether what was provided in the config was ipv4 or ipv6 and then populate the template rather than rely on interpreting libvirt specific options as that is likely less intuitive to others looking to enable the same behaviour.

@rubenk
Copy link
Contributor Author

rubenk commented Jan 22, 2022

For ipv4 this works:

config.vm.network :private_network,
:ip => "10.10.11.1",
:libvirt__forward_mode => "route"

and this results in this xml:

virsh net-dumpxml archlinux0
<network connections='1' ipv6='yes'>
  <name>archlinux0</name>
  <uuid>08451ce4-4fc9-4442-a165-3325f6338fed</uuid>
  <forward mode='route'/>
  <bridge name='virbr2' stp='on' delay='0'/>
  <mac address='52:54:00:78:f5:bc'/>
  <ip address='10.10.11.1' netmask='255.255.255.0'>
    <dhcp>
      <range start='10.10.11.1' end='10.10.11.254'/>
    </dhcp>
  </ip>
</network>

but the thing is, you can have both ipv4 and ipv6 at the same time on a routed network. So detecting that :ip is an ipv6 address is not going to work for that scenario (which I think is the common case).

But I definitely agree that we should make things more intuitive where we can and rely less on libvirt-specific options.

@electrofelix
Copy link
Contributor

I think living with having the additional libvirt option for ipv6 when needing to support both is fine, it definitely looks like for the ipv6 only one though that it's worth doing a detection on the address passed in so that the following is all that is needed (unless ipv6 also needs explicit prefix defined) for the basic support:

config.vm.network :private_network,
:ip => "fc00::",
:libvirt__forward_mode => "route"

@rubenk
Copy link
Contributor Author

rubenk commented Jan 22, 2022

The template needs the ipv6 prefix and there is no default right now.

@electrofelix
Copy link
Contributor

@rubenk is there a sensible default for the prefix that could be used for ipv6? I'm not particularly familiar with it

@rubenk
Copy link
Contributor Author

rubenk commented Jan 22, 2022

/64 is a reasonable default. Please note that even if we detect ipv6 automatically the fix in this PR is still needed since it guards the dhcp block for ipv4.

@electrofelix
Copy link
Contributor

@rubenk fair enough, just that I suspect that when it's fully fixed likely will end up making either ip or dhcp required rather than relying on private vagrant-libvirt options to encourage use of standard options by default.

@electrofelix electrofelix merged commit ef0686d into vagrant-libvirt:master Jan 30, 2022
@rubenk
Copy link
Contributor Author

rubenk commented Jan 31, 2022

@electrofelix yes, that sounds good. I looked into adding some unit tests for this issue as well but there are no unit tests for networking yet. I don't think I'm confortable enough with ruby and rspec to write the framework for network xml tests. But if you have something basic that can do that I'm willing to write the unit tests for ipv6 support.

@electrofelix
Copy link
Contributor

@rubenk that's perfectly understandable, while I ask for tests in most areas I only do so if there are existing tests to that can be used as a reference otherwise it's not really fair for someone just trying to fix their issue to have to become an expert in how to test the existing code.

Hopefully at some stage soon it should be possible, I just have to wait until I have a bit of time where I can put together the initial bits for the network code.

@rubenk rubenk deleted the fix-ipv6-only-routed-network branch January 31, 2022 12:18
@rubenk
Copy link
Contributor Author

rubenk commented Jan 31, 2022

@electrofelix a bit off-topic, but are you planning to do a new release soon?

mmguero pushed a commit to mmguero-dev/vagrant-libvirt that referenced this pull request Jan 31, 2022
Trying to create a routed network with just ipv6 fails with:
Error occurred while creating new network: Call to virNetworkDefineXML failed: XML error: Missing required address attribute in network 'test'.

Since an ipv4 address should be optional for this scenario, make it so.

This makes the following configuration work:

  config.vm.network :private_network,
    :libvirt__network_name => "test",
    :libvirt__forward_mode => "route",
    :libvirt__dhcp_enabled => false,
    :libvirt__guest_ipv6 => "yes",
    :libvirt__ipv6_address => "fc00::",
    :libvirt__ipv6_prefix => "64"
@electrofelix
Copy link
Contributor

Probably be at least another 2 weeks, it's not changes going in that are needed but rather confidence that I'll be able to follow up any issues discovered relatively quickly which just isn't possible right now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants