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

Jump to bottom

libvirt: 6.6.0 -> 6.8.0 #109332

Merged
merged 2 commits into from Jan 14, 2021
Merged

libvirt: 6.6.0 -> 6.8.0 #109332

merged 2 commits into from Jan 14, 2021

Conversation

euank
Copy link
Member

@euank euank commented Jan 14, 2021 •
edited

Motivation for this change

This PR carries forward #103309 with the same motivation (fixes a bug in libvirt that causes it to fail to start on non-btrfs setups).

Extra notes

There's a few odd things about this package still. The meson build patch is pretty intrusive, but I'm overall okay with that one I think.

The fact that it uses /var/lib instead of /etc for sysconfdir is also a little abnormal. The reason I'm leaving it like that is because the nixos module expects it, and I think it may be better to update the module separately from this meson-build-system change.

I've only done minimal testing of this, specifically the following:

  1. ran libvirtd using the nixos module + this package
  2. Used 'virt-manager' to run a nixos-minimal ISO, and verify it booted with the default nat networking

Of course, that's still better than 6.6.0 for me since 6.6.0 couldn't even get to the point of creating a disk image due to a bug, let alone booting something.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested review from fpletz and globin January 14, 2021 02:56
@euank euank marked this pull request as ready for review January 14, 2021 11:11
@euank euank mentioned this pull request Jan 14, 2021
Closed
10 tasks
@euank euank force-pushed the libvirt-update branch 2 times, most recently from 06fe650 to 9ed4117 Compare January 14, 2021 11:19
@euank
libvirt: fixups for meson build
60b30f6 
The previous commit updates to a newer libvirt with a newer build setup.

This commit carries forward that work into a mergeable state.

Based on the suggestion in
NixOS#103309 (comment), I
did a fwupd-like patch for the various meson.build files.
@jonringer
Copy link
Contributor

@GrahamcOfBorg test podman

jonringer
jonringer approved these changes Jan 14, 2021
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

diff LGTM, assuming tests pass

I think the patch should be upstreamed, seems to just be restoring functionality which was available in their autotools toolchain.

@jonringer
Copy link
Contributor

failures are broken on master

https://github.com/NixOS/nixpkgs/pull/109332

1 package failed to build:
libvmi

30 packages built:
collectd collectd-data docker-machine-kvm docker-machine-kvm2 gnome3.gnome-boxes haskellPackages.libvirt-hs libguestfs libvirt libvirt-glib minikube minishift perl530Packages.SysVirt perl532Packages.SysVirt python37Packages.guestfs python37Packages.libvirt python38Packages.guestfs python38Packages.libvirt python39Packages.guestfs python39Packages.libvirt rubyPackages.ruby-libvirt rubyPackages_2_5.ruby-libvirt rubyPackages_2_7.ruby-libvirt terraform-full terraform-providers.libvirt vagrant virt-manager virt-manager-qt virt-top virt-viewer virtlyst

@jonringer jonringer merged commit 5ecaaf1 into NixOS:master Jan 14, 2021
This was referenced Jan 15, 2021
Merged
Closed
@pmeiyu
Copy link
Member

pmeiyu commented Jan 18, 2021

I think the libvirtd firewall rules are broken after this change. My VMs cannot access Internet. iptables is replaced with iptables-nftables-compat in this patch, so libvirtd firewall rules are automatically converted into nftables rules. But nftables is not working well.

sudo nft list ruleset:

