Skip to content

Conversation

pmores
Copy link
Contributor

@pmores pmores commented Nov 25, 2024

This PR aims to bring runtime-rs's behaviour with respect to vcpu allocation in line with runtime-go's behaviour implemented by PR #7623.

In short, the two main goals are

  • move from rounding individual container vcpu requests up and summing up the results to summing up container requests first and only rounding up the sum as the former approach risks allocating vcpu capacity in excess of what's actually requested or needed
  • make configuration.toml's default_vcpus value a float so that fractional vcpus can be expressed.

@pmores
Copy link
Contributor Author

pmores commented Nov 25, 2024

@fidencio can you please take a look?

@katacontainersbot katacontainersbot added the size/large Task of significant size label Nov 25, 2024
@fidencio
Copy link
Member

@fidencio can you please take a look?

Sure thing, added to my TODO list.

@fidencio
Copy link
Member

One thing that I'd love to see, is

[ "${KATA_HYPERVISOR}" = "qemu-runtime-rs" ] && skip "Requires CPU hotplug which isn't supported on ${KATA_HYPERVISOR} yet"
unskipped for runtime-rs.

@pmores
Copy link
Contributor Author

pmores commented Nov 26, 2024

Done!

@pmores pmores force-pushed the make-vcpu-allocation-more-accurate branch 3 times, most recently from 0c4bdb6 to f004fa0 Compare December 2, 2024 12:20
@pmores pmores force-pushed the make-vcpu-allocation-more-accurate branch from f004fa0 to 49b5e0c Compare January 8, 2025 13:54
@pmores
Copy link
Contributor Author

pmores commented Jan 8, 2025

Rebased onto latest main.

@pmores
Copy link
Contributor Author

pmores commented Jun 20, 2025

Rebased onto main again.

pmores added 5 commits August 7, 2025 10:32
This commit addresses a part of the same problem as PR kata-containers#7623 did for the
golang runtime.  So far we've been rounding up individual containers'
vCPU requests and then summing them up which can lead to allocation of
excess vCPUs as described in the mentioned PR's cover letter.  We address
this by reversing the order of operations, we sum the (possibly fractional)
container requests and only then round up the total.

We also align runtime-rs's behaviour with runtime-go in that we now
include the default vcpu request from the config file ('default_vcpu')
in the total.

We diverge from PR kata-containers#7623 in that `default_vcpu` is still treated as an
integer (this will be a topic of a separate commit), and that this
implementation avoids relying on 32-bit floating point arithmetic as there
are some potential problems with using f32.  For instance, some numbers
commonly used in decimal, notably all of single-decimal-digit numbers
0.1, 0.2 .. 0.9 except 0.5, are periodic in binary and thus fundamentally
not representable exactly.  Arithmetics performed on such numbers can lead
to surprising results, e.g. adding 0.1 ten times gives 1.0000001, not 1,
and taking a ceil() results in 2, clearly a wrong answer in vcpu
allocation.

So instead, we take advantage of the fact that container requests happen
to be expressed as a quota/period fraction so we can sum up quotas,
fundamentally integral numbers (possibly fractional only due to the need
to rewrite them with a common denominator) with much less danger of
precision loss.

Signed-off-by: Pavel Mores <pmores@redhat.com>
This commit focuses purely on the formal change of type.  If any subsequent
changes in semantics are needed they are purposely avoided here so that the
commit can be reviewed as a 100% formal and 0% semantic change.

Signed-off-by: Pavel Mores <pmores@redhat.com>
Also included (as commented out) is a test that does not pass although
it should.  See source code comment for explanation why fixing this seems
beyond the scope of this PR.

Signed-off-by: Pavel Mores <pmores@redhat.com>
Configuration information is adjusted after loading from file but so
far, there has been no similar check for configuration coming from
annotations.  This commit introduces re-adjusting config after
annotations have been processed.

A small refactor was necessary as a prerequisite which introduces
function TomlConfig::adjust_config() to make it easier to invoke
the adjustment for a whole TomlConfig instance.  This function is
analogous to the existing validate() function.

The immediate motivation for this change is to make sure that 0
in "default_vcpus" annotation will be properly adjusted to 1 as
is the case if 0 is loaded from a config file.  This is required
to match the golang runtime behaviour.

Signed-off-by: Pavel Mores <pmores@redhat.com>
This series should make runtime-rs's vcpu allocation behaviour match the
behaviour of runtime-go so we can now enable pertinent tests which were
skipped so far due the difference between both shims.

Signed-off-by: Pavel Mores <pmores@redhat.com>
@pmores pmores force-pushed the make-vcpu-allocation-more-accurate branch from 0cf8108 to 69f2169 Compare August 7, 2025 08:33
@lifupan lifupan merged commit b50777a into kata-containers:main Aug 8, 2025
334 of 358 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test size/large Task of significant size
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants