Skip to content

snap: Remove libvirt from the snap#850

Merged
bors[bot] merged 3 commits intomasterfrom
remove-libvirt-from-snap
Jun 25, 2019
Merged

snap: Remove libvirt from the snap#850
bors[bot] merged 3 commits intomasterfrom
remove-libvirt-from-snap

Conversation

@townsend2010
Copy link
Contributor

@townsend2010 townsend2010 commented Jun 19, 2019

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

@ricab ricab self-requested a review June 21, 2019 19:20
@Saviq Saviq mentioned this pull request Jun 24, 2019
Closed
@Saviq
Copy link
Collaborator

Saviq commented Jun 24, 2019 •
edited
Loading

I built the snap clean and got this:

The 'multipass' part needs the following libraries that are not included in the snap or base: 
usr/lib/x86_64-linux-gnu/libavahi-client.so.3
usr/lib/x86_64-linux-gnu/libavahi-common.so.3
usr/lib/x86_64-linux-gnu/libicudata.so.55
usr/lib/x86_64-linux-gnu/libicuuc.so.55
usr/lib/x86_64-linux-gnu/libvirt.so.0
usr/lib/x86_64-linux-gnu/libxml2.so.2
These dependencies can be satisfied via more stage-packages, more parts, or content sharing.
…
multipass list
/snap/multipass/x4/bin/multipass: /snap/core/current/lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.28' not found (required by /usr/lib/x86_64-linux-gnu/libxml2.so.2)

Looks like we lost some dependencies.

Chris Townsend added 3 commits June 24, 2019 12:35
snap: Remove libvirt from the snap
bb58571 
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
travis: Fix up patches
d65a2af
snap: Add back in some stage-packages removed when removing libvirt
1ca55e2
@townsend2010 townsend2010 force-pushed the remove-libvirt-from-snap branch from 476fd64 to 1ca55e2 Compare June 24, 2019 16:37
@codecov
Copy link

codecov bot commented Jun 24, 2019

Codecov Report

Merging #850 into master will increase coverage by 0.03%.
The diff coverage is 70%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #850      +/-   ##
==========================================
+ Coverage   68.64%   68.67%   +0.03%     
==========================================
  Files         192      192              
  Lines        6608     6606       -2     
==========================================
+ Hits         4536     4537       +1     
+ Misses       2072     2069       -3
Impacted Files Coverage Δ
...tform/backends/libvirt/libvirt_virtual_machine.cpp 43.44% <ø> (+1.44%) ⬆️
...backends/libvirt/libvirt_virtual_machine_factory.h 0% <ø> (ø) ⬆️
...ckends/libvirt/libvirt_virtual_machine_factory.cpp 42.22% <70%> (-0.64%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f4e951...1ca55e2. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Jun 24, 2019 •
edited
Loading

Codecov Report

Merging #850 into master will increase coverage by 0.03%.
The diff coverage is 70%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #850      +/-   ##
==========================================
+ Coverage   68.64%   68.67%   +0.03%     
==========================================
  Files         192      192              
  Lines        6608     6606       -2     
==========================================
+ Hits         4536     4537       +1     
+ Misses       2072     2069       -3
Impacted Files Coverage Δ
...tform/backends/libvirt/libvirt_virtual_machine.cpp 43.44% <ø> (+1.44%) ⬆️
...backends/libvirt/libvirt_virtual_machine_factory.h 0% <ø> (ø) ⬆️
...ckends/libvirt/libvirt_virtual_machine_factory.cpp 42.22% <70%> (-0.64%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f4e951...1ca55e2. Read the comment docs.

@Saviq
Copy link
Collaborator

Saviq commented Jun 24, 2019 •
edited
Loading

In general this works and the only issue I found is most likely preexisting:

  1. snap set multipass driver=LIBVIRT
  2. launch an instance
  3. stop the instance
  4. snap set multipass driver=QEMU

The domain remains defined in libvirt, which may result in weird situations when someone tries to manage it with virsh / virt-manager or some such.

Sorry, fat fingers.

@Saviq Saviq closed this Jun 24, 2019
@Saviq Saviq reopened this Jun 24, 2019
@townsend2010
Copy link
Contributor Author

townsend2010 commented Jun 24, 2019

@Saviq,

The domain remains defined in libvirt

Yes, this is an already occurring issue. multipassd is ignorant about which backend it used in the previous running instance of the process...which is really the crux of the issue in #854.

@Saviq
Copy link
Collaborator

Saviq commented Jun 25, 2019

A'ight!

bors r+

bors bot added a commit that referenced this pull request Jun 25, 2019
@bors
Merge #850
95579cf 
850: snap: Remove libvirt from the snap r=Saviq a=townsend2010

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

Co-authored-by: Chris Townsend <christopher.townsend@canonical.com>
@bors
Copy link
Contributor

bors bot commented Jun 25, 2019

Build succeeded

@bors bors bot merged commit 1ca55e2 into master Jun 25, 2019
@bors bors bot deleted the remove-libvirt-from-snap branch June 25, 2019 14:44
@ricab ricab removed their request for review June 25, 2019 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

snap: Remove libvirt from the snap Connect to the libvirt interface

2 participants

@townsend2010 @Saviq