table ip filter {
	chain INPUT {
		type filter hook input priority filter; policy accept;
		counter packets 101704 bytes 38404913 jump LIBVIRT_INP
	}

	chain FORWARD {
		type filter hook forward priority filter; policy accept;
		counter packets 1110 bytes 75398 jump LIBVIRT_FWX
		counter packets 1110 bytes 75398 jump LIBVIRT_FWI
		counter packets 1110 bytes 75398 jump LIBVIRT_FWO
	}

	chain OUTPUT {
		type filter hook output priority filter; policy accept;
		counter packets 108355 bytes 32463097 jump LIBVIRT_OUT
	}

	chain LIBVIRT_INP {
		iifname "virbr0" meta l4proto udp # xt_udp counter packets 518 bytes 34994 accept
		iifname "virbr0" meta l4proto tcp # xt_tcp counter packets 0 bytes 0 accept
		iifname "virbr0" meta l4proto udp # xt_udp counter packets 23 bytes 7954 accept
		iifname "virbr0" meta l4proto tcp # xt_tcp counter packets 0 bytes 0 accept
	}

	chain LIBVIRT_OUT {
		oifname "virbr0" meta l4proto udp # xt_udp counter packets 0 bytes 0 accept
		oifname "virbr0" meta l4proto tcp # xt_tcp counter packets 0 bytes 0 accept
		oifname "virbr0" meta l4proto udp # xt_udp counter packets 0 bytes 0 accept
		oifname "virbr0" meta l4proto tcp # xt_tcp counter packets 0 bytes 0 accept
	}

	chain LIBVIRT_FWO {
		iifname "virbr0" ip saddr 192.168.122.0/24 counter packets 461 bytes 23972 accept
		iifname "virbr0" counter packets 0 bytes 0 # xt_REJECT
	}

	chain LIBVIRT_FWI {
		oifname "virbr0" ip daddr 192.168.122.0/24 # xt_conntrack counter packets 0 bytes 0 accept
		oifname "virbr0" counter packets 0 bytes 0 # xt_REJECT
	}

	chain LIBVIRT_FWX {
		iifname "virbr0" oifname "virbr0" counter packets 0 bytes 0 accept
	}
}
table ip6 filter {
	chain INPUT {
		type filter hook input priority filter; policy accept;
		counter packets 15942 bytes 14562354 jump LIBVIRT_INP
	}

	chain FORWARD {
		type filter hook forward priority filter; policy accept;
		counter packets 0 bytes 0 jump LIBVIRT_FWX
		counter packets 0 bytes 0 jump LIBVIRT_FWI
		counter packets 0 bytes 0 jump LIBVIRT_FWO
	}

	chain OUTPUT {
		type filter hook output priority filter; policy accept;
		counter packets 14276 bytes 14295702 jump LIBVIRT_OUT
	}

	chain LIBVIRT_INP {
	}

	chain LIBVIRT_OUT {
	}

	chain LIBVIRT_FWO {
	}

	chain LIBVIRT_FWI {
	}

	chain LIBVIRT_FWX {
	}
}
table bridge filter {
	chain INPUT {
		type filter hook input priority filter; policy accept;
	}

	chain FORWARD {
		type filter hook forward priority filter; policy accept;
	}

	chain OUTPUT {
		type filter hook output priority filter; policy accept;
	}
}
table ip nat {
	chain PREROUTING {
		type nat hook prerouting priority dstnat; policy accept;
	}

	chain INPUT {
		type nat hook input priority 100; policy accept;
	}

	chain POSTROUTING {
		type nat hook postrouting priority srcnat; policy accept;
		counter packets 6084 bytes 693325 jump LIBVIRT_PRT
	}

	chain OUTPUT {
		type nat hook output priority -100; policy accept;
	}

	chain LIBVIRT_PRT {
		ip saddr 192.168.122.0/24 ip daddr 224.0.0.0/24 counter packets 3 bytes 186 return
		ip saddr 192.168.122.0/24 ip daddr 255.255.255.255 counter packets 0 bytes 0 return
		meta l4proto tcp ip saddr 192.168.122.0/24 ip daddr != 192.168.122.0/24 counter packets 0 bytes 0 # xt_MASQUERADE
		meta l4proto udp ip saddr 192.168.122.0/24 ip daddr != 192.168.122.0/24 counter packets 2 bytes 390 # xt_MASQUERADE
		ip saddr 192.168.122.0/24 ip daddr != 192.168.122.0/24 counter packets 0 bytes 0 # xt_MASQUERADE
	}
}
table ip mangle {
	chain PREROUTING {
		type filter hook prerouting priority mangle; policy accept;
	}

	chain INPUT {
		type filter hook input priority mangle; policy accept;
	}

	chain FORWARD {
		type filter hook forward priority mangle; policy accept;
	}

	chain OUTPUT {
		type route hook output priority mangle; policy accept;
	}

	chain POSTROUTING {
		type filter hook postrouting priority mangle; policy accept;
		counter packets 108842 bytes 32630537 jump LIBVIRT_PRT
	}

	chain LIBVIRT_PRT {
		oifname "virbr0" meta l4proto udp # xt_udp counter packets 0 bytes 0 # xt_CHECKSUM
	}
}
table ip6 nat {
	chain PREROUTING {
		type nat hook prerouting priority dstnat; policy accept;
	}

	chain INPUT {
		type nat hook input priority 100; policy accept;
	}

	chain POSTROUTING {
		type nat hook postrouting priority srcnat; policy accept;
		counter packets 1155 bytes 194651 jump LIBVIRT_PRT
	}

	chain OUTPUT {
		type nat hook output priority -100; policy accept;
	}

	chain LIBVIRT_PRT {
	}
}
table ip6 mangle {
	chain PREROUTING {
		type filter hook prerouting priority mangle; policy accept;
	}

	chain INPUT {
		type filter hook input priority mangle; policy accept;
	}

	chain FORWARD {
		type filter hook forward priority mangle; policy accept;
	}

	chain OUTPUT {
		type route hook output priority mangle; policy accept;
	}

	chain POSTROUTING {
		type filter hook postrouting priority mangle; policy accept;
		counter packets 14492 bytes 14395504 jump LIBVIRT_PRT
	}

	chain LIBVIRT_PRT {
	}
}

