Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

virtcontainers: Add vhost-user-net hotplug support#611

Closed
sboeuf wants to merge 2 commits intokata-containers:masterfrom
sboeuf:vhost_user_hp
Closed

virtcontainers: Add vhost-user-net hotplug support#611
sboeuf wants to merge 2 commits intokata-containers:masterfrom
sboeuf:vhost_user_hp

Conversation

@sboeuf
Copy link
Copy Markdown

@sboeuf sboeuf commented Aug 21, 2018

In order to fully move to network hotplug, we first need to support
vhost-user-net hotplug. This commit relies on appropriate QMP commands
to hotplug a vhost user network by first creating a new char device
pointing to the socket path, by creating the appropriate network
device, and finally by adding a virtio-net-pci device.

Fixes #610

Signed-off-by: Sebastien Boeuf sebastien.boeuf@intel.com

@egernst egernst added the review label Aug 21, 2018
@sboeuf
Copy link
Copy Markdown
Author

sboeuf commented Aug 21, 2018

/cc @egernst

@katacontainersbot
Copy link
Copy Markdown
Contributor

PSS Measurement:
Qemu: 169776 KB
Proxy: 4162 KB
Shim: 8812 KB

Memory inside container:
Total Memory: 2043464 KB
Free Memory: 2003564 KB

@opendev-zuul
Copy link
Copy Markdown

opendev-zuul bot commented Aug 21, 2018

Build failed (third-party-check pipeline) integration testing with
OpenStack. For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

@devimc
Copy link
Copy Markdown

devimc commented Aug 21, 2018

CI is not happy

--- FAIL: TestVhostUserEndpoint_HotAttach (0.00s)
	assertions.go:239: 
                          
	Error Trace:	network_test.go:494
		
	Error:      	An error is expected but got nil.

@sboeuf sboeuf force-pushed the vhost_user_hp branch 2 times, most recently from 3c92a3c to 9e2979f Compare August 21, 2018 15:14
@sboeuf
Copy link
Copy Markdown
Author

sboeuf commented Aug 21, 2018

@devimc should be fixed now!

@opendev-zuul
Copy link
Copy Markdown

opendev-zuul bot commented Aug 21, 2018

Build failed (third-party-check pipeline) integration testing with
OpenStack. For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

@opendev-zuul
Copy link
Copy Markdown

opendev-zuul bot commented Aug 21, 2018

Build failed (third-party-check pipeline) integration testing with
OpenStack. For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

@katacontainersbot
Copy link
Copy Markdown
Contributor

PSS Measurement:
Qemu: 169198 KB
Proxy: 4170 KB
Shim: 8791 KB

Memory inside container:
Total Memory: 2043464 KB
Free Memory: 2003564 KB

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 21, 2018

Codecov Report

Merging #611 into master will decrease coverage by 0.14%.
The diff coverage is 25.64%.

@@            Coverage Diff             @@
##           master     #611      +/-   ##
==========================================
- Coverage   64.69%   64.55%   -0.15%     
==========================================
  Files          84       84              
  Lines        9558     9594      +36     
==========================================
+ Hits         6184     6193       +9     
- Misses       2726     2752      +26     
- Partials      648      649       +1

Copy link
Copy Markdown

@devimc devimc left a comment

Choose a reason for hiding this comment

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

lgtm

@sboeuf
Copy link
Copy Markdown
Author

sboeuf commented Aug 21, 2018

Depends on kata-containers/govmm#39

Sebastien Boeuf added 2 commits August 22, 2018 09:55
The update of the govmm vendoring forced some changes in the code
virtcontainers/qemu.go, related to the new flag shared-rw.
In order to be able to share an image between several VMs, Qemu
version 2.10 and newest need this flag to be enabled, otherwise
the image is locked to be exclusively used by a single VM.

Shortlog:

21504d3 qemu/qmp: Add netdev_add with chardev support
ed34f61 Add some negative test cases for qmp.go
17cacc7 Add negative test cases for qemu.go
2706a07 qemu: Use the supplied context.Context for launching
e46092e qemu: Do not try and generate invalid RTC parameters
fcaf61d qemu/qmp: add vfio mediated device support
4461c45 disk: Add --share-rw option for hotplugging disks
6851999 qemu/qmp: add addr and bus to hotplug vsock devices

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
In order to fully move to network hotplug, we first need to support
vhost-user-net hotplug. This commit relies on appropriate QMP commands
to hotplug a vhost user network by first creating a new char device
pointing to the socket path, by creating the appropriate network
device, and finally by adding a virtio-net-pci device.

Fixes kata-containers#610

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
@katacontainersbot
Copy link
Copy Markdown
Contributor

PSS Measurement:
Qemu: 169636 KB
Proxy: 4160 KB
Shim: 8925 KB

Memory inside container:
Total Memory: 2043464 KB
Free Memory: 2003728 KB

@sboeuf
Copy link
Copy Markdown
Author

sboeuf commented Aug 22, 2018

@amshinde PTAL

@opendev-zuul
Copy link
Copy Markdown

opendev-zuul bot commented Aug 22, 2018

Build failed (third-party-check pipeline) integration testing with
OpenStack. For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

@@ -338,7 +338,24 @@ func (endpoint *VhostUserEndpoint) Detach(netNsCreated bool, netNsPath string) e

// HotAttach for vhostuser endpoint not supported yet
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@sboeuf Can you get rid of this comment now?

@sboeuf
Copy link
Copy Markdown
Author

sboeuf commented Sep 19, 2018

Need #758 to be fixed first.

@raravena80
Copy link
Copy Markdown
Member

@sboeuf any updates? 😄 looks like #758 is closed.

@sboeuf
Copy link
Copy Markdown
Author

sboeuf commented Sep 28, 2018

@raravena80 nope sorry, I'll work on it when I get some time.

@amshinde
Copy link
Copy Markdown
Member

amshinde commented Oct 8, 2018

@sboeuf any updates?

@sboeuf
Copy link
Copy Markdown
Author

sboeuf commented Oct 8, 2018

No, but this needs some time to be spent if we want it to be properly integrated with the device manager model.

@raravena80
Copy link
Copy Markdown
Member

@sboeuf ping, any updates? (From your weekly Kata herder)

@caoruidong
Copy link
Copy Markdown
Member

Check 🏁

@raravena80
Copy link
Copy Markdown
Member

@caoruidong @sboeuf ping, from your weekly Kata herder.

@raravena80
Copy link
Copy Markdown
Member

@caoruidong @sboeuf is this ready for merging?

@jodh-intel
Copy link
Copy Markdown

@sboeuf - could you update the branch to resolve the conflict?

@jodh-intel
Copy link
Copy Markdown

Ping @sboeuf.

@raravena80
Copy link
Copy Markdown
Member

ping @sboeuf

@jodh-intel
Copy link
Copy Markdown

@sboeuf - shall we close this one?

@sboeuf
Copy link
Copy Markdown
Author

sboeuf commented Feb 26, 2019

@jodh-intel yes let's close it and get back later to it if needed.

@sboeuf sboeuf closed this Feb 26, 2019
egernst pushed a commit to egernst/runtime that referenced this pull request Feb 9, 2021
Make NoPivotRoot depend on the actual FS type of `/` being rootfs,
instead of agent being the init process.
In this way, an initrd built without AGENT_INIT can still be used to run
containers.

Fixes: kata-containers#611

Signed-off-by: Marco Vedovati <mvedovati@suse.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature New functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants