The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/8013
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === - Moves check for preventing update of devices when VM is running into updateDevices(). - Fixes regression introduced by 6697599#diff-8ac4860b5f669c17c92c37545fe2ecfd. - Removes 1 call to IsRunning() by passing into updateDevices() from Update().
From 57f1dc94a63d5faf175b096f280269adcca51786 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Fri, 9 Oct 2020 19:39:09 +0100 Subject: [PATCH 1/2] lxd/instance/drivers/driver/qemu: Restores ability to resize VM disks - Moves check for preventing update of devices when VM is running into updateDevices(). - Fixes regression introduced by https://github.com/lxc/lxd/commit/6697599975c1199c226c00b68c7e3e278ea49c66#diff-8ac4860b5f669c17c92c37545fe2ecfd. - Removes 1 call to IsRunning() by passing into updateDevices() from Update(). Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/instance/drivers/driver_qemu.go | 44 +++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go index 0b697cbf58..7c641ef033 100644 --- a/lxd/instance/drivers/driver_qemu.go +++ b/lxd/instance/drivers/driver_qemu.go @@ -2836,7 +2836,37 @@ func (vm *qemu) Update(args db.InstanceArgs, userRequested bool) error { } // Diff the devices. - removeDevices, addDevices, updateDevices, allUpdatedKeys := oldExpandedDevices.Update(vm.expandedDevices, nil) + removeDevices, addDevices, updateDevices, allUpdatedKeys := oldExpandedDevices.Update(vm.expandedDevices, func(oldDevice deviceConfig.Device, newDevice deviceConfig.Device) []string { + // This function needs to return a list of fields that are excluded from differences + // between oldDevice and newDevice. The result of this is that as long as the + // devices are otherwise identical except for the fields returned here, then the + // device is considered to be being "updated" rather than "added & removed". + oldNICType, err := nictype.NICType(vm.state, vm.Project(), newDevice) + if err != nil { + return []string{} // Cannot hot-update due to config error. + } + + newNICType, err := nictype.NICType(vm.state, vm.Project(), oldDevice) + if err != nil { + return []string{} // Cannot hot-update due to config error. + } + + if oldDevice["type"] != newDevice["type"] || oldNICType != newNICType { + return []string{} // Device types aren't the same, so this cannot be an update. + } + + d, err := device.New(vm, vm.state, "", newDevice, nil, nil) + if err != nil { + return []string{} // Couldn't create Device, so this cannot be an update. + } + + // TODO modify device framework to differentiate between devices that can only be updated when VM + // is stopped, but don't need to be removed then re-added. For now we rely upon the disk device + // indicating that it supports hot plugging, even for VMs, which it cannot. However this is + // needed so that the disk device's Update() function is called to allow disk resizing. + _, updateFields := d.CanHotPlug() + return updateFields + }) if userRequested { // Do some validation of the config diff. @@ -2854,12 +2884,8 @@ func (vm *qemu) Update(args db.InstanceArgs, userRequested bool) error { isRunning := vm.IsRunning() - if isRunning && len(allUpdatedKeys) > 0 { - return fmt.Errorf("Devices cannot be changed when VM is running") - } - // Use the device interface to apply update changes. - err = vm.updateDevices(removeDevices, addDevices, updateDevices, oldExpandedDevices) + err = vm.updateDevices(removeDevices, addDevices, updateDevices, oldExpandedDevices, isRunning) if err != nil { return err } @@ -3055,8 +3081,10 @@ func (vm *qemu) updateMemoryLimit(newLimit string) error { return fmt.Errorf("Failed setting memory to %d bytes (currently %d bytes) as it was taking too long", newSizeBytes, curSizeBytes) } -func (vm *qemu) updateDevices(removeDevices deviceConfig.Devices, addDevices deviceConfig.Devices, updateDevices deviceConfig.Devices, oldExpandedDevices deviceConfig.Devices) error { - isRunning := vm.IsRunning() +func (vm *qemu) updateDevices(removeDevices deviceConfig.Devices, addDevices deviceConfig.Devices, updateDevices deviceConfig.Devices, oldExpandedDevices deviceConfig.Devices, isRunning bool) error { + if isRunning && (len(removeDevices) > 0 || len(addDevices) > 0 || len(updateDevices) > 0) { + return fmt.Errorf("Devices cannot be changed when VM is running") + } // Remove devices in reverse order to how they were added. for _, dev := range removeDevices.Reversed() { From 68dde3234a2e89118806ee57f60e8f4cd04ef997 Mon Sep 17 00:00:00 2001 From: Thomas Parrott <thomas.parr...@canonical.com> Date: Fri, 9 Oct 2020 19:44:42 +0100 Subject: [PATCH 2/2] lxd/device/disk: Adds comment about VM instances depending on CanHotPlug fields for stopped disk resize Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com> --- lxd/device/disk.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lxd/device/disk.go b/lxd/device/disk.go index 790618cc4a..4f728007e7 100644 --- a/lxd/device/disk.go +++ b/lxd/device/disk.go @@ -231,6 +231,8 @@ func (d *disk) validateEnvironment() error { // CanHotPlug returns whether the device can be managed whilst the instance is running, it also // returns a list of fields that can be updated without triggering a device remove & add. +// Note: At current time VM instances rely on this function indicating live update of "size" field is possible +// to allow disk resize when VM is stopped. func (d *disk) CanHotPlug() (bool, []string) { return true, []string{"limits.max", "limits.read", "limits.write", "size"} }
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