@euank euank mentioned this pull request Jan 18, 2021
Merged
10 tasks
@euank
Copy link
Member Author

euank commented Jan 18, 2021

Hey @pmeiyu thanks for the heads up it caused an issue. I've reverted that bit over here: #109722

Can you check if that PR fixes the networking issue you're running into so we can make sure that it's due to that change, and not some other change in libvirt 6.8.0?

@pmeiyu
Copy link
Member

pmeiyu commented Jan 18, 2021

Hi @euank, Look at the comment in nftables ruleset:

	chain LIBVIRT_PRT {
		ip saddr 192.168.122.0/24 ip daddr 224.0.0.0/24 counter packets 3 bytes 186 return
		ip saddr 192.168.122.0/24 ip daddr 255.255.255.255 counter packets 0 bytes 0 return
		meta l4proto tcp ip saddr 192.168.122.0/24 ip daddr != 192.168.122.0/24 counter packets 0 bytes 0 # xt_MASQUERADE
		meta l4proto udp ip saddr 192.168.122.0/24 ip daddr != 192.168.122.0/24 counter packets 2 bytes 390 # xt_MASQUERADE
		ip saddr 192.168.122.0/24 ip daddr != 192.168.122.0/24 counter packets 0 bytes 0 # xt_MASQUERADE
	}

# xt_MASQUERADE is commented out, so it's not a real nftables command. After searching online, I find out setting nftables package's withXtables to true should solve some problem. But I don't know if it will make libvirt work well.

@pmeiyu
Copy link
Member

pmeiyu commented Jan 18, 2021 •
edited

I override nftables package's withXtables to true. Now the output of sudo nft list ruleset is normal.

And I find out VM's internet connection problem is caused by conflicts between iptables and nftables. iptables DROPs forwarded packets and it can override nftables rules. When I tell iptables to allow forwarded packets, the VMs can access Internet.

