From d2aaf2c8823d4172b1be673f959c4c922266534f Mon Sep 17 00:00:00 2001 From: Pavel Czerny Date: Mon, 11 Mar 2024 17:46:21 +0100 Subject: [PATCH] B #6503: Fix disk size after disk-snapshot-revert When a disk with snapshots is resized and a subsequent revert operation is performed, the following actions are now done: * DISK/SIZE is updated to reflect the active snapshot size * Quotas are updated to account the space freed after the resize operation (cherry picked from commit 178142105dea30c7deffd1842d0f0ad23d7dc93b) --- include/Image.h | 10 ++-- include/VirtualMachine.h | 29 ---------- include/VirtualMachineDisk.h | 21 +++++-- src/image/Image.cc | 16 ++++++ src/image/ImageManagerProtocol.cc | 9 ++- src/lcm/LifeCycleStates.cc | 70 ++++++++++++----------- src/rm/RequestManagerVirtualMachine.cc | 5 +- src/vm/VirtualMachineDisk.cc | 77 ++++++++++++++------------ 8 files changed, 129 insertions(+), 108 deletions(-) diff --git a/include/Image.h b/include/Image.h index dd5ef214eb3..49d253bd478 100644 --- a/include/Image.h +++ b/include/Image.h @@ -583,15 +583,17 @@ class Image : public PoolObjectSQL snapshots.clear_disk_id(); }; + /** + * This function reverts the image active snapshot, adjust sizes and quotas + * if needed + */ + void revert_snapshot(int snap_id, Template& ds_quotas); + void delete_snapshot(int snap_id) { snapshots.delete_snapshot(snap_id); }; - void revert_snapshot(int snap_id) - { - snapshots.active_snapshot(snap_id, true); - }; void set_target_snapshot(int snap_id) { diff --git a/include/VirtualMachine.h b/include/VirtualMachine.h index c30d7784413..780e243474f 100644 --- a/include/VirtualMachine.h +++ b/include/VirtualMachine.h @@ -1542,35 +1542,6 @@ class VirtualMachine : public PoolObjectSQL return disks.create_snapshot(disk_id, name, error); } - /** - * Sets the snap_id as active, the VM will boot from it next time - * @param disk_id of the disk - * @param snap_id of the snapshot - * @param error if any - * @param revert true if the cause of changing the active snapshot - * is because a revert - * @return -1 if error - */ - int revert_disk_snapshot(int disk_id, int snap_id, bool revert) - { - return disks.revert_snapshot(disk_id, snap_id, revert); - } - - /** - * Deletes the snap_id from the list - * @param disk_id of the disk - * @param snap_id of the snapshot - * @param ds_quotas template with snapshot usage for the DS quotas - * @param vm_quotas template with snapshot usage for the VM quotas - * @param io delete ds quotas from image owners - * @param vo delete ds quotas from vm owners - */ - void delete_disk_snapshot(int disk_id, int snap_id, Template **ds_quotas, - Template **vm_quotas, bool& io, bool& vo) - { - disks.delete_snapshot(disk_id, snap_id, ds_quotas, vm_quotas, io, vo); - } - /** * Renames the snap_id from the list * @param disk_id of the disk diff --git a/include/VirtualMachineDisk.h b/include/VirtualMachineDisk.h index 28b1e3566b9..47497f049aa 100644 --- a/include/VirtualMachineDisk.h +++ b/include/VirtualMachineDisk.h @@ -291,7 +291,18 @@ class VirtualMachineDisk : public VirtualMachineAttribute * @param io delete ds quotas from image owners * @param vo delete ds quotas from vm owners */ - void delete_snapshot(int snap_id, Template **ds_quota, Template **vm_quota, + void revert_snapshot_quotas(int snap_id, Template& ds_quota, Template& vm_quota, + bool& io, bool& vo); + + /** + * Deletes the snap_id from the list + * @param snap_id of the snapshot + * @param ds_quotas template with snapshot usage for the DS quotas + * @param vm_quotas template with snapshot usage for the VM quotas + * @param io delete ds quotas from image owners + * @param vo delete ds quotas from vm owners + */ + void delete_snapshot(int snap_id, Template& ds_quota, Template& vm_quota, bool& io, bool& vo); /* ---------------------------------------------------------------------- */ @@ -305,14 +316,14 @@ class VirtualMachineDisk : public VirtualMachineAttribute /** * Calculate the quotas for a resize operation on the disk - * @param new_size of disk + * @param delta_size = new_size - old_size * @param dsdeltas increment in datastore usage * @param vmdelta increment in system datastore usage * @param do_img_owner quotas counter allocated for image uid/gid * @param do_vm_owner quotas counter allocated for vm uid/gid * */ - void resize_quotas(long long new_size, Template& dsdelta, Template& vmdelta, + void resize_quotas(long long delta_size, Template& dsdelta, Template& vmdelta, bool& do_img_owner, bool& do_vm_owner); /* ---------------------------------------------------------------------- */ @@ -771,8 +782,8 @@ class VirtualMachineDisks : public VirtualMachineAttributeSet * @param io delete ds quotas from image owners * @param vo delete ds quotas from vm owners */ - void delete_snapshot(int disk_id, int snap_id, Template **ds_quota, - Template **vm_quota, bool& io, bool& vo); + void delete_snapshot(int disk_id, int snap_id, Template& ds_quota, Template& vm_quota, + bool& io, bool& vo); /** * Renames a given snapshot diff --git a/src/image/Image.cc b/src/image/Image.cc index 3569d329f62..2ec45628870 100644 --- a/src/image/Image.cc +++ b/src/image/Image.cc @@ -1147,3 +1147,19 @@ bool Image::test_set_persistent(Template * image_template, int uid, int gid, /* -------------------------------------------------------------------------- */ /* -------------------------------------------------------------------------- */ + +void Image::revert_snapshot(int snap_id, Template& ds_quotas) +{ + snapshots.active_snapshot(snap_id, true); + + auto snap_size = snapshots.snapshot_size(snap_id); + + if (snap_size != get_size()) + { + ds_quotas.add("DATASTORE", get_ds_id()); + ds_quotas.add("SIZE", get_size() - snap_size); + ds_quotas.add("IMAGES", 0); + + set_size(snap_size); + } +} diff --git a/src/image/ImageManagerProtocol.cc b/src/image/ImageManagerProtocol.cc index 06ba6a76d8d..a301781db72 100644 --- a/src/image/ImageManagerProtocol.cc +++ b/src/image/ImageManagerProtocol.cc @@ -734,8 +734,13 @@ void ImageManager::_snap_revert(unique_ptr msg) return; } + int uid = image->get_uid(); + int gid = image->get_gid(); + int snap_id = image->get_target_snapshot(); + Template ds_quotas; + image->set_state_unlock(); if (snap_id == -1) @@ -749,7 +754,7 @@ void ImageManager::_snap_revert(unique_ptr msg) if (msg->status() == "SUCCESS") { - image->revert_snapshot(snap_id); + image->revert_snapshot(snap_id, ds_quotas); } else { @@ -772,6 +777,8 @@ void ImageManager::_snap_revert(unique_ptr msg) image->clear_target_snapshot(); ipool->update(image.get()); + + Quotas::ds_del(uid, gid, &ds_quotas); } /* -------------------------------------------------------------------------- */ diff --git a/src/lcm/LifeCycleStates.cc b/src/lcm/LifeCycleStates.cc index ea71681d86d..094b7cb2b27 100644 --- a/src/lcm/LifeCycleStates.cc +++ b/src/lcm/LifeCycleStates.cc @@ -1916,12 +1916,11 @@ void LifeCycleManager::trigger_disk_snapshot_success(int vid) int disk_id, ds_id, snap_id; int img_id = -1; - Template *ds_quotas = nullptr; - Template *vm_quotas = nullptr; + Template ds_quotas; + Template vm_quotas; bool img_owner, vm_owner; - const VirtualMachineDisk * disk; Snapshots snaps(-1, Snapshots::DENY); const Snapshots* tmp_snaps; string error_str; @@ -1934,6 +1933,8 @@ void LifeCycleManager::trigger_disk_snapshot_success(int vid) bool is_persistent; string target; + long long disk_size = -1; + if ( auto vm = vmpool->get(vid) ) { if (vm->get_snapshot_disk(ds_id, tm_mad, disk_id, snap_id) == -1) @@ -1946,6 +1947,8 @@ void LifeCycleManager::trigger_disk_snapshot_success(int vid) vm_uid = vm->get_uid(); vm_gid = vm->get_gid(); + auto disk = vm->get_disk(disk_id); + state = vm->get_lcm_state(); switch (state) @@ -1956,12 +1959,16 @@ void LifeCycleManager::trigger_disk_snapshot_success(int vid) case VirtualMachine::DISK_SNAPSHOT_POWEROFF: case VirtualMachine::DISK_SNAPSHOT_SUSPENDED: vm->log("LCM", Log::INFO, "VM disk snapshot operation completed."); - vm->revert_disk_snapshot(disk_id, snap_id, false); + disk->revert_snapshot(snap_id, false); break; case VirtualMachine::DISK_SNAPSHOT_REVERT_POWEROFF: vm->log("LCM", Log::INFO, "VM disk snapshot operation completed."); - vm->revert_disk_snapshot(disk_id, snap_id, true); + disk_size = disk->get_snapshot_size(snap_id); + + disk->revert_snapshot_quotas(snap_id, ds_quotas, vm_quotas, + img_owner, vm_owner); + disk->revert_snapshot(snap_id, true); break; case VirtualMachine::DISK_SNAPSHOT_DELETE: @@ -1970,7 +1977,7 @@ void LifeCycleManager::trigger_disk_snapshot_success(int vid) case VirtualMachine::DISK_SNAPSHOT_DELETE_POWEROFF: case VirtualMachine::DISK_SNAPSHOT_DELETE_SUSPENDED: vm->log("LCM", Log::INFO, "VM disk snapshot deleted."); - vm->delete_disk_snapshot(disk_id, snap_id, &ds_quotas, &vm_quotas, + disk->delete_snapshot(snap_id, ds_quotas, vm_quotas, img_owner, vm_owner); break; @@ -1982,13 +1989,12 @@ void LifeCycleManager::trigger_disk_snapshot_success(int vid) vm->clear_snapshot_disk(); tmp_snaps = vm->get_disk_snapshots(disk_id, error_str); + if (tmp_snaps != nullptr) { snaps = *tmp_snaps; } - disk = vm->get_disk(disk_id); - disk->vector_value("IMAGE_ID", img_id); is_persistent = disk->is_persistent(); @@ -2005,7 +2011,7 @@ void LifeCycleManager::trigger_disk_snapshot_success(int vid) return; } - if ( ds_quotas != nullptr ) + if ( !ds_quotas.empty() ) { if ( img_owner ) { @@ -2014,28 +2020,29 @@ void LifeCycleManager::trigger_disk_snapshot_success(int vid) int img_uid = img->get_uid(); int img_gid = img->get_gid(); - Quotas::ds_del(img_uid, img_gid, ds_quotas); + Quotas::ds_del(img_uid, img_gid, &ds_quotas); } } if ( vm_owner ) { - Quotas::ds_del(vm_uid, vm_gid, ds_quotas); + Quotas::ds_del(vm_uid, vm_gid, &ds_quotas); } - - delete ds_quotas; } - if ( vm_quotas != nullptr ) + if ( !vm_quotas.empty() ) { - Quotas::vm_del(vm_uid, vm_gid, vm_quotas); - - delete vm_quotas; + Quotas::vm_del(vm_uid, vm_gid, &vm_quotas); } // Update image if it is persistent and ln mode does not clone it if ( img_id != -1 && is_persistent && target == "NONE" ) { + if (disk_size >= 0) + { + imagem->set_image_size(img_id, disk_size); + } + imagem->set_image_snapshots(img_id, snaps); } @@ -2068,8 +2075,8 @@ void LifeCycleManager::trigger_disk_snapshot_failure(int vid) int disk_id, ds_id, snap_id; int img_id = -1; - Template *ds_quotas = nullptr; - Template *vm_quotas = nullptr; + Template ds_quotas; + Template vm_quotas; const VirtualMachineDisk* disk; Snapshots snaps(-1, Snapshots::DENY); @@ -2108,8 +2115,8 @@ void LifeCycleManager::trigger_disk_snapshot_failure(int vid) case VirtualMachine::DISK_SNAPSHOT_POWEROFF: case VirtualMachine::DISK_SNAPSHOT_SUSPENDED: vm->log("LCM", Log::ERROR, "Could not take disk snapshot."); - vm->delete_disk_snapshot(disk_id, snap_id, &ds_quotas, &vm_quotas, - img_owner, vm_owner); + vm->get_disks().delete_snapshot(disk_id, snap_id, ds_quotas, + vm_quotas, img_owner, vm_owner); break; case VirtualMachine::DISK_SNAPSHOT_DELETE: @@ -2129,6 +2136,7 @@ void LifeCycleManager::trigger_disk_snapshot_failure(int vid) vm->clear_snapshot_disk(); tmp_snaps = vm->get_disk_snapshots(disk_id, error_str); + if (tmp_snaps != nullptr) { snaps = *tmp_snaps; @@ -2153,7 +2161,7 @@ void LifeCycleManager::trigger_disk_snapshot_failure(int vid) return; } - if ( ds_quotas != nullptr ) + if ( !ds_quotas.empty() ) { if ( img_owner ) { @@ -2162,23 +2170,19 @@ void LifeCycleManager::trigger_disk_snapshot_failure(int vid) int img_uid = img->get_uid(); int img_gid = img->get_gid(); - Quotas::ds_del(img_uid, img_gid, ds_quotas); + Quotas::ds_del(img_uid, img_gid, &ds_quotas); } } if ( vm_owner) { - Quotas::ds_del(vm_uid, vm_gid, ds_quotas); + Quotas::ds_del(vm_uid, vm_gid, &ds_quotas); } - - delete ds_quotas; } - if ( vm_quotas != nullptr ) + if ( !vm_quotas.empty() ) { - Quotas::vm_del(vm_uid, vm_gid, vm_quotas); - - delete vm_quotas; + Quotas::vm_del(vm_uid, vm_gid, &vm_quotas); } // Update image if it is persistent and ln mode does not clone it @@ -2395,7 +2399,7 @@ void LifeCycleManager::trigger_disk_resize_failure(int vid) Template vm_deltas; int img_id = -1; - long long size_prev; + long long size_prev, size; VirtualMachine::LcmState state; @@ -2437,8 +2441,10 @@ void LifeCycleManager::trigger_disk_resize_failure(int vid) vm_gid = vm->get_gid(); disk->vector_value("IMAGE_ID", img_id); + disk->vector_value("SIZE", size); disk->vector_value("SIZE_PREV", size_prev); - disk->resize_quotas(size_prev, ds_deltas, vm_deltas, img_quota, vm_quota); + + disk->resize_quotas(size - size_prev, ds_deltas, vm_deltas, img_quota, vm_quota); disk->clear_resize(true); diff --git a/src/rm/RequestManagerVirtualMachine.cc b/src/rm/RequestManagerVirtualMachine.cc index 2aa2898234f..d31b67a47a2 100644 --- a/src/rm/RequestManagerVirtualMachine.cc +++ b/src/rm/RequestManagerVirtualMachine.cc @@ -3043,8 +3043,7 @@ Request::ErrorCode VirtualMachineDiskSnapshotCreate::request_execute( long long ssize; disk->vector_value("SIZE", ssize); - ssize = 2 * ssize; //Sanpshot accounts as another disk of same size - + // Snapshot accounts as another disk of same size disk->resize_quotas(ssize, ds_deltas, vm_deltas, img_ds_quota, vm_ds_quota); is_volatile = disk->is_volatile(); @@ -3666,7 +3665,7 @@ void VirtualMachineDiskResize::request_execute( } /* ------------- Get information about the disk and image --------------- */ - disk->resize_quotas(size, ds_deltas, vm_deltas, img_ds_quota, vm_ds_quota); + disk->resize_quotas(size - current_size, ds_deltas, vm_deltas, img_ds_quota, vm_ds_quota); disk->vector_value("IMAGE_ID", img_id); diff --git a/src/vm/VirtualMachineDisk.cc b/src/vm/VirtualMachineDisk.cc index 8d7e20e7b9d..84b77cab215 100644 --- a/src/vm/VirtualMachineDisk.cc +++ b/src/vm/VirtualMachineDisk.cc @@ -310,6 +310,33 @@ int VirtualMachineDisk::revert_snapshot(int snap_id, bool revert) return snapshots->active_snapshot(snap_id, revert); } +/* -------------------------------------------------------------------------- */ +/* -------------------------------------------------------------------------- */ +void VirtualMachineDisk::revert_snapshot_quotas(int snap_id, Template& ds_quota, + Template& vm_quota, bool& io, bool& vo) +{ + if ( snapshots == 0 ) + { + return; + } + + auto snap_size = snapshots->snapshot_size(snap_id); + long long disk_size = 0; + + if ( vector_value("SIZE", disk_size) != 0 ) + { + return; + } + + // Update disk SIZE and quotas so the delta size is reversed + if ( disk_size != snap_size) + { + resize_quotas(disk_size - snap_size, ds_quota, vm_quota, io, vo); + + replace("SIZE", snap_size); + } +} + /* -------------------------------------------------------------------------- */ /* -------------------------------------------------------------------------- */ // +--------+-------------------------------------+ @@ -323,8 +350,8 @@ int VirtualMachineDisk::revert_snapshot(int snap_id, bool revert) // +----------------------------------------------+ /* -------------------------------------------------------------------------- */ -void VirtualMachineDisk::delete_snapshot(int snap_id, Template **ds_quotas, - Template **vm_quotas, bool& img_owner, bool& vm_owner) +void VirtualMachineDisk::delete_snapshot(int snap_id, Template& ds_quotas, + Template& vm_quotas, bool& img_owner, bool& vm_owner) { vm_owner = false; img_owner = false; @@ -347,24 +374,21 @@ void VirtualMachineDisk::delete_snapshot(int snap_id, Template **ds_quotas, vm_owner = tm_target == "SELF"; img_owner = is_persistent() || tm_target == "NONE"; - if ( img_owner || vm_owner ) - { - *ds_quotas = new Template(); - - (*ds_quotas)->add("DATASTORE", vector_value("DATASTORE_ID")); - (*ds_quotas)->add("SIZE", ssize); - (*ds_quotas)->add("IMAGES",0 ); - } + if ( img_owner || vm_owner ) + { + ds_quotas.add("DATASTORE", vector_value("DATASTORE_ID")); + ds_quotas.add("SIZE", ssize); + ds_quotas.add("IMAGES",0 ); + } if (tm_target == "SYSTEM") { - *vm_quotas = new Template(); - VectorAttribute * delta_disk = new VectorAttribute("DISK"); + delta_disk->replace("TYPE", "FS"); delta_disk->replace("SIZE", ssize); - (*vm_quotas)->set(delta_disk); + vm_quotas.set(delta_disk); } } @@ -436,27 +460,12 @@ long long VirtualMachineDisk::image_ds_size() const // | NONE | image | IMG | image | IMG | // +----------------------------------------------+ /* -------------------------------------------------------------------------- */ -void VirtualMachineDisk::resize_quotas(long long new_size, Template& ds_deltas, +void VirtualMachineDisk::resize_quotas(long long delta_size, Template& ds_deltas, Template& vm_deltas, bool& do_img_owner, bool& do_vm_owner) { - long long current_size, delta_size; - do_vm_owner = false; do_img_owner= false; - if ( vector_value("SIZE", current_size) != 0 ) - { - return; - } - - delta_size = new_size - current_size; - - //Quotas uses del operation to substract counters, delta needs to be > 0 - if ( delta_size < 0 ) - { - delta_size = - delta_size; - } - string tm = get_tm_target(); do_vm_owner = !is_volatile() && tm == "SELF"; do_img_owner = !is_volatile() && (is_persistent() || tm == "NONE"); @@ -619,7 +628,7 @@ long long VirtualMachineDisks::system_ds_size(bool include_snapshots) for ( disk_iterator disk = begin() ; disk != end() ; ++disk ) { - size += (*disk)->system_ds_size(include_snapshots); + size += (*disk)->system_ds_size(include_snapshots); } return size; @@ -1411,15 +1420,15 @@ int VirtualMachineDisks::revert_snapshot(int id, int snap_id, bool revert) /* -------------------------------------------------------------------------- */ /* -------------------------------------------------------------------------- */ +/* -------------------------------------------------------------------------- */ +/* -------------------------------------------------------------------------- */ + void VirtualMachineDisks::delete_snapshot(int disk_id, int snap_id, - Template **ds_quota, Template **vm_quota,bool& img_owner, bool& vm_owner) + Template& ds_quota, Template& vm_quota, bool& img_owner, bool& vm_owner) { VirtualMachineDisk * disk = static_cast(get_attribute(disk_id)); - *ds_quota = 0; - *vm_quota = 0; - if ( disk == 0 ) { return;