Bug #3620: Move previous state managment to VM class for hook management

include/VirtualMachine.h

@@ -958,6 +958,11 @@ public:
         return state;
     };
 
+    VmState get_prev_state() const
+    {
+        return prev_state;
+    };
+
     /**
      *  Returns the VM state (life-cycle Manager)
      *    @return the VM state
@@ -967,6 +972,11 @@ public:
         return lcm_state;
     };
 
+    LcmState get_prev_lcm_state() const
+    {
+        return prev_lcm_state;
+    };
+
     /**
      *  Sets VM state
      *    @param s state
@@ -985,6 +995,24 @@ public:
         lcm_state = s;
     };
 
+    /**
+     *  Sets the previous state to the current one
+     */
+    void set_prev_state()
+    {
+        prev_state     = state;
+        prev_lcm_state = lcm_state;
+    };
+
+    /**
+     *  Test if the VM has changed state since last time prev state was set
+     *    @return true if VM changed state
+     */
+    bool has_changed_state()
+    {
+        return (prev_lcm_state != lcm_state || prev_state != state);
+    }
+
     /**
      *  Sets the re-scheduling flag
      *    @param set or unset the re-schedule flag
@@ -1418,11 +1446,21 @@ private:
      */
     VmState     state;
 
+    /**
+     *  Previous state og the virtual machine, to trigger state hooks
+     */
+    VmState     prev_state;
+
     /**
      *  The state of the virtual machine (in the Life-cycle Manager).
      */
     LcmState    lcm_state;
 
+    /**
+     *  Previous state og the virtual machine, to trigger state hooks
+     */
+    LcmState    prev_lcm_state;
+
     /**
      *  Marks the VM as to be re-scheduled
      */

include/VirtualMachineHook.h

@@ -25,72 +25,12 @@
 
 using namespace std;
 
-/**
- *  This class provides basic functionality to store VM states for state Hooks.
- *  The state Map is shared by all the State hooks. A maintenance hook that
- *  updates the map should be added.
- */
-class VirtualMachineStateMapHook: public Hook
-{
-public:
-    virtual void do_hook(void *arg) = 0;
-
-protected:
-    // -------------------------------------------------------------------------
-    // Init the Map
-    // -------------------------------------------------------------------------
-    VirtualMachineStateMapHook(const string& name,
-                           const string& cmd,
-                           const string& args,
-                           bool          remote):
-        Hook(name, cmd, args, Hook::UPDATE | Hook::ALLOCATE, remote){};
-
-    virtual ~VirtualMachineStateMapHook(){};
-
-    // -------------------------------------------------------------------------
-    // Functions to handle the VM state map
-    // -------------------------------------------------------------------------
-    /**
-     *  Gets the state associated to the VM
-     *    @param id of the VM
-     *    @param lcm_state (previous) of the VM
-     *    @return 0 if the previous state for the VM has been recorded
-     */
-    int get_state(int id,
-                  VirtualMachine::LcmState &lcm_state,
-                  VirtualMachine::VmState  &vm_state);
-
-    /**
-     *  Updates the state associated to the VM
-     *    @param id of the VM
-     *    @param lcm_state (current) of the VM
-     */
-    void update_state (int                      id,
-                       VirtualMachine::LcmState lcm_state,
-                       VirtualMachine::VmState  vm_state);
-private:
-
-    struct VmStates
-    {
-        VmStates(VirtualMachine::LcmState _lcm, VirtualMachine::VmState _vm):
-            lcm(_lcm), vm(_vm){};
-
-        VirtualMachine::LcmState lcm;
-        VirtualMachine::VmState  vm;
-    };
-
-    /**
-     *  The state Map for the VMs
-     */
-    static map<int,VmStates> vm_states;
-};
-
 /**
  *  This class is a general VM State Hook that executes a command locally or
  *  remotelly when the VM gets into a given state (one shot). The VirtualMachine
  *  object is looked when the hook is invoked.
  */
-class VirtualMachineStateHook : public VirtualMachineStateMapHook
+class VirtualMachineStateHook : public Hook
 {
 public:
     // -------------------------------------------------------------------------
@@ -109,7 +49,9 @@ public:
                             bool                     remote,
                             VirtualMachine::LcmState _lcm,
                             VirtualMachine::VmState  _vm):