table ip filter {
	chain INPUT {
		type filter hook input priority filter; policy accept;
		counter packets 734690 bytes 302936459 jump LIBVIRT_INP
	}

	chain FORWARD {
		type filter hook forward priority filter; policy accept;
		counter packets 1296 bytes 85070 jump LIBVIRT_FWX
		counter packets 1296 bytes 85070 jump LIBVIRT_FWI
		counter packets 1296 bytes 85070 jump LIBVIRT_FWO
	}

	chain OUTPUT {
		type filter hook output priority filter; policy accept;
		counter packets 760197 bytes 247465539 jump LIBVIRT_OUT
	}

	chain LIBVIRT_INP {
		iifname "virbr0" meta l4proto udp udp dport 53 counter packets 693 bytes 46664 accept
		iifname "virbr0" meta l4proto tcp tcp dport 53 counter packets 0 bytes 0 accept
		iifname "virbr0" meta l4proto udp udp dport 67 counter packets 915 bytes 309258 accept
		iifname "virbr0" meta l4proto tcp tcp dport 67 counter packets 0 bytes 0 accept
	}

	chain LIBVIRT_OUT {
		oifname "virbr0" meta l4proto udp udp dport 53 counter packets 0 bytes 0 accept
		oifname "virbr0" meta l4proto tcp tcp dport 53 counter packets 0 bytes 0 accept
		oifname "virbr0" meta l4proto udp udp dport 68 counter packets 0 bytes 0 accept
		oifname "virbr0" meta l4proto tcp tcp dport 68 counter packets 0 bytes 0 accept
	}

	chain LIBVIRT_FWO {
		iifname "virbr0" ip saddr 192.168.122.0/24 counter packets 647 bytes 33644 accept
		iifname "virbr0" counter packets 0 bytes 0 reject
	}

	chain LIBVIRT_FWI {
		oifname "virbr0" ip daddr 192.168.122.0/24 ct state related,established counter packets 0 bytes 0 accept
		oifname "virbr0" counter packets 0 bytes 0 reject
	}

	chain LIBVIRT_FWX {
		iifname "virbr0" oifname "virbr0" counter packets 0 bytes 0 accept
	}
}
table ip6 filter {
	chain INPUT {
		type filter hook input priority filter; policy accept;
		counter packets 130625 bytes 120978722 jump LIBVIRT_INP
	}

	chain FORWARD {
		type filter hook forward priority filter; policy accept;
		counter packets 0 bytes 0 jump LIBVIRT_FWX
		counter packets 0 bytes 0 jump LIBVIRT_FWI
		counter packets 0 bytes 0 jump LIBVIRT_FWO
	}

	chain OUTPUT {
		type filter hook output priority filter; policy accept;
		counter packets 115950 bytes 118718908 jump LIBVIRT_OUT
	}

	chain LIBVIRT_INP {
	}

	chain LIBVIRT_OUT {
	}

	chain LIBVIRT_FWO {
	}

	chain LIBVIRT_FWI {
	}

	chain LIBVIRT_FWX {
	}
}
table bridge filter {
	chain INPUT {
		type filter hook input priority filter; policy accept;
	}

	chain FORWARD {
		type filter hook forward priority filter; policy accept;
	}

	chain OUTPUT {
		type filter hook output priority filter; policy accept;
	}
}
table ip nat {
	chain PREROUTING {
		type nat hook prerouting priority dstnat; policy accept;
	}

	chain INPUT {
		type nat hook input priority 100; policy accept;
	}

	chain POSTROUTING {
		type nat hook postrouting priority srcnat; policy accept;
		counter packets 38026 bytes 4683958 jump LIBVIRT_PRT
	}

	chain OUTPUT {
		type nat hook output priority -100; policy accept;
	}

	chain LIBVIRT_PRT {
		ip saddr 192.168.122.0/24 ip daddr 224.0.0.0/24 counter packets 18 bytes 951 return
		ip saddr 192.168.122.0/24 ip daddr 255.255.255.255 counter packets 0 bytes 0 return
		meta l4proto tcp ip saddr 192.168.122.0/24 ip daddr != 192.168.122.0/24 counter packets 0 bytes 0 masquerade to :1024-65535
		meta l4proto udp ip saddr 192.168.122.0/24 ip daddr != 192.168.122.0/24 counter packets 22 bytes 4290 masquerade to :1024-65535
		ip saddr 192.168.122.0/24 ip daddr != 192.168.122.0/24 counter packets 0 bytes 0 masquerade
	}
}
table ip mangle {
	chain PREROUTING {
		type filter hook prerouting priority mangle; policy accept;
	}

	chain INPUT {
		type filter hook input priority mangle; policy accept;
	}

	chain FORWARD {
		type filter hook forward priority mangle; policy accept;
	}

	chain OUTPUT {
		type route hook output priority mangle; policy accept;
	}

	chain POSTROUTING {
		type filter hook postrouting priority mangle; policy accept;
		counter packets 762673 bytes 248791561 jump LIBVIRT_PRT
	}

	chain LIBVIRT_PRT {
		oifname "virbr0" meta l4proto udp udp dport 68 counter packets 0 bytes 0 # CHECKSUM fill
	}
}
table ip6 nat {
	chain PREROUTING {
		type nat hook prerouting priority dstnat; policy accept;
	}

	chain INPUT {
		type nat hook input priority 100; policy accept;
	}

	chain POSTROUTING {
		type nat hook postrouting priority srcnat; policy accept;
		counter packets 8157 bytes 1579246 jump LIBVIRT_PRT
	}

	chain OUTPUT {
		type nat hook output priority -100; policy accept;
	}

	chain LIBVIRT_PRT {
	}
}
table ip6 mangle {
	chain PREROUTING {
		type filter hook prerouting priority mangle; policy accept;
	}

	chain INPUT {
		type filter hook input priority mangle; policy accept;
	}

	chain FORWARD {
		type filter hook forward priority mangle; policy accept;
	}

	chain OUTPUT {
		type route hook output priority mangle; policy accept;
	}

	chain POSTROUTING {
		type filter hook postrouting priority mangle; policy accept;
		counter packets 117388 bytes 119590636 jump LIBVIRT_PRT
	}

	chain LIBVIRT_PRT {
	}
}

@pmeiyu
Copy link
Member

pmeiyu commented Jan 18, 2021

My iptables' forward policy is DROP. It conflicts with nftables, so nftables is not working well.

According to 1, mixing iptables and nftables is problematic. The default firewall in NixOS is still iptables so I think other people will also be affected by problem. We should either warn people about the problem or revert back to iptables for now.

euank added a commit to euank/nixpkgs that referenced this pull request Jan 22, 2021
@euank
libvirt: don't use iptables-nftables
f0b1cdb 
Per a comment on the PR that made this change, it turns out to cause
issues in some cases: NixOS#109332 (comment)

For now, let's revert back. Presumably the issues derive from the system
iptables not matching libvirt's iptables.

In the future, NixOS#81172 should move us back into the future, and I'm
perfectly fine waiting for that PR to handle this separately.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@jonringer jonringer jonringer approved these changes

@fpletz fpletz Awaiting requested review from fpletz

@globin globin Awaiting requested review from globin

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: python 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@euank @jonringer @pmeiyu @SuperSandro2000 @teto