runtime-rs: support Memory hotplug #6876

Tim-0731-Hzt · 2023-05-18T06:55:12Z

support memory hotplug for runtime-rs

Tim-0731-Hzt · 2023-05-18T07:00:19Z

This PR will depend on the framework of #6289, and the memory hotplug functionality of dragonball.

fidencio · 2023-05-18T07:17:39Z

@Tim-0731-Hzt, please, when this PR becomes ready to be merged, re-enable this test here: https://github.com/kata-containers/kata-containers/blob/main/tests/integration/kubernetes/k8s-cpu-ns.bats

We'd like to have it tested as part of our CI.

Apokleos

Thx @Tim-0731-Hzt for your good job.
some comments for it.

src/runtime-rs/crates/hypervisor/src/dragonball/inner.rs

src/runtime-rs/crates/resource/src/cpu_mem/mem.rs

src/runtime-rs/crates/hypervisor/src/dragonball/vmm_instance.rs

src/runtime-rs/crates/resource/src/cpu_mem/mem.rs

YushuoEdge

Thanks @Tim-0731-Hzt ! Some initial comments:

src/libs/kata-types/src/capabilities.rs

src/runtime-rs/crates/hypervisor/src/dragonball/inner.rs

src/runtime-rs/crates/resource/src/cpu_mem/mem.rs

src/runtime-rs/crates/resource/src/manager_inner.rs

YushuoEdge

Thanks @Tim-0731-Hzt , i still have some questions:

src/libs/kata-types/src/capabilities.rs

src/runtime-rs/crates/hypervisor/src/dragonball/inner.rs

src/runtime-rs/crates/resource/src/cpu_mem/initial_size.rs

src/runtime-rs/crates/resource/src/cpu_mem/mem.rs

src/runtime-rs/crates/hypervisor/src/dragonball/inner.rs

src/runtime-rs/crates/resource/src/cpu_mem/initial_size.rs

src/runtime-rs/crates/hypervisor/src/dragonball/inner.rs

studychao · 2023-11-27T08:54:06Z

/test

HerlinCoder · 2023-12-05T07:16:38Z

src/runtime-rs/crates/resource/src/cpu_mem/mem.rs

