Skip to content

Commit

Permalink
B #6503: Fix disk size after disk-snapshot-revert
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@paczerny @rsmontero
paczerny authored and rsmontero committed Mar 11, 2024
1 parent bcb3e7d commit 1781421
Show file tree
Hide file tree
Showing 8 changed files with 129 additions and 108 deletions.
10 changes: 6 additions & 4 deletions include/Image.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
29 changes: 0 additions & 29 deletions include/VirtualMachine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1557,35 +1557,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
Expand Down
21 changes: 16 additions & 5 deletions include/VirtualMachineDisk.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

/* ---------------------------------------------------------------------- */
Expand All @@ -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);

/* ---------------------------------------------------------------------- */
Expand Down Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions src/image/Image.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
9 changes: 8 additions & 1 deletion src/image/ImageManagerProtocol.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -734,8 +734,13 @@ void ImageManager::_snap_revert(unique_ptr<image_msg_t> 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)
Expand All @@ -749,7 +754,7 @@ void ImageManager::_snap_revert(unique_ptr<image_msg_t> msg)

if (msg->status() == "SUCCESS")
{
image->revert_snapshot(snap_id);
image->revert_snapshot(snap_id, ds_quotas);
}
else
{
Expand All @@ -772,6 +777,8 @@ void ImageManager::_snap_revert(unique_ptr<image_msg_t> msg)
image->clear_target_snapshot();

ipool->update(image.get());

Quotas::ds_del(uid, gid, &ds_quotas);
}

/* -------------------------------------------------------------------------- */
Expand Down
70 changes: 38 additions & 32 deletions src/lcm/LifeCycleStates.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1915,12 +1915,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;
Expand All @@ -1933,6 +1932,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)
Expand All @@ -1945,6 +1946,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)
Expand All @@ -1955,12 +1958,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:
Expand All @@ -1969,7 +1976,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;

Expand All @@ -1981,13 +1988,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();
Expand All @@ -2004,7 +2010,7 @@ void LifeCycleManager::trigger_disk_snapshot_success(int vid)
return;
}

if ( ds_quotas != nullptr )
if ( !ds_quotas.empty() )
{
if ( img_owner )
{
Expand All @@ -2013,28 +2019,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);
}

Expand Down Expand Up @@ -2067,8 +2074,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);
Expand Down Expand Up @@ -2107,8 +2114,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:
Expand All @@ -2128,6 +2135,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;
Expand All @@ -2152,7 +2160,7 @@ void LifeCycleManager::trigger_disk_snapshot_failure(int vid)
return;
}

if ( ds_quotas != nullptr )
if ( !ds_quotas.empty() )
{
if ( img_owner )
{
Expand All @@ -2161,23 +2169,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
Expand Down Expand Up @@ -2394,7 +2398,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;

Expand Down Expand Up @@ -2436,8 +2440,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);

Expand Down
5 changes: 2 additions & 3 deletions src/rm/RequestManagerVirtualMachine.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3038,8 +3038,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();
Expand Down Expand Up @@ -3661,7 +3660,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);

Expand Down

0 comments on commit 1781421

Please sign in to comment.