-
Notifications
You must be signed in to change notification settings - Fork 375
kata-check: require kvm modules for amd64 #1986
Conversation
cli/kata-check_amd64.go
Outdated
}, | ||
kernelModvhostvsock: { | ||
desc: msgKernelVirtioVhostVsock, | ||
required: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing required: true,
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vosck is not required, and by default required is false, so I just removed the line as it's redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know that by default it's false
, can we keep it explicit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's how Go works, but I am happy to set it explicitly to false
if this gives more clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @marcov
KVM/vhost modules are required when using QEMU or firecracker. Fixes: kata-containers#1985 Signed-off-by: Marco Vedovati <mvedovati@suse.com>
/test |
1 similar comment
/test |
Codecov Report
@@ Coverage Diff @@
## master #1986 +/- ##
==========================================
+ Coverage 52.01% 52.06% +0.05%
==========================================
Files 107 107
Lines 14352 14348 -4
==========================================
+ Hits 7465 7471 +6
+ Misses 5999 5994 -5
+ Partials 888 883 -5 |
Add a test for the checkKernelModules returned error count value. Signed-off-by: Marco Vedovati <mvedovati@suse.com>
/test |
All tests are good, but we need a pair of reviews :-) Calling in the usual folks 😁 @jodh-intel @grahamwhaley @devimc @jcvenegas |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @marcov
KVM modules are required when using QEMU or firecracker.
Fixes: #1985
Signed-off-by: Marco Vedovati mvedovati@suse.com