+#[derive(Default, Debug, Clone)]
+pub struct MemResource {
+    /// Current memory
+    pub(crate) current_mem: Arc<RwLock<u32>>,


do we really need this Arc<RwLock> field which is only used to judge whether to clear balloon size or not?

As runtime is a multi-thread application, there might be a situation that we have one thread that needs to update the current_mem, while other threads are reading it to make decisions based on the current state

I'm not focus on the Arc Rwlock, I mean this field is never used again after you refactor balloon logic.

HerlinCoder · 2023-12-05T07:18:23Z

src/runtime-rs/crates/hypervisor/src/dragonball/inner.rs

@@ -338,13 +348,115 @@ impl DragonballInner {
        Ok((old_vcpus, new_vcpus))
    }

+    pub(crate) fn resize_memory(
+        &mut self,
+        old_mem_mb: u32,


we cloud simply save a balloon size in self, or just force to clear balloon size. It seems that resize_memory() only need a target memory size is enough

HerlinCoder · 2023-12-05T07:23:03Z

src/runtime-rs/crates/hypervisor/src/dragonball/inner.rs

 use std::{collections::HashSet, fs::create_dir_all};

 const DRAGONBALL_KERNEL: &str = "vmlinux";
 const DRAGONBALL_ROOT_FS: &str = "rootfs";
-
+const BALLOON0: &str = "balloon0";


what about BALLOON_DEVICE_ID? and why mem device don't use a const name?

HerlinCoder · 2023-12-05T07:31:34Z

src/libs/kata-types/src/capabilities.rs

@@ -17,6 +17,8 @@ pub enum CapabilityBits {
    MultiQueueSupport,
    /// hypervisor supports filesystem share
    FsSharingSupport,
+    /// hypervisor supports memory hotplug probe interface
+    GuestMemoryHotplugProbe,


GuestMemoryProbe may be better?

This is to know if the hypervisor supports Hotplug, maybe keep the keyword Hotplug is better here

The path you read in agent is /sys/devices/system/memory/probe, leave only probe here is more precise. And you can also choose leaving only hotplug here, which is also make sense.

HerlinCoder · 2023-12-05T07:36:13Z

src/runtime-rs/crates/runtimes/virt_container/src/sandbox.rs

+        // set memory hotplug probe
+        if guest_details.support_mem_hotplug_probe {
+            self.hypervisor
+                .set_capabilities(CapabilityBits::GuestMemoryHotplugProbe)


why do we need this checking? set cap and check it?

This is because we need to know if the hypervisor support memory hotplug. we get support_mem_hotplug_probe from agent response, and it is to determine the capability of memory hotplug, if it is, we set the capability to hypervisor.

HerlinCoder · 2023-12-05T07:37:40Z

src/runtime-rs/crates/runtimes/virt_container/src/sandbox.rs

+            .await;
+
+        // set memory hotplug probe
+        if guest_details.support_mem_hotplug_probe {


is the probe interface in guest kernel a necessary condition for hypervisor?

it might be unnecessary for Dragonball, but it is used for qemu in Kata 2.0

So when kernel does not support this feature, will dragonball still works? If so, you may need to set this capability as default, and consider if this need to be update to agent response.

fixed, set this capability when we initialize dragonball instance

HerlinCoder · 2023-12-05T07:40:26Z

src/runtime-rs/crates/hypervisor/src/dragonball/inner.rs

+    ) -> Result<(u32, MemoryConfig)> {
+        // check the invalid request memory
+        if new_mem_mb > self.hypervisor_config().memory_info.default_maxmemory
+            && self.hypervisor_config().memory_info.default_maxmemory > 0


what about set it to intmax or host total memory when it is 0?

this new_mem_mb variable cannot be 0, because it is calculated by adding it from the hypervisor's default memory.

Does default_maxmemory can be 0?

HerlinCoder · 2023-12-12T08:31:45Z

LGTM!

Tim-0731-Hzt · 2023-12-14T05:48:16Z

/test

Tim-0731-Hzt · 2023-12-14T08:32:39Z

/retest

get memory block size and guest mem hotplug probe Fixes:kata-containers#6356 Signed-off-by: Zhongtao Hu <zhongtaohu.tim@linux.alibaba.com>

Fixes: kata-containers#6875 Signed-off-by: Zhongtao Hu <zhongtaohu.tim@linux.alibaba.com>

Fixes:kata-containers#6875 Signed-off-by: Zhongtao Hu <zhongtaohu.tim@linux.alibaba.com>

add default_maxmemory in config file Fixes:kata-containers#6875 Signed-off-by: Zhongtao Hu <zhongtaohu.tim@linux.alibaba.com>

Tim-0731-Hzt · 2023-12-15T02:26:35Z

/test

check the update memory size greater than default max memory size Fixes:kata-containers#6875 Signed-off-by: Zhongtao Hu <zhongtaohu.tim@linux.alibaba.com>

Tim-0731-Hzt · 2023-12-15T03:32:48Z

/test

Tim-0731-Hzt added the runtime-rs label May 18, 2023

katacontainersbot added the size/tiny Smallest and simplest task label May 18, 2023

Tim-0731-Hzt marked this pull request as draft May 18, 2023 06:56

Tim-0731-Hzt changed the title ~~Memory hotlug~~ runtime-rs: support Memory hotlug May 18, 2023

Tim-0731-Hzt changed the title ~~runtime-rs: support Memory hotlug~~ WIP: support Memory hotlug May 18, 2023

Tim-0731-Hzt changed the title ~~WIP: support Memory hotlug~~ WIP: support Memory hotplug May 18, 2023

Tim-0731-Hzt force-pushed the memory_hotlug branch from 02c0558 to f436059 Compare June 13, 2023 08:12

katacontainersbot added size/huge Largest and most complex task (probably needs breaking into small pieces) and removed size/tiny Smallest and simplest task labels Jun 13, 2023

Tim-0731-Hzt marked this pull request as ready for review June 13, 2023 08:14

Tim-0731-Hzt changed the title ~~WIP: support Memory hotplug~~ runtime-rs: support Memory hotplug Jun 13, 2023

Tim-0731-Hzt force-pushed the memory_hotlug branch from f436059 to 0ef31dc Compare June 13, 2023 08:38

Apokleos reviewed Jun 13, 2023

View reviewed changes

YushuoEdge reviewed Jun 13, 2023

View reviewed changes

Tim-0731-Hzt force-pushed the memory_hotlug branch 2 times, most recently from 1c3c24b to ec053cb Compare June 15, 2023 06:58

katacontainersbot added size/tiny Smallest and simplest task and removed size/huge Largest and most complex task (probably needs breaking into small pieces) labels Jun 15, 2023

Tim-0731-Hzt force-pushed the memory_hotlug branch from ec053cb to 6320eb6 Compare June 15, 2023 07:04

YushuoEdge reviewed Jun 16, 2023

View reviewed changes

Tim-0731-Hzt force-pushed the memory_hotlug branch from 6320eb6 to 001e6a5 Compare June 20, 2023 08:53

katacontainersbot added size/huge Largest and most complex task (probably needs breaking into small pieces) and removed size/tiny Smallest and simplest task labels Jun 20, 2023

Tim-0731-Hzt added the no-backport-needed label Jun 20, 2023

YushuoEdge reviewed Jun 20, 2023

View reviewed changes

src/runtime-rs/crates/hypervisor/src/dragonball/inner.rs Outdated Show resolved Hide resolved

src/runtime-rs/crates/resource/src/cpu_mem/initial_size.rs Outdated Show resolved Hide resolved

Tim-0731-Hzt force-pushed the memory_hotlug branch 2 times, most recently from 53f825c to 4ddb4f7 Compare June 21, 2023 06:10

YushuoEdge reviewed Jun 21, 2023

View reviewed changes

src/runtime-rs/crates/hypervisor/src/dragonball/inner.rs Outdated Show resolved Hide resolved

Tim-0731-Hzt force-pushed the memory_hotlug branch 2 times, most recently from d10e8f0 to 275842a Compare November 20, 2023 09:24

Tim-0731-Hzt force-pushed the memory_hotlug branch 2 times, most recently from 2078804 to 2b63b58 Compare November 27, 2023 08:36

studychao added the ok-to-test label Nov 27, 2023

Tim-0731-Hzt force-pushed the memory_hotlug branch from 2b63b58 to 561e726 Compare November 28, 2023 06:44

HerlinCoder reviewed Dec 5, 2023

View reviewed changes

Tim-0731-Hzt force-pushed the memory_hotlug branch 5 times, most recently from 327ec15 to abbf70e Compare December 12, 2023 07:23

studychao approved these changes Dec 14, 2023

View reviewed changes

Tim-0731-Hzt added 4 commits December 15, 2023 10:22

runtim-rs: get guest memory details

d428a3f

get memory block size and guest mem hotplug probe Fixes:kata-containers#6356 Signed-off-by: Zhongtao Hu <zhongtaohu.tim@linux.alibaba.com>

runtime-rs: add resize_memory trait for hypervisor

81e55c4

Fixes: kata-containers#6875 Signed-off-by: Zhongtao Hu <zhongtaohu.tim@linux.alibaba.com>

runtime-rs: support memory resize

8d9fd9c

Fixes:kata-containers#6875 Signed-off-by: Zhongtao Hu <zhongtaohu.tim@linux.alibaba.com>

runtime-rs: add default_maxmemory in config file

6039417

add default_maxmemory in config file Fixes:kata-containers#6875 Signed-off-by: Zhongtao Hu <zhongtaohu.tim@linux.alibaba.com>

Tim-0731-Hzt force-pushed the memory_hotlug branch from abbf70e to 8ddec91 Compare December 15, 2023 02:26

runtime-rs: check the update memory size

9a37e77

check the update memory size greater than default max memory size Fixes:kata-containers#6875 Signed-off-by: Zhongtao Hu <zhongtaohu.tim@linux.alibaba.com>

Tim-0731-Hzt force-pushed the memory_hotlug branch from 8ddec91 to 9a37e77 Compare December 15, 2023 03:25

Tim-0731-Hzt merged commit 0f80dc6 into kata-containers:main Dec 15, 2023
167 of 176 checks passed

Tim-0731-Hzt deleted the memory_hotlug branch December 15, 2023 06:28

This was referenced Jan 8, 2024

runtime-rs: ch: Memory hotplug PR broke Cloud Hypervisor #8784

Closed

runtime-rs: ch: Unbreak CH driver #8803

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime-rs: support Memory hotplug #6876

runtime-rs: support Memory hotplug #6876

Tim-0731-Hzt commented May 18, 2023 •

edited

Tim-0731-Hzt commented May 18, 2023 •

edited

fidencio commented May 18, 2023

Apokleos left a comment

YushuoEdge left a comment

YushuoEdge left a comment

studychao commented Nov 27, 2023

HerlinCoder Dec 5, 2023

Tim-0731-Hzt Dec 6, 2023

HerlinCoder Dec 11, 2023

Tim-0731-Hzt Dec 12, 2023

HerlinCoder Dec 5, 2023

Tim-0731-Hzt Dec 11, 2023

HerlinCoder Dec 5, 2023

Tim-0731-Hzt Dec 11, 2023

HerlinCoder Dec 5, 2023

Tim-0731-Hzt Dec 11, 2023

HerlinCoder Dec 11, 2023

Tim-0731-Hzt Dec 12, 2023

HerlinCoder Dec 5, 2023

Tim-0731-Hzt Dec 11, 2023

HerlinCoder Dec 5, 2023

Tim-0731-Hzt Dec 11, 2023

HerlinCoder Dec 11, 2023

Tim-0731-Hzt Dec 12, 2023 •

edited

HerlinCoder Dec 5, 2023

Tim-0731-Hzt Dec 11, 2023 •

edited

HerlinCoder Dec 11, 2023

Tim-0731-Hzt Dec 12, 2023

HerlinCoder commented Dec 12, 2023

Tim-0731-Hzt commented Dec 14, 2023

Tim-0731-Hzt commented Dec 14, 2023

Tim-0731-Hzt commented Dec 15, 2023

Tim-0731-Hzt commented Dec 15, 2023

runtime-rs: support Memory hotplug #6876

runtime-rs: support Memory hotplug #6876

Conversation

Tim-0731-Hzt commented May 18, 2023 • edited

Tim-0731-Hzt commented May 18, 2023 • edited

fidencio commented May 18, 2023

Apokleos left a comment

Choose a reason for hiding this comment

YushuoEdge left a comment

Choose a reason for hiding this comment

YushuoEdge left a comment

Choose a reason for hiding this comment

studychao commented Nov 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tim-0731-Hzt Dec 12, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tim-0731-Hzt Dec 11, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HerlinCoder commented Dec 12, 2023

Tim-0731-Hzt commented Dec 14, 2023

Tim-0731-Hzt commented Dec 14, 2023

Tim-0731-Hzt commented Dec 15, 2023

Tim-0731-Hzt commented Dec 15, 2023

Tim-0731-Hzt commented May 18, 2023 •

edited

Tim-0731-Hzt commented May 18, 2023 •

edited

Tim-0731-Hzt Dec 12, 2023 •

edited

Tim-0731-Hzt Dec 11, 2023 •

edited