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

qemu: Add rng virtio device #676

Merged
merged 3 commits into from Sep 11, 2018
Merged

Conversation

jcvenegas
Copy link
Member

Kata Containers does not provide good entropy. Enable a paravirtual rng device to provide a random number generator source.

Depens on: kata-containers/govmm#45

Fixes: #445

@egernst egernst added the review label Aug 30, 2018
@jodh-intel
Copy link
Contributor

jodh-intel commented Aug 30, 2018

lgtm

Approved with PullApprove

@opendev-zuul
Copy link

opendev-zuul bot commented Aug 30, 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
Contributor

PSS Measurement:
Qemu: 165775 KB
Proxy: 4085 KB
Shim: 8761 KB

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

@devimc
Copy link

devimc commented Aug 30, 2018

I wonder if this works with vm templating
cc @bergwolf

@WeiZhang555
Copy link
Member

Retrigger CI now...

@bergwolf
Copy link
Member

Does it require a new version of QEMU? I'm getting this error
docker: Error response from daemon: OCI runtime create failed: qemu-system-x86_64-v2.11.1-template: Property '.static-prt' not found: unknown
w/ or w/o vm templating.

@jodh-intel
Copy link
Contributor

@bergwolf - looks like you are using a "special build" of qemu? fwics, you have to use qemu-lite or qemu-vanilla for this feature.

@bergwolf
Copy link
Member

@jodh-intel I was using self-built qemu-lite. I guess I was missing some qemu feature configs. I'll retry with kata's qemu-lite package.

@jodh-intel
Copy link
Contributor

@jodh-intel
Copy link
Contributor

@bergwolf - see #445 (comment).

@bergwolf
Copy link
Member

@jodh-intel I was using q35 not pc yet still hit the problem described in #445 (comment). So qemu-lite breaks virtio rng device?

@jodh-intel
Copy link
Contributor

On an Ubuntu 18.04 test box, this PR:

  • works with qemu-lite.
  • works with qemu-vanilla.
  • fails with the same error you got using the Ubuntu-packaged version of qemu (qemu-system-x86 version 1:2.11+dfsg-1ubuntu7.4).

@jcvenegas
Copy link
Member Author

@bergwolf @jodh-intel yes qemu-lite breaks virtio-rng, meanwhile is fixed, I added created this PR:
kata-containers/qemu#3

@opendev-zuul
Copy link

opendev-zuul bot commented Aug 31, 2018

Build succeeded (third-party-check pipeline).

@opendev-zuul
Copy link

opendev-zuul bot commented Sep 3, 2018

Build succeeded (third-party-check pipeline).

@@ -20,7 +20,7 @@ const defaultQemuPath = "/usr/bin/qemu-system-x86_64"

const defaultQemuMachineType = QemuPC

const defaultQemuMachineOptions = "accel=kvm,kernel_irqchip,nvdimm"
const defaultQemuMachineOptions = "accel=kvm,kernel_irqchip,nvdimm,nostatic_prt"
Copy link

Choose a reason for hiding this comment

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

@jcvenegas why do you define nostatic_prt as a default option for pc machine type? I thought you said this was only introduced by the very specific qemu-lite branch of qemu. What if we use a vanilla qemu?
I just want to make sure we won't get an error from a vanilla Qemu because we're using a non defined flag by default here.

@jcvenegas
Copy link
Member Author

@sboeuf agree, now that qemu-lite branch static_prt is false by default that would not be an issue, I removed the commit, now I need to debug why is failing the CI. I see my PR working locally. And it used to work the first day I created the PR.

@opendev-zuul
Copy link

opendev-zuul bot commented Sep 4, 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

@codecov
Copy link

codecov bot commented Sep 4, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@8f5fec8). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master     #676   +/-   ##
=========================================
  Coverage          ?   64.97%           
=========================================
  Files             ?       85           
  Lines             ?    10925           
  Branches          ?        0           
=========================================
  Hits              ?     7098           
  Misses            ?     3133           
  Partials          ?      694

@opendev-zuul
Copy link

opendev-zuul bot commented Sep 4, 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

opendev-zuul bot commented Sep 4, 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

@jcvenegas
Copy link
Member Author

jcvenegas commented Sep 5, 2018

update, the PR is failing randomly due to sometimes the boot times take about 30 seconds in the CI machines.

