From 31a18e332b7620f753f18fa7e0337422c71fa287 Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 12 Aug 2026 16:24:21 +0800 Subject: [PATCH 1/2] vm: keep qcow2 overlays buffered so a shared base stays in the page cache Cloud Hypervisor used to open a qcow2 backing file with buffered I/O no matter what the disk asked for, so the shared base image behind every cloudimg and Windows VM was served from one host page-cache copy. It now opens the backing file with the disk's own direct flag, and cocoon sets direct=on for every writable disk, so each VM started reading the base image straight from storage instead. Default direct=off for a qcow2 that layers over a base image. The overlay itself loses O_DIRECT, so its writes land in the host page cache as well as the guest one. That memory is reclaimable, and the residual cost is one extra copy per written byte, which is the cheaper half of this trade: without it every VM refetches the shared base from storage. Raw COW disks, read-only layers and data disks are unchanged, and an explicit direct_io on a data disk still wins. Expressing the split the old Cloud Hypervisor gave us by accident, a direct overlay above a buffered base, needs a per-backing-file cache policy that no released version has. That is requested upstream in cloud-hypervisor/cloud-hypervisor#8718. Measured on a 16-core host with a 700 MiB base image and 8 VMs reading 1 GiB each: base image resident in the page cache 310 MiB with the fix against 0 MiB without it, aggregate read 1.86s against 2.47-2.67s. --- hypervisor/cloudhypervisor/args.go | 10 ++++++- hypervisor/cloudhypervisor/args_test.go | 36 +++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/hypervisor/cloudhypervisor/args.go b/hypervisor/cloudhypervisor/args.go index d5152d4e..46cd4902 100644 --- a/hypervisor/cloudhypervisor/args.go +++ b/hypervisor/cloudhypervisor/args.go @@ -178,10 +178,18 @@ func serialConsoleFor(directBoot bool, consoleSock string) (serial, console *chR return &chRuntimeFile{Mode: "Socket", Socket: consoleSock}, &chRuntimeFile{Mode: "Off"} } +func qcow2Overlay(sc *types.StorageConfig) bool { + return !sc.RO && filepath.Ext(sc.Path) == ".qcow2" +} + func effectiveDirectIO(sc *types.StorageConfig, noDirectIO bool) bool { if sc.DirectIO != nil { return *sc.DirectIO } + // CH applies this flag to the backing file too, so O_DIRECT would stop one page-cache copy of the shared base serving every VM. + if qcow2Overlay(sc) { + return false + } return !sc.RO && !noDirectIO } @@ -201,7 +209,7 @@ func storageConfigToDisk(storageConfig *types.StorageConfig, cpuCount, diskQueue switch { case filepath.Ext(storageConfig.Path) == ".qcow2": d.ImageType = "Qcow2" - d.BackingFiles = !storageConfig.RO + d.BackingFiles = qcow2Overlay(storageConfig) case storageConfig.RO: d.ImageType = "Raw" default: diff --git a/hypervisor/cloudhypervisor/args_test.go b/hypervisor/cloudhypervisor/args_test.go index 8283adbd..5b1d6ed1 100644 --- a/hypervisor/cloudhypervisor/args_test.go +++ b/hypervisor/cloudhypervisor/args_test.go @@ -30,3 +30,39 @@ func TestMemoryCLIArg(t *testing.T) { }) } } + +func TestEffectiveDirectIO(t *testing.T) { + tests := []struct { + name string + sc types.StorageConfig + noDirectIO bool + want bool + }{ + {name: "raw cow", sc: types.StorageConfig{Path: "/v/cow.raw", Role: types.StorageRoleCOW}, want: true}, + {name: "raw cow with no-direct-io", sc: types.StorageConfig{Path: "/v/cow.raw", Role: types.StorageRoleCOW}, noDirectIO: true}, + {name: "readonly layer", sc: types.StorageConfig{Path: "/v/base.raw", RO: true, Role: types.StorageRoleLayer}}, + {name: "qcow2 overlay stays buffered", sc: types.StorageConfig{Path: "/v/overlay.qcow2", Role: types.StorageRoleCOW}}, + {name: "readonly qcow2 has no backing chain", sc: types.StorageConfig{Path: "/v/base.qcow2", RO: true, Role: types.StorageRoleLayer}}, + {name: "explicit override wins", sc: types.StorageConfig{Path: "/v/data.raw", Role: types.StorageRoleData, DirectIO: ptr(false)}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := effectiveDirectIO(&tt.sc, tt.noDirectIO); got != tt.want { + t.Errorf("got %v, want %v", got, tt.want) + } + }) + } +} + +func TestQcow2OverlayDiskArgs(t *testing.T) { + sc := &types.StorageConfig{Path: "/v/overlay.qcow2", Role: types.StorageRoleCOW} + got := diskToCLIArg(storageConfigToDisk(sc, 1, 0, false, nil)) + if strings.Contains(got, "direct=on") { + t.Errorf("qcow2 overlay must stay buffered so the shared base keeps one page-cache copy: %s", got) + } + if !strings.Contains(got, "backing_files=on") { + t.Errorf("qcow2 overlay must keep backing_files=on: %s", got) + } +} + +func ptr[T any](v T) *T { return &v } From 886e829d24849439a8fa3bebc517de8282fe38af Mon Sep 17 00:00:00 2001 From: CMGS Date: Wed, 12 Aug 2026 13:34:53 +0800 Subject: [PATCH 2/2] review: keep Store contiguous with its method set asl flags RefuseManifest for splitting the Store type from its first method. It is a standalone function, so it belongs with the other utilities below the method sets, exported ones first. --- meta/sqlite/store.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/meta/sqlite/store.go b/meta/sqlite/store.go index 3aab7ad5..c74dbb90 100644 --- a/meta/sqlite/store.go +++ b/meta/sqlite/store.go @@ -81,15 +81,6 @@ func OpenForRecovery(dbPath string, namespaces ...Namespace) (*Store, error) { return openStore(dbPath, namespaces) } -// RefuseManifest fails when a conversion manifest sits beside dbPath, meaning an offline conversion is unfinished. -func RefuseManifest(dbPath string) error { - manifest := filepath.Join(filepath.Dir(dbPath), ManifestName) - if utils.FileExists(manifest) { - return fmt.Errorf("%s exists: a conversion is in flight, run `cocoon meta convert` to finish it", manifest) - } - return nil -} - func openStore(dbPath string, namespaces []Namespace) (*Store, error) { // The driver creates a file on first touch; Open never creates — that is Init's job (§6) — and §4 refuses network filesystems before WAL work. if !utils.FileExists(dbPath) { @@ -381,6 +372,15 @@ func (h *txHandle) checkRead(ns string) error { return nil } +// RefuseManifest fails when a conversion manifest sits beside dbPath, meaning an offline conversion is unfinished. +func RefuseManifest(dbPath string) error { + manifest := filepath.Join(filepath.Dir(dbPath), ManifestName) + if utils.FileExists(manifest) { + return fmt.Errorf("%s exists: a conversion is in flight, run `cocoon meta convert` to finish it", manifest) + } + return nil +} + func tableName(ns, table string) string { return quoteIdent(ns + "__" + table) }