From 156cd04ba508624e42138a850ee1361e5fe71c4d Mon Sep 17 00:00:00 2001 From: "Ruben S. Montero" Date: Thu, 20 Jul 2023 09:51:15 +0200 Subject: [PATCH] F OpenNebula/one#6265: Delete of snapshots in the middle of the chain - Add suport for delete snapshots in the middle of the chain in qcow2 (ALLOW_ORPHANS=NO). Example, snapshot 2 below: 0 <- 1 <- 2 <- 3 - It supports removing the last snapshot and active (blockpull) and first one (snapshot 0). The first snapshot cannot be deleted in persistent images because of the underlying link setting. - Snapshots are deleted by blockcommit. For example to delete 2, 3 is committed into 2: 0 <- 1 <- 2: (2+3) A new file 3.current is created in the filesystem to look for the actual file name supporting snapshot 3. - qcow2/shared operations snap_delete, snap_revert and cpds (disk-saveas) has been updated to react to the .current files - The kvm/deploy action has been updated to resolve links in source file attributes. This is needed so the of the libvirt domain does not contain the sysmlinks that changes on the revert/create operations. - The kvm/migrate action also includes a minor modification to resolv the VM disk - Snapshots with more than 1 child (or their relatives) cannot be deleted to not break backing chains. - It has been found that the snap-delete operation in suspend state is insecure as the guest caches may not be updated accordingly. The operation is now not allowed in this state. - The associated state has been removed from oned. It needs to be removed in CLI/Sunstone and APIs - The changes are for VM disk-snapshots. A similar update can be made for images in the datastore/fs snap_delete action. This operation is not implemented in this commit. Note: Output of xmllint may or may not include new lines when multiple matches of an xpath expression occurs. Parsing of xmllint output adds additional new lines to deal with different versions. --- include/Snapshots.h | 64 +++++--- include/VirtualMachine.h | 2 +- include/VirtualMachineDisk.h | 4 +- src/dm/DispatchManagerActions.cc | 28 ++-- src/dm/DispatchManagerStates.cc | 1 - src/image/Image.cc | 2 +- src/image/ImageManagerActions.cc | 4 +- src/image/ImageManagerProtocol.cc | 6 +- src/lcm/LifeCycleActions.cc | 4 - src/lcm/LifeCycleStates.cc | 4 - src/rm/RequestManagerChown.cc | 2 +- src/tm/TransferManagerProtocol.cc | 2 - src/tm_mad/qcow2/cpds | 12 +- src/tm_mad/qcow2/cpds.ssh | 12 +- src/tm_mad/qcow2/ln | 2 +- src/tm_mad/qcow2/snap_delete | 264 +++++++++++++++++++++++++++++- src/tm_mad/qcow2/snap_delete.ssh | 47 +----- src/tm_mad/qcow2/snap_revert | 12 +- src/tm_mad/qcow2/snap_revert.ssh | 11 +- src/vm/Snapshots.cc | 243 +++++++++++++++++++++++---- src/vm/VirtualMachine.cc | 7 +- src/vm/VirtualMachineDisk.cc | 13 +- src/vmm_mad/remotes/kvm/deploy | 75 ++++++--- src/vmm_mad/remotes/kvm/migrate | 4 + 24 files changed, 639 insertions(+), 186 deletions(-) mode change 100755 => 120000 src/tm_mad/qcow2/snap_delete.ssh diff --git a/include/Snapshots.h b/include/Snapshots.h index 53fe4de894..ad08d16e10 100644 --- a/include/Snapshots.h +++ b/include/Snapshots.h @@ -139,12 +139,16 @@ public: int create_snapshot(const std::string& name, long long size_mb); /** - * Check if an snapshot can be deleted (no children, no active) + * Check if an snapshot can be deleted, for the snap_delete operation on + * VMs * @param id of the snapshot * @param error if any * @return true if can be deleted, false otherwise */ - bool test_delete(int id, std::string& error) const; + bool test_delete(int id, bool persistent, std::string& error) const; + + // Version for the snap_delete operation on images + bool test_delete_image(int id, std::string& error) const; /** * Removes the snapshot from the list @@ -173,8 +177,8 @@ public: */ void clear() { - active = -1; - disk_id = -1; + active = -1; + _disk_id = -1; snapshot_template.clear(); snapshot_pool.clear(); @@ -183,15 +187,25 @@ public: /** * Return the disk_id of the snapshot list */ - int get_disk_id() const + int disk_id() const { - return disk_id; + return _disk_id; } + /** + * Sets the disk id for this snapshot list + * @param did the id + */ + void set_disk_id(int did) + { + _disk_id = did; + snapshot_template.replace("DISK_ID", did); + }; + /** * Return the active snapshot id */ - int get_active_id() const + int active_id() const { return active; } @@ -201,20 +215,10 @@ public: */ void clear_disk_id() { - disk_id = -1; + _disk_id = -1; snapshot_template.erase("DISK_ID"); }; - /** - * Sets the disk id for this snapshot list - * @param did the id - */ - void set_disk_id(int did) - { - disk_id = did; - snapshot_template.replace("DISK_ID", did); - }; - /** * @return number of snapshots in the list */ @@ -223,6 +227,11 @@ public: return snapshot_pool.size(); }; + unsigned int next() const + { + return next_snapshot; + } + /** * @return true if snapshot_pool is empty */ @@ -246,14 +255,25 @@ public: /** * @return total snapshot size (virtual) in mb */ - long long get_total_size() const; + long long total_size() const; /** * Get the size (virtual) in mb of the given snapshot * @param id of the snapshot * @return size or 0 if not found */ - long long get_snapshot_size(int id) const; + long long snapshot_size(int id) const; + + /** + * Return Snapshot children + * @param if of the snapshot + * @param children the attribute string + * @return the number of children + * -1 No snapshot + * 0 CHILDREN not defined + * N number of children in "0,2,3,5" ---> 4 + */ + int children(int id, std::string& children) const; /** * Get Attribute from the given snapshot @@ -262,7 +282,7 @@ public: * * @return value or empty if not found */ - std::string get_snapshot_attribute(int id, const char* name) const; + std::string snapshot_attribute(int id, const char* name) const; private: @@ -317,7 +337,7 @@ private: /** * Id of the disk associated with this snapshot list */ - int disk_id; + int _disk_id; /** * Allow to remove parent snapshots and active one diff --git a/include/VirtualMachine.h b/include/VirtualMachine.h index 595f984b5d..81ab2485c2 100644 --- a/include/VirtualMachine.h +++ b/include/VirtualMachine.h @@ -126,7 +126,7 @@ public: DISK_SNAPSHOT_REVERT_POWEROFF = 52, DISK_SNAPSHOT_DELETE_POWEROFF = 53, DISK_SNAPSHOT_SUSPENDED = 54, - DISK_SNAPSHOT_REVERT_SUSPENDED = 55, + //DISK_SNAPSHOT_REVERT_SUSPENDED = 55, DISK_SNAPSHOT_DELETE_SUSPENDED = 56, DISK_SNAPSHOT = 57, //DISK_SNAPSHOT_REVERT = 58, diff --git a/include/VirtualMachineDisk.h b/include/VirtualMachineDisk.h index 88dd7e2bba..28b1e3566b 100644 --- a/include/VirtualMachineDisk.h +++ b/include/VirtualMachineDisk.h @@ -210,7 +210,7 @@ public: return 0; } - return snapshots->get_total_size(); + return snapshots->total_size(); } /** @@ -225,7 +225,7 @@ public: return 0; } - return snapshots->get_snapshot_size(snap_id); + return snapshots->snapshot_size(snap_id); } /** diff --git a/src/dm/DispatchManagerActions.cc b/src/dm/DispatchManagerActions.cc index 383457a551..65cd80ca08 100644 --- a/src/dm/DispatchManagerActions.cc +++ b/src/dm/DispatchManagerActions.cc @@ -2202,8 +2202,7 @@ int DispatchManager::disk_snapshot_revert(int vid, int did, int snap_id, VirtualMachine::VmState state = vm->get_state(); VirtualMachine::LcmState lstate = vm->get_lcm_state(); - if ((state !=VirtualMachine::POWEROFF || lstate !=VirtualMachine::LCM_INIT)&& - (state !=VirtualMachine::SUSPENDED|| lstate !=VirtualMachine::LCM_INIT)) + if (state != VirtualMachine::POWEROFF || lstate != VirtualMachine::LCM_INIT) { oss << "Could not revert to disk snapshot for VM " << vid << ", wrong state " << vm->state_str() << "."; @@ -2236,20 +2235,8 @@ int DispatchManager::disk_snapshot_revert(int vid, int did, int snap_id, close_cp_history(vmpool, vm.get(), VMActions::DISK_SNAPSHOT_REVERT_ACTION, ra); - switch (state) - { - case VirtualMachine::POWEROFF: - vm->set_state(VirtualMachine::ACTIVE); - vm->set_state(VirtualMachine::DISK_SNAPSHOT_REVERT_POWEROFF); - break; - - case VirtualMachine::SUSPENDED: - vm->set_state(VirtualMachine::ACTIVE); - vm->set_state(VirtualMachine::DISK_SNAPSHOT_REVERT_SUSPENDED); - break; - - default: break; - } + vm->set_state(VirtualMachine::ACTIVE); + vm->set_state(VirtualMachine::DISK_SNAPSHOT_REVERT_POWEROFF); vmpool->update(vm.get()); @@ -2305,7 +2292,14 @@ int DispatchManager::disk_snapshot_delete(int vid, int did, int snap_id, return -1; } - if (!snaps->test_delete(snap_id, error_str)) + const VirtualMachineDisk * disk = vm->get_disk(did); + + if (disk == nullptr) + { + return -1; + } + + if (!snaps->test_delete(snap_id, disk->is_persistent(), error_str)) { return -1; } diff --git a/src/dm/DispatchManagerStates.cc b/src/dm/DispatchManagerStates.cc index dfc718532a..2d127ef965 100644 --- a/src/dm/DispatchManagerStates.cc +++ b/src/dm/DispatchManagerStates.cc @@ -39,7 +39,6 @@ void DispatchManager::trigger_suspend_success(int vid) vm->get_lcm_state() == VirtualMachine::PROLOG_MIGRATE_SUSPEND || vm->get_lcm_state() == VirtualMachine::PROLOG_MIGRATE_SUSPEND_FAILURE|| vm->get_lcm_state() == VirtualMachine::DISK_SNAPSHOT_SUSPENDED || - vm->get_lcm_state() == VirtualMachine::DISK_SNAPSHOT_REVERT_SUSPENDED|| vm->get_lcm_state() == VirtualMachine::DISK_SNAPSHOT_DELETE_SUSPENDED)) { VirtualMachineTemplate quota_tmpl; diff --git a/src/image/Image.cc b/src/image/Image.cc index ad09ce4c1a..3569d329f6 100644 --- a/src/image/Image.cc +++ b/src/image/Image.cc @@ -607,7 +607,7 @@ void Image::disk_attribute(VirtualMachineDisk * disk, disk->replace("SIZE", size_mb); } - snap_size = snapshots.get_total_size(); + snap_size = snapshots.total_size(); disk->replace("DISK_SNAPSHOT_TOTAL_SIZE", snap_size); // Force FORMAT and DRIVER from image diff --git a/src/image/ImageManagerActions.cc b/src/image/ImageManagerActions.cc index 4cba238eee..1dede04fd2 100644 --- a/src/image/ImageManagerActions.cc +++ b/src/image/ImageManagerActions.cc @@ -601,7 +601,7 @@ int ImageManager::delete_image(int iid, string& error_str) int uid = img->get_uid(); int gid = img->get_gid(); - long long size = img->get_size() + img->get_snapshots().get_total_size(); + long long size = img->get_size() + img->get_snapshots().total_size(); img.reset(); @@ -1200,7 +1200,7 @@ int ImageManager::delete_snapshot(int iid, int sid, string& error) const Snapshots& snaps = img->get_snapshots(); - if (!snaps.test_delete(sid, error)) + if (!snaps.test_delete_image(sid, error)) { return -1; } diff --git a/src/image/ImageManagerProtocol.cc b/src/image/ImageManagerProtocol.cc index 99d88e9fd5..add50c392b 100644 --- a/src/image/ImageManagerProtocol.cc +++ b/src/image/ImageManagerProtocol.cc @@ -480,7 +480,7 @@ void ImageManager::_rm(unique_ptr msg) gid = image->get_gid(); source = image->get_source(); size = image->get_size(); - snap_size = (image->get_snapshots()).get_total_size(); + snap_size = (image->get_snapshots()).total_size(); if ( image->get_type() == Image::BACKUP ) { @@ -680,7 +680,7 @@ void ImageManager::_snap_delete(unique_ptr msg) ds_id = image->get_ds_id(); uid = image->get_uid(); gid = image->get_gid(); - snap_size = (image->get_snapshots()).get_snapshot_size(snap_id); + snap_size = (image->get_snapshots()).snapshot_size(snap_id); image->delete_snapshot(snap_id); } @@ -795,7 +795,7 @@ void ImageManager::_snap_flatten(unique_ptr msg) ds_id = image->get_ds_id(); uid = image->get_uid(); gid = image->get_gid(); - snap_size = (image->get_snapshots()).get_total_size(); + snap_size = (image->get_snapshots()).total_size(); image->clear_snapshots(); } diff --git a/src/lcm/LifeCycleActions.cc b/src/lcm/LifeCycleActions.cc index 3c6fc5976e..f33e87ff01 100644 --- a/src/lcm/LifeCycleActions.cc +++ b/src/lcm/LifeCycleActions.cc @@ -1135,7 +1135,6 @@ void LifeCycleManager::clean_up_vm(VirtualMachine * vm, bool dispose, case VirtualMachine::DISK_SNAPSHOT_REVERT_POWEROFF: case VirtualMachine::DISK_SNAPSHOT_DELETE_POWEROFF: case VirtualMachine::DISK_SNAPSHOT_SUSPENDED: - case VirtualMachine::DISK_SNAPSHOT_REVERT_SUSPENDED: case VirtualMachine::DISK_SNAPSHOT_DELETE_SUSPENDED: case VirtualMachine::DISK_SNAPSHOT_DELETE: vm->clear_snapshot_disk(); @@ -1509,7 +1508,6 @@ void LifeCycleManager::recover(VirtualMachine * vm, bool success, case VirtualMachine::DISK_SNAPSHOT_REVERT_POWEROFF: case VirtualMachine::DISK_SNAPSHOT_DELETE_POWEROFF: case VirtualMachine::DISK_SNAPSHOT_SUSPENDED: - case VirtualMachine::DISK_SNAPSHOT_REVERT_SUSPENDED: case VirtualMachine::DISK_SNAPSHOT_DELETE_SUSPENDED: case VirtualMachine::DISK_SNAPSHOT: case VirtualMachine::DISK_SNAPSHOT_DELETE: @@ -1747,7 +1745,6 @@ void LifeCycleManager::retry(VirtualMachine * vm) case VirtualMachine::DISK_SNAPSHOT_REVERT_POWEROFF: case VirtualMachine::DISK_SNAPSHOT_DELETE_POWEROFF: case VirtualMachine::DISK_SNAPSHOT_SUSPENDED: - case VirtualMachine::DISK_SNAPSHOT_REVERT_SUSPENDED: case VirtualMachine::DISK_SNAPSHOT_DELETE_SUSPENDED: case VirtualMachine::DISK_SNAPSHOT: case VirtualMachine::DISK_SNAPSHOT_DELETE: @@ -1867,7 +1864,6 @@ void LifeCycleManager::trigger_updatesg(int sgid) case VirtualMachine::DISK_SNAPSHOT_REVERT_POWEROFF: case VirtualMachine::DISK_SNAPSHOT_DELETE_POWEROFF: case VirtualMachine::DISK_SNAPSHOT_SUSPENDED: - case VirtualMachine::DISK_SNAPSHOT_REVERT_SUSPENDED: case VirtualMachine::DISK_SNAPSHOT_DELETE_SUSPENDED: case VirtualMachine::HOTPLUG_SAVEAS_POWEROFF: case VirtualMachine::HOTPLUG_SAVEAS_SUSPENDED: diff --git a/src/lcm/LifeCycleStates.cc b/src/lcm/LifeCycleStates.cc index 2cf4fb5946..aa58b39148 100644 --- a/src/lcm/LifeCycleStates.cc +++ b/src/lcm/LifeCycleStates.cc @@ -1961,7 +1961,6 @@ void LifeCycleManager::trigger_disk_snapshot_success(int vid) break; case VirtualMachine::DISK_SNAPSHOT_REVERT_POWEROFF: - case VirtualMachine::DISK_SNAPSHOT_REVERT_SUSPENDED: vm->log("LCM", Log::INFO, "VM disk snapshot operation completed."); vm->revert_disk_snapshot(disk_id, snap_id, true); break; @@ -2046,7 +2045,6 @@ void LifeCycleManager::trigger_disk_snapshot_success(int vid) break; case VirtualMachine::DISK_SNAPSHOT_SUSPENDED: - case VirtualMachine::DISK_SNAPSHOT_REVERT_SUSPENDED: case VirtualMachine::DISK_SNAPSHOT_DELETE_SUSPENDED: dm->trigger_suspend_success(vid); break; @@ -2117,7 +2115,6 @@ void LifeCycleManager::trigger_disk_snapshot_failure(int vid) case VirtualMachine::DISK_SNAPSHOT_DELETE_POWEROFF: case VirtualMachine::DISK_SNAPSHOT_REVERT_POWEROFF: case VirtualMachine::DISK_SNAPSHOT_DELETE_SUSPENDED: - case VirtualMachine::DISK_SNAPSHOT_REVERT_SUSPENDED: vm->log("LCM", Log::ERROR, "VM disk snapshot operation failed."); break; @@ -2191,7 +2188,6 @@ void LifeCycleManager::trigger_disk_snapshot_failure(int vid) break; case VirtualMachine::DISK_SNAPSHOT_SUSPENDED: - case VirtualMachine::DISK_SNAPSHOT_REVERT_SUSPENDED: case VirtualMachine::DISK_SNAPSHOT_DELETE_SUSPENDED: dm->trigger_suspend_success(vid); break; diff --git a/src/rm/RequestManagerChown.cc b/src/rm/RequestManagerChown.cc index 3b346e6404..6e1100733d 100644 --- a/src/rm/RequestManagerChown.cc +++ b/src/rm/RequestManagerChown.cc @@ -94,7 +94,7 @@ unique_ptr RequestManagerChown::get_and_quota( auto tmpl = make_unique