[    0.354048] ACPI: PCI Interrupt Link [LNKB] enabled at IRQ 11
[    0.354752] virtio-pci 0000:00:06.0: virtio_pci: leaving for legacy driver
[    0.389049] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[    0.441705] console [hvc0] enabled
[   29.446221] brd: module loaded
[   29.448989] loop: module loaded
[   29.450840]  pmem0: p1
[   29.451326] pmem0: detected capacity change from 0 to 536870912
[   29.461476] scsi host0: Virtio SCSI HBA
[   29.532665] random: fast init done
[   29.546490] tun: Universal TUN/TAP device driver, 1.6
[   29.585697] ixgbe: Intel(R) 10 Gigabit PCI Express Network Driver - version 5.1.0-k
[   29.586723] ixgbe: Copyright (c) 1999-2016 Intel Corporation.
[   29.588834] ixgbevf: Intel(R) 10 Gigabit PCI Express Virtual Function Network Driver - version 4.1.0-k

I took a look to the kernel logs and I see the logs jump from [ 0.441705] console [hvc0] enabled
to [ 29.446221] brd: module loaded .

@liujing2 any hint why it is taking so long, the CI are VMs so this is nested virtualization.

@katacontainersbot
Copy link
Contributor

PSS Measurement:
Qemu: 167799 KB
Proxy: 4179 KB
Shim: 8751 KB

Memory inside container:
Total Memory: 2043460 KB
Free Memory: 2003688 KB

@opendev-zuul
Copy link

opendev-zuul bot commented Sep 6, 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

opendev-zuul bot commented Sep 6, 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
Contributor

PSS Measurement:
Qemu: 169661 KB
Proxy: 4139 KB
Shim: 8853 KB

Memory inside container:
Total Memory: 2043460 KB
Free Memory: 2003556 KB

@jcvenegas
Copy link
Member Author

@jodh-intel @bergwolf @sboeuf kata-containers/govmm#45 is ready to merge, needed to update vendor here.

@sboeuf
Copy link

sboeuf commented Sep 10, 2018

@jcvenegas kata-containers/govmm#45 has been merged, please revendor accordingly :)

Changes:

- qemu/qmp: support query-memory-devices qmp command.
- qemu: Add virtio RNG device.

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
@jcvenegas jcvenegas force-pushed the rng-device branch 2 times, most recently from 3f88c1c to ebe54a6 Compare September 10, 2018 19:10
@katacontainersbot
Copy link
Contributor

PSS Measurement:
Qemu: 156013 KB
Proxy: 4031 KB
Shim: 8793 KB

Memory inside container:
Total Memory: 2043460 KB
Free Memory: 2003628 KB

@jcvenegas
Copy link
Member Author

@sboeuf updated, note that I am not adding a way to restrict the entropy bandwidth of the host, I wonder if this is something is supported via cgroups (for example if runc support it ). In case this is not possible to do it with cgroups, a possible option is to add it as part of our configuration.toml, this could a nice feature to allow administrators restrict the amount of entropy at node level.

@opendev-zuul
Copy link

opendev-zuul bot commented Sep 10, 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

Copy link

@sboeuf sboeuf left a comment

Choose a reason for hiding this comment

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

@jcvenegas one comment, but looks fine otherwise!

@@ -488,6 +488,11 @@ func (q *qemu) createSandbox() error {
if ioThread != nil {
qemuConfig.IOThreads = []govmmQemu.IOThread{*ioThread}
}
// Add RNG device to hypervisor
rngDev := config.RNGDev{
ID: "rng0",
Copy link

Choose a reason for hiding this comment

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

Maybe you could define a const or var at the top of this file to avoid hardcoding this directly from the code. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

updated, lets wait for the CI again. After that should be ready do merge.

Kata Containers does not have provide a good entropy level,
make use of a paravirtual rng device to solve this problem.

Fixes: kata-containers#445

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
Update qemu-lite this disable static PRT on pc platform,
needed to use devices like virtio-rng.

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
@katacontainersbot
Copy link
Contributor

PSS Measurement:
Qemu: 172014 KB
Proxy: 4100 KB
Shim: 8832 KB

Memory inside container:
Total Memory: 2043460 KB
Free Memory: 2002448 KB

@opendev-zuul
Copy link

opendev-zuul bot commented Sep 10, 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

@jodh-intel
Copy link
Contributor

Restarted F27 CI (which fell over due to the problem fixed by #710)...

@sboeuf sboeuf merged commit e02695b into kata-containers:master Sep 11, 2018
@egernst egernst removed the review label Sep 11, 2018
@sboeuf sboeuf added the feature New functionality label Sep 12, 2018
chavafg pushed a commit to chavafg/tests-1 that referenced this pull request Nov 12, 2018
Add haveged will allow have entropy for PR.

kata-containers/runtime#676

Fixes: kata-containers#712

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
@jcvenegas jcvenegas deleted the rng-device branch January 29, 2019 18:33
@jcvenegas jcvenegas restored the rng-device branch September 30, 2019 18:48
@jcvenegas jcvenegas deleted the rng-device branch January 23, 2020 19:44
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.

None yet

9 participants