B #5096: Check only UpdateConf attributes on update

include/Template.h

@@ -457,6 +457,12 @@ public:
         return false;
     }
 
+    virtual bool test_restricted_merge(string& rs_attr,
+            const Template* base) const
+    {
+        return false;
+    }
+
     /**
      *  Encrypt all secret attributes
      */
@@ -541,6 +547,8 @@ protected:
     bool check_restricted(string& rs_attr,
            const std::map<std::string, std::set<std::string> >& ras);
 
+    bool test_restricted_merge(string& rs_attr, const Template* base,
+           const std::map<std::string, std::set<std::string> >& ras) const;
     /**
      *  Parses a list of encrypted attributes in the form ATTRIBUTE_NAME or
      *  ATTRIBUTE_NAME/SUBATTRIBUTE.

include/VirtualMachine.h

@@ -1200,6 +1200,11 @@ public:
      */
     VirtualMachineTemplate * get_updateconf_template() const;
 
+    bool test_restricted_merge(string& ra, const VirtualMachineTemplate * source) const
+    {
+        return source->test_restricted_merge(ra, obj_template);
+    }
+
     // -------------------------------------------------------------------------
     // "Save as" Disk related functions (save_as hot)
     // -------------------------------------------------------------------------

include/VirtualMachineTemplate.h

@@ -20,6 +20,7 @@
 #include "Template.h"
 
 #include <string>
+#include <memory>
 
 using namespace std;
 
@@ -56,17 +57,6 @@ public:
         Template::set_xml_root(_xml_root);
     };
 
-    /**
-     * Replaces the given image from the DISK attribute with a new one
-     *   @param target_id IMAGE_ID the image to be replaced
-     *   @param target_name IMAGE the image to be replaced
-     *   @param target_uname IMAGE_UNAME the image to be replaced
-     *   @param new_name of the new image
-     *   @param new_uname of the owner of the new image
-     */
-    int replace_disk_image(int target_id, const string&
-        target_name, const string& target_uname, const string& new_name,
-        const string& new_uname);
 
     // -------------------------------------------------------------------------
     // Restricted attributes interface implementation
@@ -76,6 +66,11 @@ public:
         return Template::check_restricted(rs_attr, base, restricted);
     }
 
+    virtual bool test_restricted_merge(string& rs_attr, const Template* base) const override
+    {
+        return Template::test_restricted_merge(rs_attr, base, restricted);
+    }
+
     virtual bool check_restricted(string& rs_attr)
     {
         return Template::check_restricted(rs_attr, restricted);
@@ -86,6 +81,22 @@ public:
         Template::parse_restricted(ra, restricted);
     }
 
+    /**
+     *  Get restricted attributes for NIC
+     */
+    static void restricted_nic(std::set<std::string>& rs)
+    {
+        get_restricted("NIC", rs);
+    }
+
+    /**
+     *  Get restricted attributes for DISK
+     */
+    static void restricted_disk(std::set<std::string>& rs)
+    {
+        get_restricted("DISK", rs);
+    }
+
     // -------------------------------------------------------------------------
     // Encrypted attributes interface implementation
     // -------------------------------------------------------------------------
@@ -106,21 +117,31 @@ public:
 
     string& to_xml_short(string& xml) const;
 
-    /**
-     *  Get restricted attributes for NIC
-     */
-    static void restricted_nic(std::set<std::string>& rs)
-    {
-        get_restricted("NIC", rs);
-    }
 
     /**
-     *  Get restricted attributes for DISK
+     * Replaces the given image from the DISK attribute with a new one
+     *   @param target_id IMAGE_ID the image to be replaced
+     *   @param target_name IMAGE the image to be replaced
+     *   @param target_uname IMAGE_UNAME the image to be replaced
+     *   @param new_name of the new image
+     *   @param new_uname of the owner of the new image
      */
-    static void restricted_disk(std::set<std::string>& rs)
-    {
-        get_restricted("DISK", rs);
-    }
+    int replace_disk_image(int target_id, const string&
+        target_name, const string& target_uname, const string& new_name,
+        const string& new_uname);
+
+    // -----------------------------------------------------------------------
+    // UpdateConf attributes
+    // -----------------------------------------------------------------------
+    static std::map<std::string,std::vector<std::string>> UPDATECONF_ATTRS;
+
+    /**
+    *  Returns a new template that contains only the attribues vaild in an
+    *  update conf operation
+    *
+    *  @return pointer to new VM template
+    */
+    std::unique_ptr<VirtualMachineTemplate> get_updateconf_template() const;
 
 private:
     /**

src/rm/RequestManagerVirtualMachine.cc

@@ -3155,6 +3155,8 @@ void VirtualMachineUpdateConf::request_execute(
         return;
     }
 
+    auto uc_tmpl = tmpl.get_updateconf_template();
+
     /* ---------------------------------------------------------------------- */
     /*  Authorize the operation & restricted attributes                       */
     /* ---------------------------------------------------------------------- */
