diff --git a/Sources/Containerization/CHHotplugProvider.swift b/Sources/Containerization/CHHotplugProvider.swift index 870fb1ffb..02155c5b2 100644 --- a/Sources/Containerization/CHHotplugProvider.swift +++ b/Sources/Containerization/CHHotplugProvider.swift @@ -98,15 +98,14 @@ final class CHHotplugProvider: HotplugProvider { case .virtioblk: let letter = try allocator.allocate() let chId = "blk-\(id)-\(letter)" - let disk = CloudHypervisor.DiskConfig( - path: rootfs.source, - readonly: rootfs.options.contains("ro"), - id: chId, - imageType: .raw - ) + // chDiskConfig applies the per-device runtime options of the mount, the + // same as for boot disks, and throws on an invalid value. let pci: CloudHypervisor.PciDeviceInfo do { + guard let disk = try rootfs.chDiskConfig(id: chId) else { + throw ContainerizationError(.invalidArgument, message: "mount \(rootfs.source) is not a virtio-blk device") + } pci = try await chCall { try await self.client.vmAddDisk(disk) } } catch { try? allocator.release(letter) diff --git a/Sources/Containerization/CHVirtualMachineInstance.swift b/Sources/Containerization/CHVirtualMachineInstance.swift index a20e296e2..d58ee99e2 100644 --- a/Sources/Containerization/CHVirtualMachineInstance.swift +++ b/Sources/Containerization/CHVirtualMachineInstance.swift @@ -511,10 +511,12 @@ extension CHVirtualMachineInstance { // Disks: rootfs forced read-only at the device level; container disks // honor their `ro` option through chDiskConfig. + // Every disk also gets the per-device runtime options of its mount, such + // as `chDiskDirect`, through chDiskConfig. var disks: [CloudHypervisor.DiskConfig] = [] for bd in bootDisks { let chId = bd.containerId.map { "blk-\($0)-\(bd.letter)" } ?? "rootfs" - if var disk = bd.mount.chDiskConfig(id: chId) { + if var disk = try bd.mount.chDiskConfig(id: chId) { if bd.containerId == nil { disk.readonly = true } diff --git a/Sources/Containerization/Mount+CH.swift b/Sources/Containerization/Mount+CH.swift index 2e25e2b90..a0d36e55c 100644 --- a/Sources/Containerization/Mount+CH.swift +++ b/Sources/Containerization/Mount+CH.swift @@ -15,6 +15,7 @@ //===----------------------------------------------------------------------===// import CloudHypervisor +import ContainerizationError import Foundation extension Mount { @@ -31,14 +32,59 @@ extension Mount { /// back to `Unknown` and silently rejects all writes — see CH's /// `virtio-devices/src/block.rs` "Attempting to write to sector 0 on a /// disk without specifying image_type" warning. - public func chDiskConfig(id: String) -> CloudHypervisor.DiskConfig? { - guard case .virtioblk = self.runtimeOptions else { + /// + /// The runtime options of the mount (`Mount.block(runtimeOptions:)`) can carry + /// `chDiskDirect=true` or `chDiskDirect=false`, which sets `direct` on this disk. + /// With `true`, cloud-hypervisor opens the image with `O_DIRECT`, so guest I/O + /// to the disk bypasses the host page cache. + /// Without the key, `direct` is not set and cloud-hypervisor keeps its default. + /// The option is per device because the right choice depends on the disk. + /// For example, many VMs can boot from the same read-only rootfs image without + /// the key, so the host page cache holds one copy of it for all of them. + /// Each VM can then attach its own read-write volume with `chDiskDirect=true`, + /// so the writes of the guest do not also fill the host page cache. + /// + /// Other runtime options, such as the `vz` keys of the Virtualization backend, + /// are ignored. + /// The Virtualization backend rejects keys that it does not know, so do not + /// set `chDiskDirect` on a mount that is also given to that backend. + /// + /// Throws if `chDiskDirect` has a value other than `true` or `false`. + public func chDiskConfig(id: String) throws -> CloudHypervisor.DiskConfig? { + guard case .virtioblk(let runtimeOptions) = self.runtimeOptions else { return nil } + var direct: Bool? + for option in runtimeOptions { + let split = option.split(separator: "=") + if split.count != 2 { + continue + } + + let key = String(split[0]) + let value = String(split[1]) + + switch key { + case "chDiskDirect": + switch value { + case "true": + direct = true + case "false": + direct = false + default: + throw ContainerizationError( + .invalidArgument, + message: "unknown chDiskDirect value for virtio block device: \(value)" + ) + } + default: + continue + } + } return CloudHypervisor.DiskConfig( path: self.source, readonly: self.options.contains("ro"), - direct: nil, + direct: direct, iommu: nil, id: id, pciSegment: nil, diff --git a/Tests/ContainerizationTests/CHDirectDiskIOTests.swift b/Tests/ContainerizationTests/CHDirectDiskIOTests.swift new file mode 100644 index 000000000..87d29912a --- /dev/null +++ b/Tests/ContainerizationTests/CHDirectDiskIOTests.swift @@ -0,0 +1,305 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the Containerization project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +#if os(Linux) +import CloudHypervisor +import ContainerizationError +import ContainerizationExtras +import Foundation +import NIO +import NIOHTTP1 +import Synchronization +import Testing + +@testable import Containerization + +/// A virtio-blk disk gets `direct` in the cloud-hypervisor config only when its own +/// mount carries the `chDiskDirect` runtime option, at boot and on hotplug. +/// The boot tests record the VM config in the `configureCH` of an extension and +/// then fail the start, so cloud-hypervisor never runs. +/// The hotplug tests send `vm.add-disk` to a stub API server on a Unix socket. +struct CHDirectDiskIOTests { + private struct Captured: Error {} + + private final class CaptureConfig: CHInstanceExtension { + let config = Mutex(nil) + + func configureCH(_ config: inout CloudHypervisor.VmConfig) throws { + let value = config + self.config.withLock { $0 = value } + throw Captured() + } + } + + /// Starts a VM with `mounts` as the mounts of container `c1` and returns the + /// disks of its VM config, the VM rootfs first. + private func bootDisks(_ mounts: [Mount]) async throws -> [CloudHypervisor.DiskConfig] { + let runtimeRoot = FileManager.default.temporaryDirectory.appendingPathComponent("ch-direct-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: runtimeRoot, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: runtimeRoot) } + let capture = CaptureConfig() + let instance = try CHVirtualMachineInstance( + runtimeRoot: runtimeRoot, + chBinary: URL(fileURLWithPath: "/bin/false"), + virtiofsdBinary: nil + ) { config in + config.kernel = Kernel(path: URL(fileURLWithPath: "/dev/null"), platform: .linuxArm) + config.initialFilesystem = .block(format: "ext4", source: "/tmp/rootfs.ext4", destination: "/") + config.mountsByID = ["c1": mounts] + config.extensions = [capture] + } + do { + try await instance.start() + } catch is Captured { + return try #require(capture.config.withLock { $0 }?.disks) + } + Issue.record("start did not reach configureCH") + return [] + } + + /// Returns the JSON object that is sent to cloud-hypervisor for `disk`. + private func encoded(_ disk: CloudHypervisor.DiskConfig) throws -> [String: Any] { + try jsonObject(JSONEncoder().encode(disk)) + } + + private func jsonObject(_ data: Data) throws -> [String: Any] { + try #require(try JSONSerialization.jsonObject(with: data) as? [String: Any]) + } + + @Test func rootfsWithoutKeyHasNoDirect() async throws { + let containerRootfs = Mount.block(format: "ext4", source: "/tmp/c1.ext4", destination: "/", options: ["ro"]) + let disks = try await bootDisks([containerRootfs]) + #expect(disks.count == 2) + for disk in disks { + #expect(disk.direct == nil) + #expect(try encoded(disk)["direct"] == nil) + } + } + + @Test func volumeWithKeyGetsDirect() async throws { + let volume = Mount.block( + format: "ext4", + source: "/tmp/vol.ext4", + destination: "/data", + runtimeOptions: ["chDiskDirect=true"] + ) + let disks = try await bootDisks([volume]) + #expect(disks.count == 2) + #expect(disks[1].path == "/tmp/vol.ext4") + #expect(disks[1].readonly == false) + #expect(disks[1].direct == true) + #expect(try encoded(disks[1])["direct"] as? Bool == true) + } + + /// The container rootfs is shared by many VMs and stays cached, and the + /// read-write volume of this VM uses O_DIRECT. + @Test func sharedRootfsAndDirectVolumeInOneVM() async throws { + let containerRootfs = Mount.block(format: "ext4", source: "/tmp/shared.ext4", destination: "/", options: ["ro"]) + let volume = Mount.block( + format: "ext4", + source: "/tmp/vol.ext4", + destination: "/data", + runtimeOptions: ["chDiskDirect=true"] + ) + let disks = try await bootDisks([containerRootfs, volume]) + #expect(disks.map(\.path) == ["/tmp/rootfs.ext4", "/tmp/shared.ext4", "/tmp/vol.ext4"]) + #expect(disks.map(\.direct) == [nil, nil, true]) + } + + @Test func explicitFalseSendsDirectFalse() async throws { + let volume = Mount.block( + format: "ext4", + source: "/tmp/vol.ext4", + destination: "/data", + runtimeOptions: ["chDiskDirect=false"] + ) + let disks = try await bootDisks([volume]) + #expect(disks[1].direct == false) + #expect(try encoded(disks[1])["direct"] as? Bool == false) + } + + @Test func invalidValueFailsTheStart() async throws { + let volume = Mount.block( + format: "ext4", + source: "/tmp/vol.ext4", + destination: "/data", + runtimeOptions: ["chDiskDirect=yes"] + ) + await #expect { + _ = try await bootDisks([volume]) + } throws: { error in + (error as? ContainerizationError)?.isCode(.invalidArgument) == true + } + } + + @Test func invalidValueThrowsFromChDiskConfig() { + let mount = Mount.block( + format: "ext4", + source: "/tmp/vol.ext4", + destination: "/data", + runtimeOptions: ["chDiskDirect=1"] + ) + #expect(throws: ContainerizationError.self) { + _ = try mount.chDiskConfig(id: "blk-0") + } + } + + @Test func virtualizationKeysAreIgnored() async throws { + let vzOnly = Mount.block( + format: "ext4", + source: "/tmp/vz.ext4", + destination: "/vz", + runtimeOptions: ["vzDiskImageCachingMode=uncached", "vzDiskImageSynchronizationMode=none"] + ) + let both = Mount.block( + format: "ext4", + source: "/tmp/both.ext4", + destination: "/both", + runtimeOptions: ["vzDiskImageCachingMode=cached", "chDiskDirect=true", "vzTimeout=5"] + ) + let disks = try await bootDisks([vzOnly, both]) + #expect(disks.map(\.direct) == [nil, nil, true]) + } + + // MARK: - Hotplug + + fileprivate final class Bodies: Sendable { + let value = Mutex<[Data]>([]) + } + + /// Hotplugs `mount` for container `c1` through a stub cloud-hypervisor API + /// server, and returns the result and the request bodies that the server got. + private func hotplug( + _ mount: Mount, + allocator: any AddressAllocator = Character.blockDeviceTagAllocator() + ) async throws -> (result: Result, bodies: [Data]) { + let group = MultiThreadedEventLoopGroup.singleton + let socketPath = FileManager.default.temporaryDirectory + .appendingPathComponent("ch-direct-\(UUID().uuidString.prefix(8)).sock").path + let bodies = Bodies() + let server = try await ServerBootstrap(group: group) + .childChannelInitializer { channel in + channel.eventLoop.makeCompletedFuture { + try channel.pipeline.syncOperations.configureHTTPServerPipeline(withPipeliningAssistance: false) + try channel.pipeline.syncOperations.addHandler(AddDiskHandler(bodies: bodies)) + } + } + .bind(unixDomainSocketPath: socketPath, cleanupExistingSocketFile: true) + .get() + + let client = try CloudHypervisor.Client(socketPath: URL(fileURLWithPath: socketPath), eventLoopGroup: group) + let provider = CHHotplugProvider( + client: client, + workDir: FileManager.default.temporaryDirectory, + virtiofsdBinary: nil, + allocator: allocator, + initialMounts: [:], + logger: nil + ) + let result: Result + do { + result = .success(try await provider.hotplug(mount, id: "c1")) + } catch { + result = .failure(error) + } + + try await client.shutdown() + try await server.close() + try? FileManager.default.removeItem(atPath: socketPath) + return (result, bodies.value.withLock { $0 }) + } + + @Test func hotplugHonorsTheKey() async throws { + let volume = Mount.block( + format: "ext4", + source: "/tmp/vol.ext4", + destination: "/data", + runtimeOptions: ["chDiskDirect=true"] + ) + let (result, bodies) = try await hotplug(volume) + #expect(try result.get().source == "/dev/vda") + #expect(bodies.count == 1) + let disk = try JSONDecoder().decode(CloudHypervisor.DiskConfig.self, from: try #require(bodies.first)) + #expect(disk.path == "/tmp/vol.ext4") + #expect(disk.id == "blk-c1-a") + #expect(disk.imageType == .raw) + #expect(disk.direct == true) + } + + @Test func hotplugWithoutKeySendsNoDirect() async throws { + let rootfs = Mount.block(format: "ext4", source: "/tmp/c1.ext4", destination: "/", options: ["ro"]) + let (result, bodies) = try await hotplug(rootfs) + _ = try result.get() + #expect(bodies.count == 1) + let object = try jsonObject(try #require(bodies.first)) + #expect(object["direct"] == nil) + #expect(object["readonly"] as? Bool == true) + } + + @Test func hotplugInvalidValueSendsNothingAndReleasesTheLetter() async throws { + let allocator = Character.blockDeviceTagAllocator() + let volume = Mount.block( + format: "ext4", + source: "/tmp/vol.ext4", + destination: "/data", + runtimeOptions: ["chDiskDirect=on"] + ) + let (result, bodies) = try await hotplug(volume, allocator: allocator) + #expect(throws: ContainerizationError.self) { + _ = try result.get() + } + #expect(bodies.isEmpty) + #expect(try allocator.allocate() == "a") + } +} + +/// Records each request body and answers with the `PciDeviceInfo` of an added disk. +private final class AddDiskHandler: ChannelInboundHandler, @unchecked Sendable { + typealias InboundIn = HTTPServerRequestPart + typealias OutboundOut = HTTPServerResponsePart + + private let bodies: CHDirectDiskIOTests.Bodies + private var body: [UInt8] = [] + + init(bodies: CHDirectDiskIOTests.Bodies) { + self.bodies = bodies + } + + func channelRead(context: ChannelHandlerContext, data: NIOAny) { + switch unwrapInboundIn(data) { + case .head: + body = [] + case .body(var buffer): + body.append(contentsOf: buffer.readBytes(length: buffer.readableBytes) ?? []) + case .end: + let request = Data(body) + bodies.value.withLock { $0.append(request) } + let response = Array(#"{"id":"_disk0","bdf":"0000:00:05.0"}"#.utf8) + var headers = HTTPHeaders() + headers.add(name: "Content-Type", value: "application/json") + headers.add(name: "Content-Length", value: "\(response.count)") + headers.add(name: "Connection", value: "close") + let head = HTTPResponseHead(version: .http1_1, status: .ok, headers: headers) + context.write(wrapOutboundOut(.head(head)), promise: nil) + context.write(wrapOutboundOut(.body(.byteBuffer(context.channel.allocator.buffer(bytes: response)))), promise: nil) + let bound = NIOLoopBound(context, eventLoop: context.eventLoop) + context.writeAndFlush(wrapOutboundOut(.end(nil))).whenComplete { _ in + bound.value.close(promise: nil) + } + } + } +} +#endif diff --git a/Tests/ContainerizationTests/MountCHTests.swift b/Tests/ContainerizationTests/MountCHTests.swift index 4b809ea29..4cbc3c16d 100644 --- a/Tests/ContainerizationTests/MountCHTests.swift +++ b/Tests/ContainerizationTests/MountCHTests.swift @@ -22,9 +22,9 @@ import Testing @Suite("Mount+CH") struct MountCHTests { @Test("block mount without options produces DiskConfig with readonly=false") - func blockNoOptions() { + func blockNoOptions() throws { let mount = Mount.block(format: "ext4", source: "/foo.img", destination: "/data") - let cfg = mount.chDiskConfig(id: "blk-0") + let cfg = try mount.chDiskConfig(id: "blk-0") #expect(cfg?.path == "/foo.img") #expect(cfg?.readonly == false) #expect(cfg?.id == "blk-0") @@ -34,19 +34,19 @@ struct MountCHTests { } @Test("block mount with 'ro' option produces DiskConfig with readonly=true") - func blockReadOnly() { + func blockReadOnly() throws { let mount = Mount.block(format: "ext4", source: "/foo.img", destination: "/data", options: ["ro"]) - let cfg = mount.chDiskConfig(id: "blk-1") + let cfg = try mount.chDiskConfig(id: "blk-1") #expect(cfg?.readonly == true) } @Test("non-block mount returns nil from chDiskConfig") - func chDiskConfigNilForNonBlock() { + func chDiskConfigNilForNonBlock() throws { let share = Mount.share(source: "/host", destination: "/guest") - #expect(share.chDiskConfig(id: "x") == nil) + #expect(try share.chDiskConfig(id: "x") == nil) let any = Mount.any(type: "tmpfs", source: "tmpfs", destination: "/tmp") - #expect(any.chDiskConfig(id: "x") == nil) + #expect(try any.chDiskConfig(id: "x") == nil) } @Test("share mount produces FsConfig with tag and socket")