From 99e9ee4de9b1cfc49d52bd7820b84cd6a111cc41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Tue, 28 Jul 2026 18:47:25 -0400 Subject: [PATCH 1/7] incusd/storage: Strip sub-path from dependent volume sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #3738 Signed-off-by: Stéphane Graber --- internal/server/storage/backend.go | 33 ++++++++++++++++++++---------- internal/server/storage/utils.go | 10 ++++++--- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/internal/server/storage/backend.go b/internal/server/storage/backend.go index 872ae3a446..de51bb7458 100644 --- a/internal/server/storage/backend.go +++ b/internal/server/storage/backend.go @@ -1262,7 +1262,9 @@ func (b *backend) CreateInstanceFromCopy(inst instance.Instance, src instance.In return fmt.Errorf("Failed loading storage pool: %w", err) } - err = diskPool.CreateCustomVolumeFromCopy(inst.Project().Name, src.Project().Name, newDevices[dev.Name]["source"], "", nil, dev.Config["pool"], dev.Config["source"], snapshots, op) + newVolName, _ := internalInstance.SplitVolumeSource(newDevices[dev.Name]["source"]) + srcVolName, _ := internalInstance.SplitVolumeSource(dev.Config["source"]) + err = diskPool.CreateCustomVolumeFromCopy(inst.Project().Name, src.Project().Name, newVolName, "", nil, dev.Config["pool"], srcVolName, snapshots, op) if err != nil { return err } @@ -1808,7 +1810,9 @@ func (b *backend) RefreshInstance(inst instance.Instance, src instance.Instance, return fmt.Errorf("Failed loading storage pool: %w", err) } - err = diskPool.RefreshCustomVolume(inst.Project().Name, src.Project().Name, newDevices[dev.Name]["source"], "", nil, dev.Config["pool"], dev.Config["source"], snapshots, false, op) + newVolName, _ := internalInstance.SplitVolumeSource(newDevices[dev.Name]["source"]) + srcVolName, _ := internalInstance.SplitVolumeSource(dev.Config["source"]) + err = diskPool.RefreshCustomVolume(inst.Project().Name, src.Project().Name, newVolName, "", nil, dev.Config["pool"], srcVolName, snapshots, false, op) if err != nil { return err } @@ -3020,7 +3024,8 @@ func (b *backend) BackupInstance(inst instance.Instance, tarWriter *instancewrit return fmt.Errorf("Failed loading storage pool: %w", err) } - err = diskPool.BackupCustomVolume(inst.Project().Name, dev.Config["source"], tarWriter, filepath.Join(backup.DefaultBackupPrefix, dev.Name), optimized, snapshots, op) + volName, _ := internalInstance.SplitVolumeSource(dev.Config["source"]) + err = diskPool.BackupCustomVolume(inst.Project().Name, volName, tarWriter, filepath.Join(backup.DefaultBackupPrefix, dev.Name), optimized, snapshots, op) if err != nil { return err } @@ -3465,14 +3470,16 @@ func (b *backend) CreateInstanceSnapshot(inst instance.Instance, src instance.In return fmt.Errorf("Failed loading storage pool: %w", err) } + volName, _ := internalInstance.SplitVolumeSource(dev.Config["source"]) + _, snapshotName, _ := api.GetParentAndSnapshotName(inst.Name()) - err = diskPool.CreateCustomVolumeSnapshot(inst.Project().Name, dev.Config["source"], snapshotName, time.Time{}, inst.IsStateful(), op) + err = diskPool.CreateCustomVolumeSnapshot(inst.Project().Name, volName, snapshotName, time.Time{}, inst.IsStateful(), op) if err != nil { - return fmt.Errorf("Failed to create device snapshot for volume %q: %w", dev.Config["source"], err) + return fmt.Errorf("Failed to create device snapshot for volume %q: %w", volName, err) } reverter.Add(func() { - _ = diskPool.DeleteCustomVolumeSnapshot(inst.Project().Name, fmt.Sprintf("%s/%s", dev.Config["source"], snapshotName), nil) + _ = diskPool.DeleteCustomVolumeSnapshot(inst.Project().Name, fmt.Sprintf("%s/%s", volName, snapshotName), nil) }) return nil @@ -3682,9 +3689,10 @@ func (b *backend) DeleteInstanceSnapshot(inst instance.Instance, op *operations. return fmt.Errorf("Failed loading storage pool: %w", err) } - err = diskPool.DeleteCustomVolumeSnapshot(inst.Project().Name, fmt.Sprintf("%s/%s", dev.Config["source"], snapName), op) + volName, _ := internalInstance.SplitVolumeSource(dev.Config["source"]) + err = diskPool.DeleteCustomVolumeSnapshot(inst.Project().Name, fmt.Sprintf("%s/%s", volName, snapName), op) if err != nil { - return fmt.Errorf("Failed to delete snapshot for volume %q: %w", dev.Config["source"], err) + return fmt.Errorf("Failed to delete snapshot for volume %q: %w", volName, err) } return nil @@ -3886,7 +3894,8 @@ func (b *backend) RestoreInstanceSnapshot(inst instance.Instance, src instance.I return fmt.Errorf("Failed loading storage pool: %w", err) } - err = diskPool.RestoreCustomVolume(inst.Project().Name, dev.Config["source"], snapshotName, op) + volName, _ := internalInstance.SplitVolumeSource(dev.Config["source"]) + err = diskPool.RestoreCustomVolume(inst.Project().Name, volName, snapshotName, op) if err != nil { return err } @@ -7084,7 +7093,8 @@ func (b *backend) GenerateInstanceBackupConfig(inst instance.Instance, snapshots return fmt.Errorf("Failed loading storage pool: %w", err) } - diskConfig, err := diskPool.GenerateCustomVolumeBackupConfig(inst.Project().Name, dev.Config["source"], snapshots, op) + volName, _ := internalInstance.SplitVolumeSource(dev.Config["source"]) + diskConfig, err := diskPool.GenerateCustomVolumeBackupConfig(inst.Project().Name, volName, snapshots, op) if err != nil { return err } @@ -9735,7 +9745,8 @@ func (b *backend) createDependentVolumesFromBackup(srcBackup backup.Info, srcDat continue } - devKey := fmt.Sprintf("%s/%s", dev["pool"], dev["source"]) + volName, _ := internalInstance.SplitVolumeSource(dev["source"]) + devKey := fmt.Sprintf("%s/%s", dev["pool"], volName) devicesMap[devKey] = devName } diff --git a/internal/server/storage/utils.go b/internal/server/storage/utils.go index bb6d9e210e..6ef4c1a6cf 100644 --- a/internal/server/storage/utils.go +++ b/internal/server/storage/utils.go @@ -1468,8 +1468,11 @@ func DependentVolumesMatchMigrationType(s *state.State, migrationDependentVolume // needs to be migrated for this instance. Returns false if migration // can be skipped (e.g., on shared storage within the same cluster). func ShouldMigrateDependentVolume(s *state.State, poolName string, volumeName string, overrides map[string]string, clusterMove bool) (bool, error) { - if overrides != nil && ((overrides["source"] != "" && volumeName != overrides["source"]) || (overrides["pool"] != "" && poolName != overrides["pool"])) { - return true, nil + if overrides != nil { + overrideVolName, _ := internalInstance.SplitVolumeSource(overrides["source"]) + if (overrides["source"] != "" && volumeName != overrideVolName) || (overrides["pool"] != "" && poolName != overrides["pool"]) { + return true, nil + } } diskPool, err := LoadByName(s, poolName) @@ -1575,7 +1578,8 @@ func DevicesMapFromBackupConfig(config *backupConfig.Config) map[string]map[stri devicesMap[dev["pool"]] = map[string]string{} } - devicesMap[dev["pool"]][dev["source"]] = devName + volName, _ := internalInstance.SplitVolumeSource(dev["source"]) + devicesMap[dev["pool"]][volName] = devName } return devicesMap From 1f1755d40c8e5534e80770eb4674142db0e9d808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Tue, 28 Jul 2026 18:47:25 -0400 Subject: [PATCH 2/7] incusd/migration: Strip sub-path from dependent volume source overrides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- internal/server/migration/migration_volumes.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/server/migration/migration_volumes.go b/internal/server/migration/migration_volumes.go index 4e6c6e562b..8e0edd1e57 100644 --- a/internal/server/migration/migration_volumes.go +++ b/internal/server/migration/migration_volumes.go @@ -8,6 +8,7 @@ import ( "google.golang.org/protobuf/proto" + internalInstance "github.com/lxc/incus/v7/internal/instance" "github.com/lxc/incus/v7/internal/migration" backupConfig "github.com/lxc/incus/v7/internal/server/backup/config" "github.com/lxc/incus/v7/internal/server/operations" @@ -311,7 +312,7 @@ func ProtobufToDependentVolume(volume *migration.DependentVolume, migrationType } if overrides["source"] != "" { - volName = overrides["source"] + volName, _ = internalInstance.SplitVolumeSource(overrides["source"]) } } From 60e937362b4c2af51524dee4088d67ed07ac0ddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Tue, 28 Jul 2026 18:47:25 -0400 Subject: [PATCH 3/7] incusd/instance: Strip sub-path from dependent volume sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- internal/server/instance/drivers/driver_lxc.go | 3 ++- internal/server/instance/drivers/driver_qemu.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/server/instance/drivers/driver_lxc.go b/internal/server/instance/drivers/driver_lxc.go index ab32bd67c3..2d55e2a5d8 100644 --- a/internal/server/instance/drivers/driver_lxc.go +++ b/internal/server/instance/drivers/driver_lxc.go @@ -4579,7 +4579,8 @@ func (d *lxc) delete(force bool, cleanupDependencies bool) error { return fmt.Errorf("Failed loading storage pool: %w", err) } - err = diskPool.DeleteCustomVolume(d.Project().Name, dev.Config["source"], nil) + volName, _ := internalInstance.SplitVolumeSource(dev.Config["source"]) + err = diskPool.DeleteCustomVolume(d.Project().Name, volName, nil) if err != nil { return err } diff --git a/internal/server/instance/drivers/driver_qemu.go b/internal/server/instance/drivers/driver_qemu.go index 1f9b6bfc1c..70a36d0d89 100644 --- a/internal/server/instance/drivers/driver_qemu.go +++ b/internal/server/instance/drivers/driver_qemu.go @@ -7675,7 +7675,8 @@ func (d *qemu) delete(force bool, cleanupDependencies bool) error { return fmt.Errorf("Failed loading storage pool: %w", err) } - err = diskPool.DeleteCustomVolume(d.Project().Name, dev.Config["source"], nil) + volName, _ := internalInstance.SplitVolumeSource(dev.Config["source"]) + err = diskPool.DeleteCustomVolume(d.Project().Name, volName, nil) if err != nil { return err } From d37ae5fa2455f142452c750aebc27abb35296223 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Tue, 28 Jul 2026 18:47:25 -0400 Subject: [PATCH 4/7] incusd: Strip sub-path from dependent volume sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- cmd/incusd/instance_post.go | 9 ++++++--- cmd/incusd/instances_post.go | 4 +++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/cmd/incusd/instance_post.go b/cmd/incusd/instance_post.go index 099edd80b1..242c2487fb 100644 --- a/cmd/incusd/instance_post.go +++ b/cmd/incusd/instance_post.go @@ -1086,11 +1086,14 @@ func cleanupDependentDisks(s *state.State, inst instance.Instance, deviceOverrid return fmt.Errorf("Failed loading storage pool: %w", err) } + volName, _ := internalInstance.SplitVolumeSource(dev.Config["source"]) + // If new disk was created than delete source volume. override, ok := deviceOverrides[dev.Name] if ok { - if (override["source"] != "" && override["source"] != dev.Config["source"]) || (override["pool"] != "" && override["pool"] != dev.Config["pool"]) { - _ = diskPool.DeleteCustomVolume(inst.Project().Name, dev.Config["source"], op) + overrideVolName, _ := internalInstance.SplitVolumeSource(override["source"]) + if (override["source"] != "" && overrideVolName != volName) || (override["pool"] != "" && override["pool"] != dev.Config["pool"]) { + _ = diskPool.DeleteCustomVolume(inst.Project().Name, volName, op) } } @@ -1099,7 +1102,7 @@ func cleanupDependentDisks(s *state.State, inst instance.Instance, deviceOverrid return nil } - _ = diskPool.DeleteCustomVolume(inst.Project().Name, dev.Config["source"], op) + _ = diskPool.DeleteCustomVolume(inst.Project().Name, volName, op) return nil }) diff --git a/cmd/incusd/instances_post.go b/cmd/incusd/instances_post.go index 126294ee8b..b70bb5cc20 100644 --- a/cmd/incusd/instances_post.go +++ b/cmd/incusd/instances_post.go @@ -561,7 +561,9 @@ func validateDependentVolumes(source instance.Instance, req *api.InstancesPost) } // Check if the source was overridden. - if oldDevice["source"] == newDevice["source"] { + oldVolName, _ := internalInstance.SplitVolumeSource(oldDevice["source"]) + newVolName, _ := internalInstance.SplitVolumeSource(newDevice["source"]) + if oldVolName == newVolName { return fmt.Errorf("Device source name should be different during copy for dependent disk: %s", key) } } From c513d8e3e1c60c4733917123489d4075e32dea09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Tue, 28 Jul 2026 18:50:12 -0400 Subject: [PATCH 5/7] incusd/storage: Allow unattached volumes in qcow2 migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- internal/server/storage/backend.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/server/storage/backend.go b/internal/server/storage/backend.go index de51bb7458..677ba023d0 100644 --- a/internal/server/storage/backend.go +++ b/internal/server/storage/backend.go @@ -8985,7 +8985,7 @@ func (b *backend) qcow2MigrateVolume(s *state.State, vol drivers.Volume, project _, volName := project.StorageVolumeParts(vol.Name()) inst, diskName, err := InstanceByVolumeName(b.state, vol.Pool(), projectName, volName, volumeDbType) - if err != nil { + if err != nil && !errors.Is(err, ErrVolumeNotAttachedToRunningInstance) { return err } @@ -9175,7 +9175,7 @@ func (b *backend) qcow2CreateVolumeFromMigration(vol drivers.Volume, projectName _, volName := project.StorageVolumeParts(vol.Name()) inst, _, err := InstanceByVolumeName(b.state, vol.Pool(), projectName, volName, volumeDbType) - if err != nil { + if err != nil && !errors.Is(err, ErrVolumeNotAttachedToRunningInstance) { return err } From d4752a6ce021fa52308d8e04558b7ebba1313898 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Tue, 28 Jul 2026 22:43:05 -0400 Subject: [PATCH 6/7] incusd/instance/qmp: Add copy-before-write export helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- .../server/instance/drivers/qmp/commands.go | 54 +++++++++++++------ 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/internal/server/instance/drivers/qmp/commands.go b/internal/server/instance/drivers/qmp/commands.go index 85fc7a88b2..d2bed0eac2 100644 --- a/internal/server/instance/drivers/qmp/commands.go +++ b/internal/server/instance/drivers/qmp/commands.go @@ -1259,8 +1259,9 @@ func (m *Monitor) NBDServerStop() error { return nil } -// NBDBlockExportAdd exports a writable device via the NBD server. -func (m *Monitor) NBDBlockExportAdd(deviceNodeName string, exportName string, writable bool, bitmapNames []string) error { +// NBDBlockExportAdd adds a block export to the NBD server. Bitmaps are looked up on +// bitmapNode, which defaults to the exported node when empty. +func (m *Monitor) NBDBlockExportAdd(deviceNodeName string, exportName string, writable bool, bitmapNode string, bitmapNames []string) error { var args struct { ID string `json:"id"` Type string `json:"type"` @@ -1279,12 +1280,16 @@ func (m *Monitor) NBDBlockExportAdd(deviceNodeName string, exportName string, wr args.Name = exportName args.Writable = writable + if bitmapNode == "" { + bitmapNode = deviceNodeName + } + for _, b := range bitmapNames { args.Bitmaps = append(args.Bitmaps, struct { Node string `json:"node"` Name string `json:"name"` }{ - Node: deviceNodeName, + Node: bitmapNode, Name: b, }) } @@ -1403,25 +1408,27 @@ func (m *Monitor) BlockDevSnapshot(deviceNodeName string, snapshotNodeName strin return nil } -// BlockDevSnapshotTarget describes a single blockdev-snapshot action. -type BlockDevSnapshotTarget struct { - Node string `json:"node"` - Overlay string `json:"overlay"` +// BlockDevBackupTarget describes a single blockdev-backup action. +type BlockDevBackupTarget struct { + Device string `json:"device"` + Target string `json:"target"` + Sync string `json:"sync"` + JobID string `json:"job-id"` } -// BlockDevSnapshotTransaction atomically creates the given device snapshots in a single -// transaction so that all overlays are taken at the same point in time. -func (m *Monitor) BlockDevSnapshotTransaction(snapshots []BlockDevSnapshotTarget) error { +// BlockDevBackupTransaction atomically starts the given blockdev-backup jobs in a single +// transaction so that all targets share the same point in time. +func (m *Monitor) BlockDevBackupTransaction(backups []BlockDevBackupTarget) error { type action struct { - Type string `json:"type"` - Data BlockDevSnapshotTarget `json:"data"` + Type string `json:"type"` + Data BlockDevBackupTarget `json:"data"` } - actions := make([]action, 0, len(snapshots)) - for _, snapshot := range snapshots { + actions := make([]action, 0, len(backups)) + for _, backup := range backups { actions = append(actions, action{ - Type: "blockdev-snapshot", - Data: snapshot, + Type: "blockdev-backup", + Data: backup, }) } @@ -1632,6 +1639,21 @@ func (m *Monitor) BlockJobCancel(deviceNodeName string) error { return nil } +// BlockJobCancelWait cancels an ongoing block job and waits until it is gone. +func (m *Monitor) BlockJobCancelWait(jobID string) error { + err := m.BlockJobCancel(jobID) + if err != nil { + return err + } + + _, err = m.blockJobWait(jobID, false, true) + if err != nil { + return err + } + + return nil +} + // BlockJobComplete completes a block job that is in ready state. func (m *Monitor) BlockJobComplete(deviceNodeName string) error { var args struct { From 7dbbb0fc9b650aac5084711ac27844aab3b93da1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Tue, 28 Jul 2026 22:43:05 -0400 Subject: [PATCH 7/7] incusd/instance/qemu: Use copy-before-write overlays for NBD exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #3741 Signed-off-by: Stéphane Graber --- .../server/instance/drivers/driver_qemu.go | 138 ++++++++++++------ 1 file changed, 90 insertions(+), 48 deletions(-) diff --git a/internal/server/instance/drivers/driver_qemu.go b/internal/server/instance/drivers/driver_qemu.go index 70a36d0d89..fe9a55c519 100644 --- a/internal/server/instance/drivers/driver_qemu.go +++ b/internal/server/instance/drivers/driver_qemu.go @@ -1048,7 +1048,7 @@ func (d *qemu) receiveMigrationSnapshot(monitor *qmp.Monitor, blockExport string _ = monitor.NBDServerStop() }() - err = monitor.NBDBlockExportAdd(blockExport, blockExport, true, nil) + err = monitor.NBDBlockExportAdd(blockExport, blockExport, true, "", nil) if err != nil { return fmt.Errorf("Failed adding root disk to NBD server: %w", err) } @@ -8298,7 +8298,9 @@ func (d *qemu) MigrateSend(args instance.MigrateSendArgs) error { } // prepareEphemeralSnapshot sets up an overlay block device suitable for short lived operations. -func (d *qemu) prepareEphemeralSnapshot(monitor *qmp.Monitor, diskName string, diskSize int64) (string, string, func(), error) { +// When backed is true, the overlay is opened with the disk's current top node as its backing +// node, as needed by copy-before-write overlays. +func (d *qemu) prepareEphemeralSnapshot(monitor *qmp.Monitor, diskName string, diskSize int64, backed bool) (string, string, func(), error) { snapshotDiskName := ephemeralSnapshotName(diskName) // Create snapshot of the disk. @@ -8345,33 +8347,7 @@ func (d *qemu) prepareEphemeralSnapshot(monitor *qmp.Monitor, diskName string, d _ = snapFile.Close() // Don't prevent clean unmount when instance is stopped. - // Add the snapshot file as a block device (not visible to the guest OS). - err = monitor.AddBlockDevice(map[string]any{ - "driver": "qcow2", - "node-name": snapshotDiskName, - "read-only": false, - "file": map[string]any{ - "driver": "file", - "filename": fmt.Sprintf("/dev/fdset/%d", info.ID), - }, - }, nil, false) - if err != nil { - return "", "", nil, fmt.Errorf("Failed adding migration storage snapshot block device: %w", err) - } - - reverter := revert.New() - defer reverter.Fail() - - removeOverlay := func() { - err := monitor.RemoveBlockDevice(snapshotDiskName) - if err != nil { - d.logger.Error("Failed removing temporary snapshot disk device", logger.Ctx{"err": err}) - } - } - - reverter.Add(removeOverlay) - - // Find the base block device that writes should be redirected away from. + // Find the disk's current top node, the base of the new overlay. blockDevs, err := d.fetchBlockDeviceChain(monitor, diskName) if err != nil { return "", "", nil, fmt.Errorf("Failed fetching block device chain: %w", err) @@ -8379,7 +8355,32 @@ func (d *qemu) prepareEphemeralSnapshot(monitor *qmp.Monitor, diskName string, d blockDevName := blockDevs[len(blockDevs)-1] - reverter.Success() + blockDev := map[string]any{ + "driver": "qcow2", + "node-name": snapshotDiskName, + "read-only": false, + "file": map[string]any{ + "driver": "file", + "filename": fmt.Sprintf("/dev/fdset/%d", info.ID), + }, + } + + if backed { + blockDev["backing"] = blockDevName + } + + // Add the snapshot file as a block device (not visible to the guest OS). + err = monitor.AddBlockDevice(blockDev, nil, false) + if err != nil { + return "", "", nil, fmt.Errorf("Failed adding migration storage snapshot block device: %w", err) + } + + removeOverlay := func() { + err := monitor.RemoveBlockDevice(snapshotDiskName) + if err != nil { + d.logger.Error("Failed removing temporary snapshot disk device", logger.Ctx{"err": err}) + } + } return snapshotDiskName, blockDevName, removeOverlay, nil } @@ -8421,6 +8422,22 @@ func (d *qemu) mergeEphemeralSnapshot(monitor *qmp.Monitor, overlayNode string) return nil } +// removeEphemeralOverlay tears down a copy-before-write overlay, cancelling its backup job first. +func (d *qemu) removeEphemeralOverlay(monitor *qmp.Monitor, overlayNode string) error { + // Cancel the copy-before-write job if it is still running. + err := monitor.BlockJobCancelWait(overlayNode) + if err != nil { + d.logger.Debug("Failed cancelling overlay block job", logger.Ctx{"overlay": overlayNode, "err": err}) + } + + err = monitor.RemoveBlockDevice(overlayNode) + if err != nil { + return fmt.Errorf("Failed removing temporary snapshot overlay %q: %w", overlayNode, err) + } + + return nil +} + // createEphemeralSnapshot creates a temporary snapshot of the disk that is intended for short-lived operations. func (d *qemu) createEphemeralSnapshot(diskName string, diskSize int64) (func(), error) { monitor, err := d.qmpConnect() @@ -8428,7 +8445,7 @@ func (d *qemu) createEphemeralSnapshot(diskName string, diskSize int64) (func(), return nil, err } - snapshotDiskName, blockDevName, removeOverlay, err := d.prepareEphemeralSnapshot(monitor, diskName, diskSize) + snapshotDiskName, blockDevName, removeOverlay, err := d.prepareEphemeralSnapshot(monitor, diskName, diskSize, false) if err != nil { return nil, err } @@ -11677,7 +11694,7 @@ func (d *qemu) ExportQcow2Block(diskName string, blockIndex int) (func(), string exportDiskPath := fmt.Sprintf("nbd+unix:///%s?socket=%s", exportBlockName, shortSocketPath) - err = monitor.NBDBlockExportAdd(exportBlockName, exportBlockName, false, nil) + err = monitor.NBDBlockExportAdd(exportBlockName, exportBlockName, false, "", nil) if err != nil { return nil, "", fmt.Errorf("Failed adding disk to NBD server: %w", err) } @@ -11829,10 +11846,19 @@ func (d *qemu) ConnectNBD(diskName string, volSize int64, writable bool) (net.Co reverter := revert.New() defer reverter.Fail() + overlayNode := "" + disconnect := func() { d.logger.Debug("User requested NBD server stopped") _ = nbdConn.Close() _ = monitor.NBDServerStop() + + if overlayNode != "" { + err := d.removeEphemeralOverlay(monitor, overlayNode) + if err != nil { + d.logger.Error("Failed removing temporary snapshot overlay", logger.Ctx{"overlay": overlayNode, "err": err}) + } + } } reverter.Add(disconnect) @@ -11860,17 +11886,27 @@ func (d *qemu) ConnectNBD(diskName string, volSize int64, writable bool) (net.Co } blockExport := blockDevs[len(blockDevs)-1] + exportNode := blockExport if !writable { - cleanupSnapshot, err := d.createEphemeralSnapshot(blockExport, volSize) + // Expose a frozen view of the disk through a copy-before-write overlay + // (see ConnectNBDAllDisks). + snapNode, baseNode, removeOverlay, err := d.prepareEphemeralSnapshot(monitor, blockExport, volSize, true) if err != nil { return nil, nil, fmt.Errorf("Failed creating temporary snapshot: %w", err) } - reverter.Add(cleanupSnapshot) + err = monitor.BlockDevBackupTransaction([]qmp.BlockDevBackupTarget{{Device: baseNode, Target: snapNode, Sync: "none", JobID: snapNode}}) + if err != nil { + removeOverlay() + return nil, nil, fmt.Errorf("Failed creating temporary snapshot: %w", err) + } + + overlayNode = snapNode + exportNode = snapNode } - err = monitor.NBDBlockExportAdd(blockExport, "", writable, bitmapNames) + err = monitor.NBDBlockExportAdd(exportNode, "", writable, blockExport, bitmapNames) if err != nil { return nil, nil, fmt.Errorf("Failed adding disk to NBD server: %w", err) } @@ -11998,9 +12034,9 @@ func (d *qemu) ConnectNBDAllDisks(reuse bool) (net.Conn, func(), error) { continue } - err = d.mergeEphemeralSnapshot(monitor, overlayNode) + err = d.removeEphemeralOverlay(monitor, overlayNode) if err != nil { - return nil, nil, fmt.Errorf("Failed recovering disk %q from an earlier failed snapshot merge: %w", devName, err) + return nil, nil, fmt.Errorf("Failed recovering disk %q from an earlier failed teardown: %w", devName, err) } } @@ -12029,14 +12065,17 @@ func (d *qemu) ConnectNBDAllDisks(reuse bool) (net.Conn, func(), error) { type exportTarget struct { deviceName string exportNode string + bitmapNode string bitmaps []string } targets := make([]exportTarget, 0, len(deviceNames)) - snapshots := make([]qmp.BlockDevSnapshotTarget, 0, len(deviceNames)) + backups := make([]qmp.BlockDevBackupTarget, 0, len(deviceNames)) overlays := make([]string, 0, len(deviceNames)) - // Prepare an overlay for each disk so the guest keeps running while we export a frozen view. + // Prepare a copy-before-write overlay for each disk, exposing a frozen view of it while the + // guest keeps writing to the disk itself. Snapshotting the disk instead would reopen it and + // its persistent dirty bitmaps read-only, making the eventual merge of the snapshot fail. for _, devName := range deviceNames { nodeName := d.blockNodeName(linux.PathNameEncode(devName)) @@ -12059,26 +12098,26 @@ func (d *qemu) ConnectNBDAllDisks(reuse bool) (net.Conn, func(), error) { return nil, nil, fmt.Errorf("Failed fetching size for %q: %w", devName, err) } - overlayNode, baseNode, removeOverlay, err := d.prepareEphemeralSnapshot(monitor, nodeName, diskSize) + overlayNode, baseNode, removeOverlay, err := d.prepareEphemeralSnapshot(monitor, nodeName, diskSize, true) if err != nil { return nil, nil, fmt.Errorf("Failed creating temporary snapshot for %q: %w", devName, err) } reverter.Add(removeOverlay) - snapshots = append(snapshots, qmp.BlockDevSnapshotTarget{Node: baseNode, Overlay: overlayNode}) - targets = append(targets, exportTarget{deviceName: devName, exportNode: baseNode, bitmaps: bitmapNames}) + backups = append(backups, qmp.BlockDevBackupTarget{Device: baseNode, Target: overlayNode, Sync: "none", JobID: overlayNode}) + targets = append(targets, exportTarget{deviceName: devName, exportNode: overlayNode, bitmapNode: baseNode, bitmaps: bitmapNames}) overlays = append(overlays, overlayNode) } - // Create all overlays atomically so the exported disks share a consistent point in time. - err = monitor.BlockDevSnapshotTransaction(snapshots) + // Start all copy-before-write jobs atomically so the exported disks share a consistent + // point in time. + err = monitor.BlockDevBackupTransaction(backups) if err != nil { return nil, nil, fmt.Errorf("Failed creating consistent storage snapshot: %w", err) } - // The overlays are now active and hold the guest's ongoing writes, so they must be committed - // back rather than simply removed. Take over cleanup from the reverter. + // The overlays never hold guest writes, teardown cancels the jobs and drops them. reverter.Success() stop := func() { @@ -12093,13 +12132,16 @@ func (d *qemu) ConnectNBDAllDisks(reuse bool) (net.Conn, func(), error) { _ = os.Remove(d.nbdPath()) for _, overlayNode := range overlays { - _ = d.mergeEphemeralSnapshot(monitor, overlayNode) + err := d.removeEphemeralOverlay(monitor, overlayNode) + if err != nil { + d.logger.Error("Failed removing temporary snapshot overlay", logger.Ctx{"overlay": overlayNode, "err": err}) + } } } // Add an NBD export per disk, using the Incus device name as the export name. for _, target := range targets { - err = monitor.NBDBlockExportAdd(target.exportNode, target.deviceName, false, target.bitmaps) + err = monitor.NBDBlockExportAdd(target.exportNode, target.deviceName, false, target.bitmapNode, target.bitmaps) if err != nil { stop() return nil, nil, fmt.Errorf("Failed adding disk %q to NBD server: %w", target.deviceName, err)