@@ -3179,11 +3181,7 @@ void VirtualMachineUpdateConf::request_execute(
     {
         string aname;
 
-        VirtualMachineTemplate * conf_tmpl = vm->get_updateconf_template();
-
-        bool has_restricted = tmpl.check_restricted(aname, conf_tmpl);
-
-        delete conf_tmpl;
+        bool has_restricted = vm->test_restricted_merge(aname, uc_tmpl.get());
 
         if (has_restricted)
         {

src/template/Template.cc

@@ -356,7 +356,7 @@ bool Template::get(const string& name, string& value) const
 
     value = s->value();
 
-	return true;
+    return true;
 }
 
 /* -------------------------------------------------------------------------- */
@@ -865,6 +865,48 @@ bool Template::check_restricted(string& ra, const Template* base,
     return false;
 }
 
+// -----------------------------------------------------------------------------
+
+bool Template::test_restricted_merge(string& ra, const Template* base,
+        const std::map<std::string, std::set<std::string> >& ras) const
+{
+    std::map<std::string, std::set<std::string> >::const_iterator rit;
+
+    for ( rit = ras.begin(); rit != ras.end(); ++rit )
+    {
+        if (!(rit->second).empty())
+        {
+            vector<string> rvalues, rvalues_base;
+            bool has_restricted;
+
+            has_restricted = restricted_values(rit->first, rit->second, this, rvalues);
+            restricted_values(rit->first, rit->second, base, rvalues_base);
+
+            if ( rvalues != rvalues_base && has_restricted)
+            {
+                ra = rit->first;
+                return true;
+            }
+        }
+        else
+        {
+            if ( get(rit->first, ra) )
+            {
+                string ra_b;
+
+                base->get(rit->first, ra_b);
+
+                if ( ra_b != ra )
+                {
+                    ra = rit->first;
+                    return true;
+                }
+            }
+        }
+    }
+
+    return false;
+}
 /* -------------------------------------------------------------------------- */
 /* -------------------------------------------------------------------------- */
 

src/vm/VirtualMachineTemplate.cc

@@ -183,3 +183,117 @@ string& VirtualMachineTemplate::to_xml_short(string& xml) const
     return xml;
 }
 
+// -----------------------------------------------------------------------------
+// -----------------------------------------------------------------------------
+
+std::map<std::string,std::vector<std::string>> VirtualMachineTemplate::UPDATECONF_ATTRS = {
+  { "OS",
+      { "ARCH",
+        "MACHINE",
+        "KERNEL",
+        "INITRD",
+        "BOOTLOADER",
+        "BOOT",
+        "KERNEL_CMD",
+        "ROOT",
+        "SD_DISK_BUS",
+        "UUID"}
+  },
+  { "FEATURES",
+      { "PAE",
+        "ACPI",
+        "APIC",
+        "LOCALTIME",
+        "HYPERV",
+        "GUEST_AGENT",
+        "VIRTIO_SCSI_QUEUES",
+        "IOTHREADS"}
+  },
+  { "INPUT",
+      { "TYPE",
+        "BUS"}
+  },
+  {"GRAPHICS",
+      { "TYPE",
+        "LISTEN",
+        "PASSWD",
+        "KEYMAP",
+        "COMMAND"}
+  },
+  {"RAW",
+      { "TYPE",
+        "DATA",
+        "DATA_VMX"}
+  },
+  {"CPU_MODEL",
+      { "MODEL" }
+  }
+};
+
+// -----------------------------------------------------------------------------
+/**
+* returns a copy the values of a vector value
+*/
+static void copy_vector_values(const Template *old_tmpl, Template *new_tmpl,
+      const char * name)
+{
+  string value;
+
+  const VectorAttribute * old_attr = old_tmpl->get(name);
+
+  if ( old_attr == 0 )
+  {
+      return;
+  }
+
+  VectorAttribute * new_vattr = new VectorAttribute(name);
+
+  std::vector<std::string> vnames = VirtualMachineTemplate::UPDATECONF_ATTRS[name];
+
+  for (const auto& vname : vnames)
+  {
+      std::string vval = old_attr->vector_value(vname);
+
+      if (!vval.empty())
+      {
+          new_vattr->replace(vname, vval);
+      }
+  }
+
+  if ( new_vattr->empty() )
+  {
+      delete new_vattr;
+  }
+  else
+  {
+      new_tmpl->set(new_vattr);
+  }
+}
+
+// -----------------------------------------------------------------------------
+
+unique_ptr<VirtualMachineTemplate> VirtualMachineTemplate::get_updateconf_template() const
+{
+  auto conf_tmpl = make_unique<VirtualMachineTemplate>();
+
+  copy_vector_values(this, conf_tmpl.get(), "OS");
+
+  copy_vector_values(this, conf_tmpl.get(), "FEATURES");
+
+  copy_vector_values(this, conf_tmpl.get(), "INPUT");
+
+  copy_vector_values(this, conf_tmpl.get(), "GRAPHICS");
+
+  copy_vector_values(this, conf_tmpl.get(), "RAW");
+
+  copy_vector_values(this, conf_tmpl.get(), "CPU_MODEL");
+
+  const VectorAttribute * context = get("CONTEXT");
+
+  if ( context != 0 )
+  {
+      conf_tmpl->set(context->clone());
+  }
+
+  return conf_tmpl;
+}