Skip to content

Commit ed072a4

Browse files
authored
Merge branch 'main' into fix_patch_async_drives
2 parents 33ecc99 + 5cbd55a commit ed072a4

File tree

6 files changed

+191
-141
lines changed

6 files changed

+191
-141
lines changed

tests/framework/microvm.py

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
from framework.jailer import JailerContext
3939
from framework.microvm_helpers import MicrovmHelpers
4040
from framework.properties import global_props
41+
from framework.utils_drive import VhostUserBlkBackend, VhostUserBlkBackendType
4142
from host_tools.memory import MemoryMonitor
4243

4344
LOG = logging.getLogger("microvm")
@@ -209,6 +210,7 @@ def __init__(
209210
# device dictionaries
210211
self.iface = {}
211212
self.disks = {}
213+
self.disks_vhost_user = {}
212214
self.vcpus_count = None
213215
self.mem_size_bytes = None
214216

@@ -229,6 +231,13 @@ def kill(self):
229231
"""All clean up associated with this microVM should go here."""
230232
# pylint: disable=subprocess-run-check
231233

234+
# We start with vhost-user backends,
235+
# because if we stop Firecracker first, the backend will want
236+
# to exit as well and this will cause a race condition.
237+
for backend in self.disks_vhost_user.values():
238+
backend.kill()
239+
self.disks_vhost_user.clear()
240+
232241
if (
233242
self.expect_kill_by_signal is False
234243
and "Shutting down VM after intercepting signal" in self.log_data
@@ -466,6 +475,8 @@ def pin_api(self, cpu_id: int):
466475
def pin_threads(self, first_cpu):
467476
"""
468477
Pins all microvm threads (VMM, API and vCPUs) to consecutive physical cpu core, starting with "first_cpu"
478+
479+
Return next "free" cpu core.
469480
"""
470481
for vcpu, pcpu in enumerate(range(first_cpu, first_cpu + self.vcpus_count)):
471482
assert self.pin_vcpu(
@@ -481,6 +492,8 @@ def pin_threads(self, first_cpu):
481492
first_cpu + self.vcpus_count + 1
482493
), "Failed to pin fc_api thread."
483494

495+
return first_cpu + self.vcpus_count + 2
496+
484497
def spawn(
485498
self,
486499
log_file="fc.log",
@@ -704,13 +717,28 @@ def add_drive(
704717
def add_vhost_user_drive(
705718
self,
706719
drive_id,
707-
socket,
720+
path_on_host,
708721
partuuid=None,
709722
is_root_device=False,
723+
is_read_only=False,
710724
cache_type=None,
725+
backend_type=VhostUserBlkBackendType.CROSVM,
711726
):
712727
"""Add a vhost-user block device."""
713728

729+
# It is possible that the user adds another drive
730+
# with the same ID. In that case, we should clean
731+
# the previous backend up first.
732+
prev = self.disks_vhost_user.pop(drive_id, None)
733+
if prev:
734+
prev.kill()
735+
736+
backend = VhostUserBlkBackend.with_backend(
737+
backend_type, path_on_host, self.chroot(), drive_id, is_read_only
738+
)
739+
740+
socket = backend.spawn(self.jailer.uid, self.jailer.gid)
741+
714742
self.api.drive.put(
715743
drive_id=drive_id,
716744
socket=socket,
@@ -719,6 +747,8 @@ def add_vhost_user_drive(
719747
cache_type=cache_type,
720748
)
721749

750+
self.disks_vhost_user[drive_id] = backend
751+
722752
def patch_drive(self, drive_id, file=None):
723753
"""Modify/patch an existing block device."""
724754
if file:

tests/framework/utils_drive.py

Lines changed: 122 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import os
77
import subprocess
88
import time
9+
from abc import ABC, abstractmethod
910
from enum import Enum
1011
from pathlib import Path
1112
from subprocess import check_output
@@ -46,78 +47,142 @@ def partuuid_and_disk_path(rootfs_ubuntu_22, disk_path):
4647
return (partuuid, disk_path)
4748

4849

49-
CROSVM_CTR_SOCKET_NAME = "crosvm_ctr.socket"
50-
51-
52-
def spawn_vhost_user_backend(
53-
vm,
54-
host_mem_path,
55-
socket_path,
56-
readonly=False,
57-
backend=VhostUserBlkBackendType.CROSVM,
58-
):
59-
"""Spawn vhost-user-blk backend."""
60-
61-
uid = vm.jailer.uid
62-
gid = vm.jailer.gid
63-
host_vhost_user_socket_path = Path(vm.chroot()) / socket_path.strip("/")
64-
65-
if backend == VhostUserBlkBackendType.QEMU:
50+
class VhostUserBlkBackend(ABC):
51+
"""vhost-user-blk backend base class"""
52+
53+
@classmethod
54+
def get_all_subclasses(cls):
55+
"""Get all subclasses of the class."""
56+
subclasses = {}
57+
for subclass in cls.__subclasses__():
58+
subclasses[subclass.__name__] = subclass
59+
subclasses.update(subclass.get_all_subclasses())
60+
return subclasses
61+
62+
@classmethod
63+
def with_backend(cls, backend: VhostUserBlkBackendType, *args, **kwargs):
64+
"""Get a backend of a specific type."""
65+
subclasses = cls.get_all_subclasses()
66+
return subclasses[backend.value + cls.__name__](*args, **kwargs)
67+
68+
def __init__(
69+
self,
70+
host_mem_path,
71+
chroot,
72+
backend_id,
73+
readonly,
74+
):
75+
self.host_mem_path = host_mem_path
76+
self.socket_path = Path(chroot) / f"{backend_id}_vhost_user.sock"
77+
self.readonly = readonly
78+
self.proc = None
79+
80+
def spawn(self, uid, gid):
81+
"""
82+
Spawn a backend.
83+
84+
Return socket path in the jail that can be used with FC API.
85+
"""
86+
assert not self.proc, "backend already spawned"
87+
args = self._spawn_cmd()
88+
proc = subprocess.Popen(args)
89+
90+
# Give the backend time to initialise.
91+
time.sleep(1)
92+
93+
assert proc is not None and proc.poll() is None, "backend is not up"
94+
assert self.socket_path.exists()
95+
96+
os.chown(self.socket_path, uid, gid)
97+
98+
self.proc = proc
99+
100+
return str(Path("/") / os.path.basename(self.socket_path))
101+
102+
@abstractmethod
103+
def _spawn_cmd(self):
104+
"""Return a spawn command for the backend"""
105+
return ""
106+
107+
@abstractmethod
108+
def resize(self, new_size):
109+
"""Resize the vhost-user-backed drive"""
110+
111+
def pin(self, cpu_id: int):
112+
"""Pin the vhost-user backend to a CPU list."""
113+
return utils.ProcessManager.set_cpu_affinity(self.proc.pid, [cpu_id])
114+
115+
def kill(self):
116+
"""Kill the backend"""
117+
if self.proc.poll() is None:
118+
self.proc.terminate()
119+
self.proc.wait()
120+
os.remove(self.socket_path)
121+
assert not os.path.exists(self.socket_path)
122+
123+
124+
class QemuVhostUserBlkBackend(VhostUserBlkBackend):
125+
"""vhost-user-blk backend implementaiton for Qemu backend"""
126+
127+
def _spawn_cmd(self):
66128
args = [
67129
"vhost-user-blk",
68130
"--socket-path",
69-
host_vhost_user_socket_path,
131+
self.socket_path,
70132
"--blk-file",
71-
host_mem_path,
133+
self.host_mem_path,
72134
]
73-
if readonly:
135+
if self.readonly:
74136
args.append("--read-only")
75-
elif backend == VhostUserBlkBackendType.CROSVM:
76-
crosvm_ctr_socket_path = Path(vm.chroot()) / CROSVM_CTR_SOCKET_NAME.strip("/")
77-
ro = ",ro" if readonly else ""
137+
return args
138+
139+
def resize(self, new_size):
140+
raise NotImplementedError("not supported for Qemu backend")
141+
142+
143+
class CrosvmVhostUserBlkBackend(VhostUserBlkBackend):
144+
"""vhost-user-blk backend implementaiton for crosvm backend"""
145+
146+
def __init__(
147+
self,
148+
host_mem_path,
149+
chroot,
150+
backend_id,
151+
readonly=False,
152+
):
153+
super().__init__(
154+
host_mem_path,
155+
chroot,
156+
backend_id,
157+
readonly,
158+
)
159+
self.ctr_socket_path = Path(chroot) / f"{backend_id}_ctr.sock"
160+
161+
def _spawn_cmd(self):
162+
ro = ",ro" if self.readonly else ""
78163
args = [
79164
"crosvm",
80165
"--log-level",
81166
"off",
82167
"devices",
83168
"--disable-sandbox",
84169
"--control-socket",
85-
crosvm_ctr_socket_path,
170+
self.ctr_socket_path,
86171
"--block",
87-
f"vhost={host_vhost_user_socket_path},path={host_mem_path}{ro}",
172+
f"vhost={self.socket_path},path={self.host_mem_path}{ro}",
88173
]
89-
if os.path.exists(crosvm_ctr_socket_path):
90-
os.remove(crosvm_ctr_socket_path)
91-
else:
92-
assert False, f"unknown vhost-user-blk backend `{backend}`"
93-
proc = subprocess.Popen(args)
174+
return args
94175

95-
# Give the backend time to initialise.
96-
time.sleep(1)
176+
def resize(self, new_size):
177+
assert self.proc, "backend is not spawned"
178+
assert self.ctr_socket_path.exists()
97179

98-
assert proc is not None and proc.poll() is None, "backend is not up"
180+
utils.run_cmd(
181+
f"crosvm disk resize 0 {new_size * 1024 * 1024} {self.ctr_socket_path}"
182+
)
99183

100-
with utils.chroot(vm.chroot()):
101-
# The backend will create the socket path with root rights.
102-
# Change rights to the jailer's.
103-
os.chown(socket_path, uid, gid)
104-
105-
return proc
106-
107-
108-
def resize_vhost_user_drive(vm, new_size):
109-
"""
110-
Resize vhost-user-blk drive and send config change notification.
111-
112-
This only works with the crosvm vhost-user-blk backend.
113-
New size is in MB.
114-
"""
115-
116-
crosvm_ctr_socket_path = Path(vm.chroot()) / CROSVM_CTR_SOCKET_NAME.strip("/")
117-
assert os.path.exists(
118-
crosvm_ctr_socket_path
119-
), "crosvm backend must be spawned first"
120-
121-
utils.run_cmd(
122-
f"crosvm disk resize 0 {new_size * 1024 * 1024} {crosvm_ctr_socket_path}"
123-
)
184+
def kill(self):
185+
super().kill()
186+
assert self.proc.poll() is not None
187+
os.remove(self.ctr_socket_path)
188+
assert not os.path.exists(self.ctr_socket_path)

tests/integration_tests/functional/test_api.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import re
1010
import resource
1111
import time
12+
from pathlib import Path
1213

1314
import packaging.version
1415
import pytest
@@ -18,7 +19,6 @@
1819
from framework import utils_cpuid
1920
from framework.utils import get_firecracker_version_from_toml, is_io_uring_supported
2021
from framework.utils_cpu_templates import nonci_on_arm
21-
from framework.utils_drive import spawn_vhost_user_backend
2222

2323
MEM_LIMIT = 1000000000
2424

@@ -749,12 +749,7 @@ def test_drive_patch(test_microvm_with_api):
749749
fs_vub = drive_tools.FilesystemFile(
750750
os.path.join(test_microvm.fsfiles, "scratch_vub")
751751
)
752-
vhost_user_socket = "/vub.socket"
753-
# Launching vhost-user-block backend
754-
_backend = spawn_vhost_user_backend(
755-
test_microvm, fs_vub.path, vhost_user_socket, False
756-
)
757-
test_microvm.add_vhost_user_drive("scratch_vub", vhost_user_socket)
752+
test_microvm.add_vhost_user_drive("scratch_vub", fs_vub.path)
758753

759754
# Patching drive before boot is not allowed.
760755
with pytest.raises(RuntimeError, match=NOT_SUPPORTED_BEFORE_START):
@@ -931,7 +926,10 @@ def _drive_patch(test_microvm):
931926
"path_on_host": None,
932927
"rate_limiter": None,
933928
"io_engine": None,
934-
"socket": "/vub.socket",
929+
"socket": str(
930+
Path("/")
931+
/ test_microvm.disks_vhost_user["scratch_vub"].socket_path.name
932+
),
935933
},
936934
]
937935

0 commit comments

Comments
 (0)