From c68c81a86e3ed363cfbb3ea09f116c28e399431d Mon Sep 17 00:00:00 2001 From: "Ruben S. Montero" Date: Thu, 9 Feb 2023 19:00:07 +0100 Subject: [PATCH] F #6029: Prevent backup removal VM is in BACKUP Some backup drivers may not allow delete (forget) operations while an active backup is being performed. This commits fails the one.image.delete API call in that case. A new configuration attribute to describe the related driver behavior has been added CONCURRENT_FORGET for DS_MAD_CONF. (cherry picked from commit 940c1d7d637141c75226b1545f7cda3fd27feee8) --- include/Datastore.h | 9 +++- src/datastore/Datastore.cc | 79 ++++++++++++++++++++------------ src/image/ImageManagerActions.cc | 32 +++++++++++++ 3 files changed, 88 insertions(+), 32 deletions(-) diff --git a/include/Datastore.h b/include/Datastore.h index 37a56782ad8..6205ad24322 100644 --- a/include/Datastore.h +++ b/include/Datastore.h @@ -240,11 +240,16 @@ class Datastore : public PoolObjectSQL, public Clusterable }; /** - * Returns true if the DS_MAD_CONF has PERSISTENT_ONLY = "YES" flag - * @return true if persistent only + * @return true if the DS_MAD_CONF has PERSISTENT_ONLY = "YES" flag */ bool is_persistent_only() const; + /** + * (only relevant for backup datastores) + * @return true if the DS_MAD_CONF has CONCURRENT_FORGET = "YES" flag + */ + bool is_concurrent_forget() const; + /** * Enable or disable the DS. Only for System DS. * @param enable true to enable diff --git a/src/datastore/Datastore.cc b/src/datastore/Datastore.cc index 320255c8a25..5bccf607eb8 100644 --- a/src/datastore/Datastore.cc +++ b/src/datastore/Datastore.cc @@ -1110,58 +1110,77 @@ bool Datastore::get_avail_mb(long long &avail) const /* ------------------------------------------------------------------------ */ /* ------------------------------------------------------------------------ */ -bool Datastore::is_persistent_only() const +template +static T ds_conf_value(const string& mad, const string& name, const T& defval) { - int rc; - bool persistent_only = false; - const VectorAttribute* vatt; - rc = Nebula::instance().get_ds_conf_attribute(ds_mad, vatt); + int rc = Nebula::instance().get_ds_conf_attribute(mad, vatt); if ( rc != 0 ) { // No DS_MAD_CONF is available for this DS_MAD. - // Assuming this DS is not PERSISTENT_ONLY - return false; + return defval; } - vatt->vector_value("PERSISTENT_ONLY", persistent_only); + T value; - return persistent_only; -}; + rc = vatt->vector_value(name, value); -/* ------------------------------------------------------------------------ */ -/* ------------------------------------------------------------------------ */ + if ( rc != 0 ) + { + // Attribute missing in DS_MAD_CONF + return defval; + } + + return value; +} + +/* -------------------------------------------------------------------------- */ + +bool Datastore::is_persistent_only() const +{ + return ds_conf_value(ds_mad, "PERSISTENT_ONLY", false); +} + +bool Datastore::is_concurrent_forget() const +{ + return ds_conf_value(ds_mad, "CONCURRENT_FORGET", false); +} + +/* -------------------------------------------------------------------------- */ +/* -------------------------------------------------------------------------- */ int Datastore::get_tm_mad_targets(const string &tm_mad, string& ln_target, string& clone_target, string& disk_type) const { - if (!tm_mad.empty()) + if (tm_mad.empty()) { - string tm_mad_t = one_util::trim(tm_mad); - one_util::toupper(tm_mad_t); + return 0; + } - get_template_attribute("CLONE_TARGET_" + tm_mad_t, clone_target); + string tm_mad_t = one_util::trim(tm_mad); + one_util::toupper(tm_mad_t); - if (clone_target.empty()) - { - return -1; - } + get_template_attribute("CLONE_TARGET_" + tm_mad_t, clone_target); - get_template_attribute("LN_TARGET_" + tm_mad_t, ln_target); + if (clone_target.empty()) + { + return -1; + } - if (ln_target.empty()) - { - return -1; - } + get_template_attribute("LN_TARGET_" + tm_mad_t, ln_target); - get_template_attribute("DISK_TYPE_" + tm_mad_t, disk_type); + if (ln_target.empty()) + { + return -1; + } - if (disk_type.empty()) - { - return -1; - } + get_template_attribute("DISK_TYPE_" + tm_mad_t, disk_type); + + if (disk_type.empty()) + { + return -1; } return 0; diff --git a/src/image/ImageManagerActions.cc b/src/image/ImageManagerActions.cc index c641fcf2b8b..fe13aeb6b82 100644 --- a/src/image/ImageManagerActions.cc +++ b/src/image/ImageManagerActions.cc @@ -462,6 +462,8 @@ int ImageManager::delete_image(int iid, string& error_str) int cloning_id = -1; int vm_saving_id = -1; + bool cforget; + ostringstream oss; if (auto img = ipool->get_ro(iid)) @@ -479,6 +481,8 @@ int ImageManager::delete_image(int iid, string& error_str) ds->decrypt(); ds->to_xml(ds_data); + + cforget = ds->is_concurrent_forget(); } else { @@ -494,6 +498,34 @@ int ImageManager::delete_image(int iid, string& error_str) return -1; } + // Check associted VM state for backups. Note VM can no longer exist + // Fai if VM is in BACKUP and INCREMENT mode or FULL with no CONCURRENT_FORGET + if ( img->get_type() == Image::BACKUP ) + { + auto ids = img->get_running_ids(); + auto first = ids.cbegin(); + + if (first != ids.cend()) + { + VirtualMachinePool* vmpool = Nebula::instance().get_vmpool(); + + if (auto vm = vmpool->get_ro(*first)) + { + auto lstate = vm->get_lcm_state(); + auto bmode = vm->backups().mode(); + + if ((lstate == VirtualMachine::BACKUP || + lstate == VirtualMachine::BACKUP_POWEROFF) && + (bmode == Backups::INCREMENT || !cforget)) + { + error_str = "Active backup on the associated VM. Wait till " + "it finish to delete the backup Image"; + return -1; + } + } + } + } + switch (img->get_state()) { case Image::READY: