-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
libvirt: Add podvm instance cpu and mem size support for libvirt #2116
base: main
Are you sure you want to change the base?
libvirt: Add podvm instance cpu and mem size support for libvirt #2116
Conversation
628f293
to
5c98498
Compare
Add podvm instance cpu and mem size support for libvirt Fixes: confidential-containers#1650 Signed-off-by : SAVITRI HUNASHEEKATTI <savitrilh@in.ibm.com>
5c98498
to
951f560
Compare
Add podvm instance cpu and mem size support for libvirt Fixes: confidential-containers#1650 Signed-off-by : SAVITRI HUNASHEEKATTI <savitrilh@in.ibm.com>
Add podvm instance cpu and mem size support for libvirt Fixes: confidential-containers#1650 Signed-off-by : SAVITRI HUNASHEEKATTI <savitrilh@in.ibm.com>
ca6e52a
to
0bf4a2d
Compare
Removed extra closing bracket Signed-off-by : SAVITRI HUNASHEEKATTI <savitrilh@in.ibm.com>
0bf4a2d
to
36c06e3
Compare
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've left a couple of comments, but we also need the commits squashed together so it is more reviewable. Thanks
src/cloud-api-adaptor/install/overlays/libvirt/kustomization.yaml
Outdated
Show resolved
Hide resolved
bb190bd
to
f17c97b
Compare
Use io.katacontainers.config.hypervisor.default_vcpus and io.katacontainers.config.hypervisor.default_memory annotations to set the libvirt podvm instance. Signed-off-by: savitrilh <138098272+savitrilh@users.noreply.github.com>
f17c97b
to
e9edb50
Compare
provider.DefaultToEnv(&memoryStr, "LIBVIRT_MEMORY", defaultMemory) | ||
if memoryStr != "" { | ||
memory, _ := strconv.ParseUint(memoryStr, 10, 64) | ||
libvirtcfg.Memory = uint(memory) |
Check failure
Code scanning / CodeQL
Incorrect conversion between integer types High
strconv.ParseUint
provider.DefaultToEnv(&processorsStr, "LIBVIRT_CPU", defaultCPU) | ||
if processorsStr != "" { | ||
cpu, _ := strconv.ParseUint(processorsStr, 10, 64) | ||
libvirtcfg.CPU = uint(cpu) |
Check failure
Code scanning / CodeQL
Incorrect conversion between integer types High
strconv.ParseUint
set -x | ||
exec cloud-api-adaptor libvirt \ | ||
-pods-dir "${PEER_PODS_DIR}" \ | ||
-socket "${REMOTE_HYPERVISOR_ENDPOINT}" \ | ||
-uri "${LIBVIRT_URI}" \ | ||
-data-dir /opt/data-dir \ | ||
-network-name "${LIBVIRT_NET:-default}" \ | ||
-pool-name "${LIBVIRT_POOL:-default}" \ | ||
-pool-name "${LIBVIRT_POOL:-default}" |
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 think this needs the \
, otherwise all the optionals are treated like a new line?
memory, _ := strconv.ParseUint(memoryStr, 10, 64) | ||
libvirtcfg.Memory = uint(memory) |
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.
This is flagged up as a potentially unsafe conversion, maybe we should explicitly make libvirtcfg.Memory
a uint64
type?
Use io.katacontainers.config.hypervisor.default_vcpus and io.katacontainers.config.hypervisor.default_memory annotations to set the libvirt podvm instance. Use the default values if no annotations are provided.
Fixes: #1650
Signed-off-by : savitrilh savitrilh@in.ibm.com