-        VirtualMachineStateMapHook(name,cmd,args,remote), lcm(_lcm), vm(_vm){};
+        Hook(name, cmd, args, Hook::UPDATE | Hook::ALLOCATE, remote), 
+        lcm(_lcm), 
+        vm(_vm){};
 
     ~VirtualMachineStateHook(){};
 
@@ -140,24 +82,4 @@ private:
     VirtualMachine::VmState  vm;
 };
 
-/**
- *  This class implements a state Map updater, one hook of this type should be
- *  added in order to mantain the VM state map.
- */
-class VirtualMachineUpdateStateHook : public VirtualMachineStateMapHook
-{
-public:
-    // -------------------------------------------------------------------------
-    // -------------------------------------------------------------------------
-    VirtualMachineUpdateStateHook():
-        VirtualMachineStateMapHook("","","",false){};
-
-    ~VirtualMachineUpdateStateHook(){};
-
-    // -------------------------------------------------------------------------
-    // Hook methods
-    // -------------------------------------------------------------------------
-    void do_hook(void *arg);
-};
-
 #endif

include/VirtualMachinePool.h

@@ -104,6 +104,23 @@ public:
         return static_cast<VirtualMachine *>(PoolSQL::get(oid,lock));
     };
 
+    /**
+     *  Updates a VM in the data base. The VM SHOULD be locked. It also updates
+     *  the previous state after executing the hooks.
+     *    @param objsql a pointer to the VM
+     *
+     *    @return 0 on success.
+     */
+    virtual int update(
+        VirtualMachine * objsql)
+    {
+        do_hooks(objsql, Hook::UPDATE);
+        
+        objsql->set_prev_state();
+
+        return objsql->update(db); 
+    };
+
     /**
      *  Gets a VM ID by its deploy_id, the dedploy_id - VM id mapping is keep
      *  in the import_table.

src/vm/VirtualMachine.cc

@@ -49,7 +49,9 @@ VirtualMachine::VirtualMachine(int           id,
         PoolObjectSQL(id,VM,"",_uid,_gid,_uname,_gname,table),
         last_poll(0),
         state(INIT),
+        prev_state(INIT),
         lcm_state(LCM_INIT),
+        prev_lcm_state(LCM_INIT),
         resched(0),
         stime(time(0)),
         etime(0),
@@ -3678,6 +3680,8 @@ string& VirtualMachine::to_xml_extended(string& xml, int n_history) const
         << "<LAST_POLL>" << last_poll << "</LAST_POLL>"
         << "<STATE>"     << state     << "</STATE>"
         << "<LCM_STATE>" << lcm_state << "</LCM_STATE>"
+        << "<PREV_STATE>"     << prev_state     << "</PREV_STATE>"
+        << "<PREV_LCM_STATE>" << prev_lcm_state << "</PREV_LCM_STATE>"
         << "<RESCHED>"   << resched   << "</RESCHED>"
         << "<STIME>"     << stime     << "</STIME>"
         << "<ETIME>"     << etime     << "</ETIME>"
@@ -3744,8 +3748,6 @@ int VirtualMachine::from_xml(const string &xml_str)
     rc += xpath(name,      "/VM/NAME",  "not_found");
 
     rc += xpath(last_poll, "/VM/LAST_POLL", 0);
-    rc += xpath(istate,    "/VM/STATE",     0);
-    rc += xpath(ilcmstate, "/VM/LCM_STATE", 0);
     rc += xpath(resched,   "/VM/RESCHED",   0);
 
     rc += xpath(stime,     "/VM/STIME",    0);
@@ -3760,9 +3762,19 @@ int VirtualMachine::from_xml(const string &xml_str)
     // Permissions
     rc += perms_from_xml();
 
+    //VM states
+    rc += xpath(istate,    "/VM/STATE",     0);
+    rc += xpath(ilcmstate, "/VM/LCM_STATE", 0);
+
     state     = static_cast<VmState>(istate);
     lcm_state = static_cast<LcmState>(ilcmstate);
 
+    rc += xpath(istate,    "/VM/PREV_STATE",     0);
+    rc += xpath(ilcmstate, "/VM/PREV_LCM_STATE", 0);
+
+    prev_state     = static_cast<VmState>(istate);
+    prev_lcm_state = static_cast<LcmState>(ilcmstate);
+
     // Virtual Machine template
     ObjectXML::get_nodes("/VM/TEMPLATE", content);
 

src/vm/VirtualMachineHook.cc

@@ -21,73 +21,12 @@
 // -----------------------------------------------------------------------------
 // -----------------------------------------------------------------------------
 
-map<int,VirtualMachineStateMapHook::VmStates>
-                                     VirtualMachineStateMapHook::vm_states;
-
-// -----------------------------------------------------------------------------
-
-int VirtualMachineStateMapHook::get_state(int id,
-        VirtualMachine::LcmState &lcm_state,
-        VirtualMachine::VmState  &vm_state)
-{
-    map<int,VmStates>::iterator it;
-
-    it = vm_states.find(id);
-
-    if ( it == vm_states.end() )
-    {
-        return -1;
-    }
-
-    lcm_state = it->second.lcm;
-    vm_state  = it->second.vm;
-
-    return 0;
-}
-
-// -----------------------------------------------------------------------------
-// -----------------------------------------------------------------------------
-
-void VirtualMachineStateMapHook::update_state (int id,
-        VirtualMachine::LcmState lcm_state,
-        VirtualMachine::VmState  vm_state)
-{
-    map<int,VmStates>::iterator it;
-
-    it = vm_states.find(id);
-
-    if ( it == vm_states.end() )
-    {
-        VmStates states(lcm_state, vm_state);
-
-        vm_states.insert(make_pair(id,states));
-    }
-    else
-    {
-        if ( vm_state == VirtualMachine::DONE )
-        {
-            vm_states.erase(it);
-        }
-        else
-        {
-            it->second.lcm = lcm_state;
-            it->second.vm  = vm_state;
-        }
-    }
-}
-
-// -----------------------------------------------------------------------------
-// -----------------------------------------------------------------------------
-
 void VirtualMachineStateHook::do_hook(void *arg)
 {
 
     VirtualMachine * vm;
     int              rc;
 
-    VirtualMachine::LcmState prev_lcm, cur_lcm;
-    VirtualMachine::VmState  prev_vm, cur_vm;
-
     vm = static_cast<VirtualMachine *>(arg);
 
     if ( vm == 0 )
@@ -95,26 +34,14 @@ void VirtualMachineStateHook::do_hook(void *arg)
         return;
     }
 
-    rc = get_state(vm->get_oid(), prev_lcm, prev_vm);
+    if ( vm->has_changed_state() && 
-
+         vm->get_lcm_state() == lcm && 
-    if ( rc != 0 )
+         vm->get_state() == this->vm )
-    {
-        return;
-    }
-
-    cur_lcm = vm->get_lcm_state();
-    cur_vm  = vm->get_state();
-
-    if ( prev_lcm == cur_lcm && prev_vm == cur_vm ) //Still in the same state
-    {
-        return;
-    }
-
-    if ( cur_lcm == lcm && cur_vm == this->vm )
     {
         string  parsed_args = args;
 
-        parse_hook_arguments(vm, prev_vm, prev_lcm, parsed_args);
+        parse_hook_arguments(vm, vm->get_prev_state(), vm->get_prev_lcm_state(),
+            parsed_args);
 
         Nebula& ne        = Nebula::instance();
         HookManager * hm  = ne.get_hm();
@@ -125,7 +52,7 @@ void VirtualMachineStateHook::do_hook(void *arg)
         {
             if ( ! remote )
             {
-                hmd->execute(vm->get_oid(),name,cmd,parsed_args);
+                hmd->execute(vm->get_oid(), name, cmd, parsed_args);
             }
             else if ( vm->hasHistory() )
             {
@@ -167,22 +94,3 @@ void VirtualMachineStateHook::parse_hook_arguments(PoolObjectSQL * obj,
         parsed.replace(found, 15, VirtualMachine::lcm_state_to_str(str, prev_lcm));
     }
 }
-
-// -----------------------------------------------------------------------------
-// -----------------------------------------------------------------------------
-
-void VirtualMachineUpdateStateHook::do_hook(void *arg)
-{
-    VirtualMachine * vm = static_cast<VirtualMachine *>(arg);
-
-    if ( vm == 0 )
-    {
-        return;
-    }
-
-    update_state(vm->get_oid(), vm->get_lcm_state(), vm->get_state());
-}
-
-// -----------------------------------------------------------------------------
-// -----------------------------------------------------------------------------
-

src/vm/VirtualMachinePool.cc

@@ -63,8 +63,6 @@ VirtualMachinePool::VirtualMachinePool(
     string arg;
     bool   remote;
 
-    bool state_hook = false;
-
     _monitor_expiration = expire_time;
     _submit_on_hold = on_hold;
     _default_cpu_cost = default_cpu_cost;
@@ -134,8 +132,6 @@ VirtualMachinePool::VirtualMachinePool(
             hook = new VirtualMachineStateHook(name, cmd, arg, remote,
                            VirtualMachine::PROLOG, VirtualMachine::ACTIVE);
             add_hook(hook);
-
-            state_hook = true;
         }
         else if ( on == "RUNNING" )
         {
@@ -144,8 +140,6 @@ VirtualMachinePool::VirtualMachinePool(
             hook = new VirtualMachineStateHook(name, cmd, arg, remote,
                            VirtualMachine::RUNNING, VirtualMachine::ACTIVE);
             add_hook(hook);
-
-            state_hook = true;
         }
         else if ( on == "SHUTDOWN" )
         {
@@ -154,8 +148,6 @@ VirtualMachinePool::VirtualMachinePool(
             hook = new VirtualMachineStateHook(name, cmd, arg, remote,
                             VirtualMachine::EPILOG, VirtualMachine::ACTIVE);
             add_hook(hook);
-
-            state_hook = true;
         }
         else if ( on == "STOP" )
         {
@@ -164,8 +156,6 @@ VirtualMachinePool::VirtualMachinePool(
             hook = new VirtualMachineStateHook(name, cmd, arg, remote,
                             VirtualMachine::LCM_INIT, VirtualMachine::STOPPED);
             add_hook(hook);
-
-            state_hook = true;
         }
         else if ( on == "DONE" )
         {
@@ -174,8 +164,6 @@ VirtualMachinePool::VirtualMachinePool(
             hook = new VirtualMachineStateHook(name, cmd, arg, remote,
                             VirtualMachine::LCM_INIT, VirtualMachine::DONE);
             add_hook(hook);
-
-            state_hook = true;
         }
         else if ( on == "FAILED" )
         {
@@ -184,8 +172,6 @@ VirtualMachinePool::VirtualMachinePool(
             hook = new VirtualMachineStateHook(name, cmd, arg, remote,
                             VirtualMachine::LCM_INIT, VirtualMachine::FAILED);
             add_hook(hook);
-
-            state_hook = true;
         }
         else if ( on == "UNKNOWN" )
         {
@@ -194,8 +180,6 @@ VirtualMachinePool::VirtualMachinePool(
             hook = new VirtualMachineStateHook(name, cmd, arg, remote,
                             VirtualMachine::UNKNOWN, VirtualMachine::ACTIVE);
             add_hook(hook);
-
-            state_hook = true;
         }
         else if ( on == "CUSTOM" )
         {
@@ -229,8 +213,6 @@ VirtualMachinePool::VirtualMachinePool(
                     lcm_state, vm_state);
 
             add_hook(hook);
-
-            state_hook = true;
         }
         else
         {
@@ -241,15 +223,6 @@ VirtualMachinePool::VirtualMachinePool(
         }
     }
 
-    if ( state_hook )
-    {
-        VirtualMachineUpdateStateHook * hook;
-
-        hook = new VirtualMachineUpdateStateHook();
-
-        add_hook(hook);
-    }
-
     // Set restricted attributes
     VirtualMachineTemplate::set_restricted_attributes(restricted_attrs);
 }