From bb58571eaeb57244480dbfb92dfe54d1ce07b241 Mon Sep 17 00:00:00 2001 From: Chris Townsend Date: Wed, 19 Jun 2019 13:29:33 -0400 Subject: [PATCH 1/3] snap: Remove libvirt from the snap Maintaining libvirt in the snap causes burdens for not much gain. Anyone wanting to use libvirt can just use the one installed on the host. Fixes #482, fixes #846 --- snap-wrappers/bin/launch-libvirtd | 7 -- snap/hooks/install | 5 -- snap/hooks/post-refresh | 7 -- snap/snapcraft.yaml | 90 +------------------ .../libvirt/libvirt_virtual_machine.cpp | 8 -- .../libvirt_virtual_machine_factory.cpp | 31 ++++--- .../libvirt/libvirt_virtual_machine_factory.h | 3 +- tests/test_libvirt_backend.cpp | 5 +- 8 files changed, 26 insertions(+), 130 deletions(-) delete mode 100755 snap-wrappers/bin/launch-libvirtd delete mode 100755 snap/hooks/install delete mode 100755 snap/hooks/post-refresh diff --git a/snap-wrappers/bin/launch-libvirtd b/snap-wrappers/bin/launch-libvirtd deleted file mode 100755 index 87b4ed3879..0000000000 --- a/snap-wrappers/bin/launch-libvirtd +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/sh -e - -driver="$(snapctl get driver)" - -if [ "$driver" = "LIBVIRT" ]; then - exec "$SNAP/sbin/libvirtd" -fi diff --git a/snap/hooks/install b/snap/hooks/install deleted file mode 100755 index 3e64fef7bc..0000000000 --- a/snap/hooks/install +++ /dev/null @@ -1,5 +0,0 @@ -#!/bin/sh -set -e - -install -D $SNAP/var/snap/multipass/common/libvirt/libvirtd.conf $SNAP_COMMON/libvirt/libvirtd.conf -sed -i 's/unix_sock_group = "libvirtd"/unix_sock_group = "sudo"/' $SNAP_COMMON/libvirt/libvirtd.conf diff --git a/snap/hooks/post-refresh b/snap/hooks/post-refresh deleted file mode 100755 index 3847fe5eb8..0000000000 --- a/snap/hooks/post-refresh +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/sh -set -e - -if [ ! -f $SNAP_COMMON/libvirt/libvirtd.conf ]; then - install -D $SNAP/var/snap/multipass/common/libvirt/libvirtd.conf $SNAP_COMMON/libvirt/libvirtd.conf - sed -i 's/unix_sock_group = "libvirtd"/unix_sock_group = "sudo"/' $SNAP_COMMON/libvirt/libvirtd.conf -fi diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index d34145aa36..4f00d7bce0 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -23,7 +23,7 @@ apps: XDG_DATA_HOME: $SNAP_COMMON/data XDG_CACHE_HOME: $SNAP_COMMON/cache daemon: simple - after: [libvirt-bin] + plugs: [libvirt] multipass: environment: LD_LIBRARY_PATH: $SNAP/lib:$SNAP/lib/$SNAPCRAFT_ARCH_TRIPLET:$SNAP/usr/lib:$SNAP/usr/lib/$SNAPCRAFT_ARCH_TRIPLET @@ -37,18 +37,6 @@ apps: PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH QT_PLUGIN_PATH: $SNAP/usr/lib/$SNAPCRAFT_ARCH_TRIPLET/qt5/plugins command: bin/multipass-gui - libvirt-bin: - command: bin/launch-libvirtd - environment: - LD_LIBRARY_PATH: $SNAP/lib:$SNAP/lib/$SNAPCRAFT_ARCH_TRIPLET:$SNAP/usr/lib:$SNAP/usr/lib/$SNAPCRAFT_ARCH_TRIPLET - PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH - LC_ALL: C - daemon: simple - virsh: - command: bin/virsh - environment: - PATH: $SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$PATH - LC_ALL: C parts: qtbase5-dev: @@ -202,7 +190,6 @@ parts: - libqt5widgets5 - libharfbuzz0b - libfreetype6 - - libvirt - libxcb-xinerama0 plugin: cmake build-packages: @@ -213,6 +200,7 @@ parts: - git - golang - libsystemd-dev + - libvirt-dev stage-packages: - on amd64: [libgl1] - on i386: [libgl1] @@ -257,80 +245,6 @@ parts: stage-packages: - try: [msr-tools] - libvirt: - source: snap - source-subdir: libvirt-1.3.1 - plugin: autotools - build-packages: - - libxml2-dev - - libxml-libxml-perl - - libcurl4-gnutls-dev - - libncurses5-dev - - libreadline-dev - - zlib1g-dev - - libgcrypt20-dev - - libgnutls28-dev - - libyajl-dev - - libpcap0.8-dev - - libaudit-dev - - libdevmapper-dev - - libpciaccess-dev - - libnl-3-dev - - libnl-route-3-dev - - uuid-dev - - try: [libnuma-dev] - - python-all - - python-six - - wget - - dpkg-dev - stage-packages: - - dmidecode - - dnsmasq - - dnsmasq-utils - - ebtables - - libxml2 - - libyajl2 - - try: [libnuma1] - - libcurl3-gnutls - - libpciaccess0 - - pm-utils - configflags: - - --with-qemu - - --without-bhyve - - --without-xen - - --without-openvz - - --without-vmware - - --without-xenapi - - --without-esx - - --without-hyperv - - --without-lxc - - --without-vz - - --without-vbox - - --without-uml - - --without-sasl - - --without-storage-iscsi - - --without-storage-sheepdog - - --without-storage-rbd - - --without-storage-lvm - - --without-selinux - - --prefix=/snap/$SNAPCRAFT_PROJECT_NAME/current - - --localstatedir=/var/snap/$SNAPCRAFT_PROJECT_NAME/common - - --sysconfdir=/var/snap/$SNAPCRAFT_PROJECT_NAME/common - - DNSMASQ=/snap/$SNAPCRAFT_PROJECT_NAME/current/usr/sbin/dnsmasq - - DMIDECODE=/snap/$SNAPCRAFT_PROJECT_NAME/current/usr/sbin/dmidecode - - EBTABLES_PATH=/snap/$SNAPCRAFT_PROJECT_NAME/current/sbin/ebtables - override-pull: | - wget http://archive.ubuntu.com/ubuntu/pool/main/libv/libvirt/libvirt_1.3.1.orig.tar.gz - wget http://archive.ubuntu.com/ubuntu/pool/main/libv/libvirt/libvirt_1.3.1-1ubuntu10.26.debian.tar.xz - wget http://archive.ubuntu.com/ubuntu/pool/main/libv/libvirt/libvirt_1.3.1-1ubuntu10.26.dsc - dpkg-source -x libvirt*.dsc - organize: - # Hack to shift installed libvirt back to root of snap - # required to ensure that pathing to files etc works at - # runtime - # * is not used to avoid directory merge conflicts - snap/multipass/current/: ./ - network-utils: plugin: nil override-pull: "" diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp index 37e8275953..5467104044 100644 --- a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp +++ b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp @@ -123,14 +123,6 @@ auto generate_xml_config_for(const mp::VirtualMachineDescription& desc, const st auto qemu_path = fmt::format("/usr/bin/qemu-system-{}", arch); - auto snap = qgetenv("SNAP"); - if (!snap.isEmpty()) - { - auto snap_path = QDir(snap); - snap_path.cd("../current"); - qemu_path = fmt::format("{}{}", snap_path.path(), qemu_path); - } - return fmt::format( "\n" " {}\n" diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine_factory.cpp b/src/platform/backends/libvirt/libvirt_virtual_machine_factory.cpp index b49cf10aff..59383746a1 100644 --- a/src/platform/backends/libvirt/libvirt_virtual_machine_factory.cpp +++ b/src/platform/backends/libvirt/libvirt_virtual_machine_factory.cpp @@ -30,16 +30,6 @@ namespace { constexpr auto multipass_bridge_name = "mpvirtbr0"; -auto connect_to_libvirt_daemon() -{ - mp::LibVirtVirtualMachineFactory::ConnectionUPtr conn{virConnectOpen("qemu:///system"), virConnectClose}; - - if (conn == nullptr) - throw std::runtime_error("Cannot connect to libvirtd"); - - return conn; -} - auto generate_libvirt_bridge_xml_config(const mp::Path& data_dir, const std::string& bridge_name) { auto network_dir = mp::utils::make_dir(QDir(data_dir), "network"); @@ -88,14 +78,28 @@ std::string enable_libvirt_network(virConnectPtr connection, const mp::Path& dat mp::LibVirtVirtualMachineFactory::LibVirtVirtualMachineFactory(const ProcessFactory* process_factory, const mp::Path& data_dir) : process_factory{process_factory}, - connection{connect_to_libvirt_daemon()}, - bridge_name{enable_libvirt_network(connection.get(), data_dir)} + connection{virConnectOpen("qemu:///system"), virConnectClose}, + data_dir{data_dir} { + if (connection) + { + bridge_name = enable_libvirt_network(connection.get(), data_dir); + } } mp::VirtualMachine::UPtr mp::LibVirtVirtualMachineFactory::create_virtual_machine(const VirtualMachineDescription& desc, VMStatusMonitor& monitor) { + if (!connection) + { + connection.reset(virConnectOpen("qemu:///system")); + + if (!connection) + throw std::runtime_error("Cannot connect to libvirtd. Please ensure libvirt is installed and running."); + + bridge_name = enable_libvirt_network(connection.get(), data_dir); + } + return std::make_unique(desc, connection.get(), bridge_name, monitor); } @@ -111,7 +115,8 @@ mp::LibVirtVirtualMachineFactory::~LibVirtVirtualMachineFactory() void mp::LibVirtVirtualMachineFactory::remove_resources_for(const std::string& name) { - virDomainUndefine(virDomainLookupByName(connection.get(), name.c_str())); + if (connection) + virDomainUndefine(virDomainLookupByName(connection.get(), name.c_str())); } mp::FetchType mp::LibVirtVirtualMachineFactory::fetch_type() diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine_factory.h b/src/platform/backends/libvirt/libvirt_virtual_machine_factory.h index 52d68c0afe..511d87688b 100644 --- a/src/platform/backends/libvirt/libvirt_virtual_machine_factory.h +++ b/src/platform/backends/libvirt/libvirt_virtual_machine_factory.h @@ -51,7 +51,8 @@ class LibVirtVirtualMachineFactory final : public VirtualMachineFactory private: const ProcessFactory* process_factory; ConnectionUPtr connection; - const std::string bridge_name; + const Path data_dir; + std::string bridge_name; }; } // namespace multipass diff --git a/tests/test_libvirt_backend.cpp b/tests/test_libvirt_backend.cpp index a0737b17e4..8acd984d90 100644 --- a/tests/test_libvirt_backend.cpp +++ b/tests/test_libvirt_backend.cpp @@ -80,7 +80,10 @@ struct LibVirtBackend : public Test TEST_F(LibVirtBackend, failed_connection_throws) { REPLACE(virConnectOpen, [](auto...) { return nullptr; }); - EXPECT_THROW(mp::LibVirtVirtualMachineFactory backend(&process_factory, data_dir.path()), std::runtime_error); + + mp::LibVirtVirtualMachineFactory backend(&process_factory, data_dir.path()); + mpt::StubVMStatusMonitor stub_monitor; + EXPECT_THROW(backend.create_virtual_machine(default_description, stub_monitor), std::runtime_error); } TEST_F(LibVirtBackend, creates_in_off_state) From d65a2af84bb5ccef6c8a2538fcd3df1fac900582 Mon Sep 17 00:00:00 2001 From: Chris Townsend Date: Wed, 19 Jun 2019 14:28:29 -0400 Subject: [PATCH 2/3] travis: Fix up patches --- tests/travis-Coverage.patch | 6 +++--- tests/travis-Debug.patch | 2 +- tests/travis.patch | 22 ++-------------------- 3 files changed, 6 insertions(+), 24 deletions(-) diff --git a/tests/travis-Coverage.patch b/tests/travis-Coverage.patch index 4398795b10..1fa7d3d377 100644 --- a/tests/travis-Coverage.patch +++ b/tests/travis-Coverage.patch @@ -1,14 +1,14 @@ --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml -@@ -208,6 +208,7 @@ parts: - - git +@@ -201,6 +201,7 @@ parts: - golang - libsystemd-dev + - libvirt-dev + - lcov stage-packages: - on amd64: [libgl1] - on i386: [libgl1] -@@ -216,9 +217,8 @@ parts: +@@ -209,9 +210,8 @@ parts: - libpng16-16 source: . configflags: diff --git a/tests/travis-Debug.patch b/tests/travis-Debug.patch index 4a27ba641f..bea9aa9752 100644 --- a/tests/travis-Debug.patch +++ b/tests/travis-Debug.patch @@ -1,6 +1,6 @@ --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml -@@ -216,9 +216,8 @@ parts: +@@ -209,9 +209,8 @@ parts: - libpng16-16 source: . configflags: diff --git a/tests/travis.patch b/tests/travis.patch index 02b608f28c..67b357c68d 100644 --- a/tests/travis.patch +++ b/tests/travis.patch @@ -1,6 +1,6 @@ --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml -@@ -212,6 +212,7 @@ parts: +@@ -196,6 +196,7 @@ parts: - on arm64: [libgles2-mesa-dev] - on armhf: [libgles2-mesa-dev] - build-essential @@ -8,7 +8,7 @@ - cmake-extras - git - golang -@@ -228,6 +229,7 @@ parts: +@@ -213,6 +214,7 @@ parts: - -DCMAKE_INSTALL_PREFIX=/ - -DMULTIPASS_ENABLE_TESTS=off override-build: | @@ -16,21 +16,3 @@ snapcraftctl build set -e mkdir -p ${SNAPCRAFT_PART_INSTALL}/etc/bash_completion.d/ -@@ -265,6 +267,7 @@ parts: - source-subdir: libvirt-1.3.1 - plugin: autotools - build-packages: -+ - ccache - - libxml2-dev - - libxml-libxml-perl - - libcurl4-gnutls-dev -@@ -327,6 +330,9 @@ parts: - wget http://archive.ubuntu.com/ubuntu/pool/main/libv/libvirt/libvirt_1.3.1-1ubuntu10.26.debian.tar.xz - wget http://archive.ubuntu.com/ubuntu/pool/main/libv/libvirt/libvirt_1.3.1-1ubuntu10.26.dsc - dpkg-source -x libvirt*.dsc -+ override-build: | -+ update-ccache-symlinks -+ snapcraftctl build - organize: - # Hack to shift installed libvirt back to root of snap - # required to ensure that pathing to files etc works at From 1ca55e29486b6a51a6fa0461a604df44c889b96c Mon Sep 17 00:00:00 2001 From: Chris Townsend Date: Mon, 24 Jun 2019 12:34:47 -0400 Subject: [PATCH 3/3] snap: Add back in some stage-packages removed when removing libvirt --- snap/snapcraft.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index 4f00d7bce0..fe54e9f766 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -207,6 +207,9 @@ parts: - on armhf: [libgles2-mesa] - on arm64: [libgles2-mesa] - libpng16-16 + - libxml2 + - libvirt0 + - dnsmasq source: . configflags: - -DCMAKE_BUILD_TYPE=RelWithDebInfo