Merge branch 'feature-1223' of git.opennebula.org:one into feature-1223

Conflicts:
	include/DispatchManager.h
	include/VirtualMachine.h
	src/rm/RequestManagerVirtualMachine.cc

include/DispatchManager.h

@@ -250,6 +250,21 @@ public:
      */
     int attach(int vid, VirtualMachineTemplate * tmpl, string & error_str);
 
+    /**
+     * Starts the detach disk action.
+     *
+     * @param vm pointer to a VirtualMachine with its mutex locked. It will be
+     * unlocked
+     * @param disk_id Disk to detach
+     * @param error_str Error reason, if any
+     *
+     * @return 0 on success, -1 action error, -2 if the VM is in a wrong a state
+     */
+    int detach(
+        VirtualMachine* vm,
+        int             disk_id,
+        string &        error_str);
+
 private:
     /**
      *  Thread id for the Dispatch Manager

include/LifeCycleManager.h

@@ -60,6 +60,8 @@ public:
         EPILOG_FAILURE,   /**< Sent by the TM when the epilog phase fails     */
         ATTACH_SUCCESS,   /**< Sent by the VMM when an attach action succeeds */
         ATTACH_FAILURE,   /**< Sent by the VMM when an attach action fails    */
+        DETACH_SUCCESS,   /**< Sent by the VMM when a detach action succeeds  */
+        DETACH_FAILURE,   /**< Sent by the VMM when a detach action fails     */
         DEPLOY,           /**< Sent by the DM to deploy a VM on a host        */
         SUSPEND,          /**< Sent by the DM to suspend an running VM        */
         RESTORE,          /**< Sent by the DM to restore a suspended VM       */
@@ -178,6 +180,10 @@ private:
 
     void attach_failure_action(int vid);
 
+    void detach_success_action(int vid);
+
+    void detach_failure_action(int vid);
+
     void deploy_action(int vid);
 
     void suspend_action(int vid);

include/Log.h

@@ -80,6 +80,14 @@ public:
         const MessageType       type,
         const char *            message);
 
+    virtual void log(
+        const char *           module,
+        const Log::MessageType type,
+        const string&          message)
+    {
+        log(module,type,message.c_str());
+    };
+
 private:
     char * log_file;
 };

include/RequestManagerVirtualMachine.h

@@ -184,6 +184,23 @@ public:
             RequestAttributes& att);
 };
 
+/* ------------------------------------------------------------------------- */
+/* ------------------------------------------------------------------------- */
+
+class VirtualMachineDetach : public RequestManagerVirtualMachine
+{
+public:
+    VirtualMachineDetach():
+        RequestManagerVirtualMachine("VirtualMachineDetach",
+                           "Detaches a disk from a virtual machine",
+                           "A:sii"){};
+
+    ~VirtualMachineDetach(){};
+
+    void request_execute(xmlrpc_c::paramList const& _paramList,
+            RequestAttributes& att);
+};
+
 /* -------------------------------------------------------------------------- */
 /* -------------------------------------------------------------------------- */
 /* -------------------------------------------------------------------------- */

include/TransferManager.h

@@ -110,12 +110,30 @@ public:
     int prolog_transfer_command(
             VirtualMachine *        vm,
             const VectorAttribute * disk,
-            int                     disk_index,
             string&                 system_tm_mad,
             string&                 opennebula_hostname,
             ostream&                xfr,
             string&                 error_str);
 
+    /**
+     * Inserts a transfer command in the xfs stream
+     *
+     * @param vm The VM
+     * @param disk Disk to transfer
+     * @param disk_index Disk index
+     * @param xfr Stream where the transfer command will be written
+     * @param error_str Error reason, if any
+     *
+     * @return 0 if the command was written to xfr, -1 if there is an
+     * error (and error reason in error_str), -2 if the disk didn't need
+     * a transfer command
+     */
+    int epilog_transfer_command(
+            VirtualMachine *        vm,
+            const VectorAttribute * disk,
+            ostream&                xfr,
+            string&                 error_str);
+
 private:    
     /**
      *  Thread id for the Transfer Manager

include/VirtualMachine.h

@@ -118,6 +118,21 @@ public:
         }
     };
 
+    /**
+     *  writes a log message in vm.log. The class lock should be locked and
+     *  the VM MUST BE obtained through the VirtualMachinePool get() method.
+     */
+    void log(
+        const char *           module,
+        const Log::MessageType type,
+        const string&          message) const
+    {
+        if (_log != 0)
+        {
+            _log->log(module,type,message);
+        }
+    };
+
     /**
      * Function to print the VirtualMachine object into a string in
      * XML format
@@ -801,6 +816,20 @@ public:
         obj_template->set(new_disk);
     }
 
+    /**
+     * Cleans the ATTACH = YES attribute from the disks
+     *
+     * @return 0 on success, -1 otherwise
+     */
+    int detach_success();
+
+    /**
+     * Cleans the ATTACH = YES attribute from the disks
+     *
+     * @return 0 on success, -1 otherwise
+     */
+    int detach_failure();
+
 private:
 
     // -------------------------------------------------------------------------

include/VirtualMachineManager.h

@@ -57,7 +57,8 @@ public:
         TIMER,
         DRIVER_CANCEL,
         FINALIZE,
-        ATTACH
+        ATTACH,
+        DETACH
     };
 
     /**
@@ -305,6 +306,14 @@ private:
     void attach_action(
         int vid);
 
+    /**
+     * Detaches a disk from a VM. The VM must have a disk with the
+     * attribute ATTACH = YES
+     *    @param vid the id of the VM.
+     */
+    void detach_action(
+        int vid);
+
     /**
      *  This function cancels the current driver operation
      */

include/VirtualMachineManagerDriver.h

@@ -244,6 +244,18 @@ private:
         write_drv("ATTACH", oid, drv_msg);
     }
 
+    /**
+     *  Sends a detach request to the MAD: "DETACH ID XML_DRV_MSG"
+     *    @param oid the virtual machine id.
+     *    @param drv_msg xml data for the mad operation
+     */
+    void detach (
+        const int     oid,
+        const string& drv_msg) const
+    {
+        write_drv("DETACH", oid, drv_msg);
+    }
+
 private:
 
     void write_drv(const char * aname, const int oid, const string& msg) const

src/dm/DispatchManagerActions.cc

@@ -676,6 +676,7 @@ error:
 
     return -2;
 }
+
 /* -------------------------------------------------------------------------- */
 /* -------------------------------------------------------------------------- */
 
@@ -852,6 +853,9 @@ int DispatchManager::attach(int vid,
     
     vm->get_disk_info(num_disks, used_targets);
 
+    // TODO: Cancel resched?
+    // vm->set_resched(false);
+
     vm->set_state(VirtualMachine::HOTPLUG);
 
     vm->set_resched(false);
@@ -906,3 +910,63 @@ int DispatchManager::attach(int vid,
 
     return 0;
 }
+
+/* -------------------------------------------------------------------------- */
+/* -------------------------------------------------------------------------- */
+
+int DispatchManager::detach(
+    VirtualMachine* vm,
+    int             disk_id,
+    string &        error_str)
+{
+    ostringstream   oss;
+    int             rc;
+    int             vid = vm->get_oid();
+
+    Nebula&                 nd = Nebula::instance();
+    VirtualMachineManager * vmm = nd.get_vmm();
+
+    oss << "Detaching disk " << disk_id << " from VM " << vid;
+    NebulaLog::log("DiM",Log::DEBUG,oss);
+
+    if ( vm->get_state() != VirtualMachine::ACTIVE ||
+         vm->get_lcm_state() != VirtualMachine::RUNNING )
+    {
+        goto error_state;
+    }
+
+    rc = vm->detach_disk(disk_id, error_str);
+
+    if ( rc != 0 )
+    {
+        goto error;
+    }
+
+    // TODO: Cancel resched?
+    // vm->set_resched(false);
+
+    vm->set_state(VirtualMachine::HOTPLUG);
+    vmpool->update(vm);
+
+    vm->unlock();
+
+    vmm->trigger(VirtualMachineManager::DETACH,vid);
+
+    return 0;
+
+error:
+    vm->unlock();
+
+    return -1;
+
+error_state:
+    oss.str("");
+    oss << "Could not attach a new disk to VM " << vid << ", wrong state.";
+    error_str = oss.str();
+
+    NebulaLog::log("DiM", Log::ERROR, error_str);
+
+    vm->unlock();
+
+    return -2;
+}

src/lcm/LifeCycleManager.cc

@@ -137,6 +137,14 @@ void LifeCycleManager::trigger(Actions action, int _vid)
         aname = "ATTACH_FAILURE";
         break;
 
+    case DETACH_SUCCESS:
+        aname = "DETACH_SUCCESS";
+        break;
+
+    case DETACH_FAILURE:
+        aname = "DETACH_FAILURE";
+        break;
+
     case DEPLOY:
         aname = "DEPLOY";
         break;
@@ -278,6 +286,14 @@ void LifeCycleManager::do_action(const string &action, void * arg)
     {
         attach_failure_action(vid);
     }
+    else if (action == "DETACH_SUCCESS")
+    {
+        detach_success_action(vid);
+    }
+    else if (action == "DETACH_FAILURE")
+    {
+        detach_failure_action(vid);
+    }
     else if (action == "DEPLOY")
     {
         deploy_action(vid);

src/lcm/LifeCycleStates.cc

@@ -896,4 +896,53 @@ void LifeCycleManager::attach_failure_action(int vid)
 /* -------------------------------------------------------------------------- */
 /* -------------------------------------------------------------------------- */
 
+void LifeCycleManager::detach_success_action(int vid)
+{
+    VirtualMachine *    vm;
+
+    vm = vmpool->get(vid,true);
+
+    if ( vm == 0 )
+    {
+        return;
+    }
+
+    vm->detach_success();
+
+    vm->set_state(VirtualMachine::RUNNING);
+
+    vmpool->update(vm);
+
+    vm->unlock();
+
+    // TODO: update quotas
+}
+
+/* -------------------------------------------------------------------------- */
+/* -------------------------------------------------------------------------- */
+
+void LifeCycleManager::detach_failure_action(int vid)
+{
+    VirtualMachine *    vm;
+
+    vm = vmpool->get(vid,true);
+
+    if ( vm == 0 )
+    {
+        return;
+    }
+
+    vm->detach_failure();
+
+    vm->set_state(VirtualMachine::RUNNING);
+
+    vmpool->update(vm);
+
+    vm->unlock();
+
+}
+
+/* -------------------------------------------------------------------------- */
+/* -------------------------------------------------------------------------- */
+
 

src/rm/RequestManager.cc

@@ -250,6 +250,7 @@ void RequestManager::register_xml_methods()
     xmlrpc_c::methodPtr vm_savedisk(new VirtualMachineSaveDisk());
     xmlrpc_c::methodPtr vm_monitoring(new VirtualMachineMonitoring());
     xmlrpc_c::methodPtr vm_attach(new VirtualMachineAttach());
+    xmlrpc_c::methodPtr vm_detach(new VirtualMachineDetach());
     xmlrpc_c::methodPtr vm_pool_acct(new VirtualMachinePoolAccounting());
     xmlrpc_c::methodPtr vm_pool_monitoring(new VirtualMachinePoolMonitoring());
 
@@ -359,6 +360,7 @@ void RequestManager::register_xml_methods()
     RequestManagerRegistry.addMethod("one.vm.chmod", vm_chmod);
     RequestManagerRegistry.addMethod("one.vm.monitoring", vm_monitoring);
     RequestManagerRegistry.addMethod("one.vm.attach", vm_attach);
+    RequestManagerRegistry.addMethod("one.vm.detach", vm_detach);
 
     RequestManagerRegistry.addMethod("one.vmpool.info", vm_pool_info);
     RequestManagerRegistry.addMethod("one.vmpool.accounting", vm_pool_acct);

src/rm/RequestManagerVirtualMachine.cc

@@ -694,3 +694,48 @@ void VirtualMachineAttach::request_execute(xmlrpc_c::paramList const& paramList,
 
 /* -------------------------------------------------------------------------- */
 /* -------------------------------------------------------------------------- */
+
+void VirtualMachineDetach::request_execute(xmlrpc_c::paramList const& paramList,
+                                            RequestAttributes& att)
+{
+    Nebula&             nd = Nebula::instance();
+    DispatchManager *   dm = nd.get_dm();
+
+    VirtualMachine * vm;
+    PoolObjectAuth host_perms;
+
+    int rc;
+    string error_str;
+
+    int     id      = xmlrpc_c::value_int(paramList.getInt(1));
+    int     disk_id = xmlrpc_c::value_int(paramList.getInt(2));
+
+    // TODO: auth & quotas
+
+    vm = get_vm(id, att);
+
+    if ( vm == 0 )
+    {
+        failure_response(NO_EXISTS,
+                get_error(object_name(auth_object),id),
+                att);
+
+        return;
+    }
+
+    rc = dm->detach(vm, disk_id, error_str);
+
+    if ( rc != 0 )
+    {
+        failure_response(ACTION,
+                request_error(error_str, ""),
+                att);
+
+        return;
+    }
+
+    success_response(id, att);
+}
+
+/* -------------------------------------------------------------------------- */
+/* -------------------------------------------------------------------------- */

src/tm/TransferManager.cc

@@ -204,7 +204,6 @@ void TransferManager::do_action(const string &action, void * arg)
 int TransferManager::prolog_transfer_command(
         VirtualMachine *        vm,
         const VectorAttribute * disk,
-        int                     disk_index,
         string&                 system_tm_mad,
         string&                 opennebula_hostname,
         ostream&                xfr,
@@ -217,9 +216,12 @@ int TransferManager::prolog_transfer_command(
     string format;
     string tm_mad;
     string ds_id;
+    int    disk_index;
 
     ostringstream os;
 
+    disk->vector_value("DISK_ID", disk_index);
+
     type = disk->vector_value("TYPE");
 
     transform(type.begin(),type.end(),type.begin(),(int(*)(int))toupper);
@@ -412,7 +414,7 @@ void TransferManager::prolog_action(int vid)
             continue;
         }
 
-        rc = prolog_transfer_command(vm, disk, i, system_tm_mad,
+        rc = prolog_transfer_command(vm, disk, system_tm_mad,
                 opennebula_hostname, xfr, error_str);
 
         if ( rc != 0 )
@@ -502,6 +504,7 @@ void TransferManager::prolog_migr_action(int vid)
     string tm_mad;
     string system_tm_mad;
     string ds_id;
+    int    disk_id;
 
     vector<const Attribute *> attrs;
     int                       num;
@@ -562,6 +565,7 @@ void TransferManager::prolog_migr_action(int vid)
 
         tm_mad = disk->vector_value("TM_MAD");
         ds_id  = disk->vector_value("DATASTORE_ID");
+        disk->vector_value_str("DISK_ID", disk_id);
 
         if ( tm_mad.empty() ||  ds_id.empty() )
         {
@@ -572,9 +576,9 @@ void TransferManager::prolog_migr_action(int vid)
         xfr << "MV "
             << tm_mad << " "
             << vm->get_previous_hostname() << ":" 
-            << vm->get_remote_system_dir() << "/disk." << i << " "
+            << vm->get_remote_system_dir() << "/disk." << disk_id << " "
             << vm->get_hostname() << ":"
-            << vm->get_remote_system_dir() << "/disk." << i << " "
+            << vm->get_remote_system_dir() << "/disk." << disk_id << " "
             << vm->get_oid() << " "
             << ds_id << endl;
     }
@@ -627,6 +631,7 @@ void TransferManager::prolog_resume_action(int vid)
     string tm_mad;
     string system_tm_mad;
     string ds_id;
+    int    disk_id;
 
     vector<const Attribute *> attrs;
     int                       num;
@@ -686,6 +691,7 @@ void TransferManager::prolog_resume_action(int vid)
 
         tm_mad = disk->vector_value("TM_MAD");
         ds_id  = disk->vector_value("DATASTORE_ID");
+        disk->vector_value_str("DISK_ID", disk_id);
 
         if ( tm_mad.empty() || ds_id.empty() )
         {
@@ -696,9 +702,9 @@ void TransferManager::prolog_resume_action(int vid)
         xfr << "MV "
             << tm_mad << " "
             << nd.get_nebula_hostname() << ":" 
-            << vm->get_system_dir() << "/disk." << i << " "
+            << vm->get_system_dir() << "/disk." << disk_id << " "
             << vm->get_hostname() << ":"
-            << vm->get_remote_system_dir() << "/disk." << i << " "
+            << vm->get_remote_system_dir() << "/disk." << disk_id << " "
             << vm->get_oid() << " "
             << ds_id << endl;
     }
@@ -739,17 +745,85 @@ error_common:
 /* -------------------------------------------------------------------------- */
 /* -------------------------------------------------------------------------- */
 
+int TransferManager::epilog_transfer_command(
+        VirtualMachine *        vm,
+        const VectorAttribute * disk,
+        ostream&                xfr,
+        string&                 error_str)
+{
+    string save;
+    string tm_mad;
+    string ds_id;
+    int    disk_index;
+
+    save   = disk->vector_value("SAVE");
+    ds_id  = disk->vector_value("DATASTORE_ID");
+    tm_mad = disk->vector_value("TM_MAD");
+
+    if ( save.empty() || ds_id.empty() || tm_mad.empty() )
+    {
+        return -2;
+    }
+
+    disk->vector_value("DISK_ID", disk_index);
+
+    transform(save.begin(),save.end(),save.begin(),(int(*)(int))toupper);
+
+    if ( save == "YES" )
+    {
+        string source;
+        string save_source;
+
+        source      = disk->vector_value("SOURCE");
+        save_source = disk->vector_value("SAVE_AS_SOURCE");
+
+        if (source.empty() && save_source.empty())
+        {
+            error_str = "No SOURCE to save disk image";
+            return -1;
+        }
+
+        if (!save_source.empty())//Use the save_as_source instead
+        {
+            source = save_source;
+        }
+
+        //MVDS tm_mad hostname:remote_system_dir/disk.0 <fe:SOURCE|SOURCE> vmid dsid
+        xfr << "MVDS "
+            << tm_mad << " "
+            << vm->get_hostname() << ":"
+            << vm->get_remote_system_dir() << "/disk." << disk_index << " "
+            << source << " "
+            << vm->get_oid() << " "
+            << ds_id;
+    }
+    else //No saving disk
+    {
+        //DELETE tm_mad hostname:remote_system_dir/disk.i vmid ds_id
+        xfr << "DELETE "
+            << tm_mad << " "
+            << vm->get_hostname() << ":"
+            << vm->get_remote_system_dir() << "/disk." << disk_index << " "
+            << vm->get_oid() << " "
+            << ds_id;
+    }
+
+    return 0;
+}
+
+/* -------------------------------------------------------------------------- */
+/* -------------------------------------------------------------------------- */
+
 void TransferManager::epilog_action(int vid)
 {
     ofstream        xfr;
     ostringstream   os;
     string          xfr_name;
     string          system_tm_mad;
-    string          tm_mad;
+    string          error_str;
-    string          ds_id;
+    int             rc;
 
     const VectorAttribute * disk;
-    string          save;
 
     VirtualMachine *    vm;
     Nebula&             nd = Nebula::instance();
@@ -807,55 +881,19 @@ void TransferManager::epilog_action(int vid)
             continue;
         }
 
-        save   = disk->vector_value("SAVE");
+        rc = epilog_transfer_command(vm, disk, xfr, error_str);
-        ds_id  = disk->vector_value("DATASTORE_ID");
-        tm_mad = disk->vector_value("TM_MAD");
 
-        if ( save.empty() || ds_id.empty() || tm_mad.empty() )
+        if ( rc == -2 )
         {
             continue;
         }
-        
-        transform(save.begin(),save.end(),save.begin(),(int(*)(int))toupper);
 
-        if ( save == "YES" )
+        if ( rc == -1 )
         {
-            string source;
+            vm->log("TM", Log::ERROR, error_str);
-            string save_source;
-
-            source      = disk->vector_value("SOURCE");
-            save_source = disk->vector_value("SAVE_AS_SOURCE");
-
-            if (source.empty() && save_source.empty())
-            {
-                vm->log("TM", Log::ERROR, "No SOURCE to save disk image");
-                continue;
-            }
-
-            if (!save_source.empty())//Use the save_as_source instead
-            {
-                source = save_source;
-            }
-
-            //MVDS tm_mad hostname:remote_system_dir/disk.0 <fe:SOURCE|SOURCE> vmid dsid
-            xfr << "MVDS " 
-                << tm_mad << " "
-                << vm->get_hostname() << ":" 
-                << vm->get_remote_system_dir() << "/disk." << i << " "
-                << source << " "
-                << vm->get_oid() << " "
-                << ds_id << endl;
-        }
-        else //No saving disk
-        {
-            //DELETE tm_mad hostname:remote_system_dir/disk.i vmid ds_id
-            xfr << "DELETE "
-                << tm_mad << " "
-                << vm->get_hostname() << ":"
-                << vm->get_remote_system_dir() << "/disk." << i << " "
-                << vm->get_oid() << " "
-                << ds_id << endl;
         }
+
+        xfr << endl;
     }
 
     //DELETE system_tm_mad hostname:remote_system_dir vmid ds_id
@@ -901,6 +939,7 @@ void TransferManager::epilog_stop_action(int vid)
     string        tm_mad;
     string        system_tm_mad;
     string        ds_id;
+    int           disk_id;
 
     VirtualMachine * vm;
     Nebula&          nd = Nebula::instance();
@@ -961,6 +1000,7 @@ void TransferManager::epilog_stop_action(int vid)
 
         tm_mad = disk->vector_value("TM_MAD");
         ds_id  = disk->vector_value("DATASTORE_ID");
+        disk->vector_value_str("DISK_ID", disk_id);
 
         if (tm_mad.empty() || ds_id.empty())
         {
@@ -971,9 +1011,9 @@ void TransferManager::epilog_stop_action(int vid)
         xfr << "MV "
             << tm_mad << " "
             << vm->get_hostname() << ":"
-            << vm->get_remote_system_dir() << "/disk." << i << " "
+            << vm->get_remote_system_dir() << "/disk." << disk_id << " "
             << nd.get_nebula_hostname() << ":" 
-            << vm->get_system_dir() << "/disk." << i << " "
+            << vm->get_system_dir() << "/disk." << disk_id << " "
             << vm->get_oid() << " "
             << ds_id << endl;
     }
@@ -1024,6 +1064,7 @@ void TransferManager::epilog_delete_action(int vid)
     string        system_tm_mad;
     string        tm_mad;
     string        ds_id;
+    int           disk_id;
 
     VirtualMachine * vm;
     Nebula&          nd = Nebula::instance();
@@ -1084,6 +1125,7 @@ void TransferManager::epilog_delete_action(int vid)
 
         tm_mad = disk->vector_value("TM_MAD");
         ds_id = disk->vector_value("DATASTORE_ID");
+        disk->vector_value_str("DISK_ID", disk_id);
 
         if ( tm_mad.empty() || ds_id.empty() )
         {
@@ -1094,7 +1136,7 @@ void TransferManager::epilog_delete_action(int vid)
         xfr << "DELETE "
             << tm_mad << " "
             << vm->get_hostname() << ":"
-            << vm->get_remote_system_dir() << "/disk." << i << " "
+            << vm->get_remote_system_dir() << "/disk." << disk_id << " "
             << vm->get_oid() << " "
             << ds_id << endl;
     }
@@ -1144,6 +1186,7 @@ void TransferManager::epilog_delete_previous_action(int vid)
     string        system_tm_mad;
     string        tm_mad;
     string        ds_id;
+    int           disk_id;
 
     VirtualMachine * vm;
     Nebula&          nd = Nebula::instance();
@@ -1204,6 +1247,7 @@ void TransferManager::epilog_delete_previous_action(int vid)
 
         tm_mad = disk->vector_value("TM_MAD");
         ds_id  = disk->vector_value("DATASTORE_ID");
+        disk->vector_value_str("DISK_ID", disk_id);
 
         if (tm_mad.empty() || ds_id.empty())
         {
@@ -1214,7 +1258,7 @@ void TransferManager::epilog_delete_previous_action(int vid)
         xfr << "DELETE "
             << tm_mad << " "
             << vm->get_previous_hostname() << ":"
-            << vm->get_remote_system_dir() << "/disk." << i << " "
+            << vm->get_remote_system_dir() << "/disk." << disk_id << " "
             << vm->get_oid() << " "
             << ds_id << endl;
     }

src/vm/VirtualMachine.cc

@@ -1236,6 +1236,51 @@ VectorAttribute * VirtualMachine::set_up_attach_disk(
 /* -------------------------------------------------------------------------- */
 /* -------------------------------------------------------------------------- */
 
+// TODO: this method requires the VM to be locked, and then it locks the Image
+// to acquire. Check if this can be troublesome
+
+int VirtualMachine::detach_disk(int disk_id, string& error_str)
+{
+    int                  num_disks;
+    vector<Attribute  *> disks;
+    VectorAttribute *    disk;
+    bool                 found = false;
+
+    num_disks = obj_template->get("DISK", disks);
+
+    int i = 0;
+    int d_id;
+
+    while( !found && i<num_disks )
+    {
+        disk = dynamic_cast<VectorAttribute * >(disks[i]);
+
+        i++;
+
+        if ( disk == 0 )
+        {
+            continue;
+        }
+
+        disk->vector_value("DISK_ID", d_id);
+        if ( d_id == disk_id )
+        {
+            disk->replace("ATTACH", "YES");
+            found = true;
+        }
+    }
+
+    if ( !found )
+    {
+        return -1;
+    }
+
+    return 0;
+}
+
+/* -------------------------------------------------------------------------- */
+/* -------------------------------------------------------------------------- */
+
 VectorAttribute* VirtualMachine::get_attach_disk()
 {
     int                  num_disks;
@@ -1321,6 +1366,22 @@ VectorAttribute * VirtualMachine::delete_attach_disk()
 /* -------------------------------------------------------------------------- */
 /* -------------------------------------------------------------------------- */
 
+int VirtualMachine::detach_success()
+{
+    return attach_failure();
+}
+
+/* -------------------------------------------------------------------------- */
+/* -------------------------------------------------------------------------- */
+
+int VirtualMachine::detach_failure()
+{
+    return attach_success();
+}
+
+/* -------------------------------------------------------------------------- */
+/* -------------------------------------------------------------------------- */
+
 void VirtualMachine::release_disk_images()
 {
     int     iid;

src/vmm/LibVirtDriverKVM.cc

@@ -54,6 +54,7 @@ int LibVirtDriver::deployment_description_kvm(
     string  ro         = "";
     string  driver     = "";
     string  cache      = "";
+    int     disk_id;
     string  default_driver       = "";
     string  default_driver_cache = "";
     bool    readonly;
@@ -306,6 +307,7 @@ int LibVirtDriver::deployment_description_kvm(
         bus    = disk->vector_value("BUS");
         driver = disk->vector_value("DRIVER");
         cache  = disk->vector_value("CACHE");
+        disk->vector_value_str("DISK_ID", disk_id);
 
         if (target.empty())
         {
@@ -335,19 +337,19 @@ int LibVirtDriver::deployment_description_kvm(
         {
             file << "\t\t<disk type='block' device='disk'>" << endl
                  << "\t\t\t<source dev='" << vm->get_remote_system_dir() 
-                 << "/disk." << i << "'/>" << endl;
+                 << "/disk." << disk_id << "'/>" << endl;
         }
         else if ( type == "CDROM" )
         {
             file << "\t\t<disk type='file' device='cdrom'>" << endl
                  << "\t\t\t<source file='" << vm->get_remote_system_dir() 
-                 << "/disk." << i << "'/>" << endl;
+                 << "/disk." << disk_id << "'/>" << endl;
         }
         else
         {
             file << "\t\t<disk type='file' device='disk'>" << endl
                  << "\t\t\t<source file='" << vm->get_remote_system_dir() 
-                 << "/disk." << i << "'/>" << endl;
+                 << "/disk." << disk_id << "'/>" << endl;
         }
 
         // ---- target device to map the disk ----

src/vmm/LibVirtDriverVMware.cc

@@ -55,6 +55,7 @@ int LibVirtDriver::deployment_description_vmware(
     string  source     = "";
     string  datastore  = "";
     string  driver     = "";
+    int     disk_id;
     string  default_driver = "";
     bool    readonly;
 
@@ -199,6 +200,7 @@ int LibVirtDriver::deployment_description_vmware(
         bus    = disk->vector_value("BUS");
         source = disk->vector_value("SOURCE");
         driver = disk->vector_value("DRIVER");
+        disk->vector_value_str("DISK_ID", disk_id);
 
         if (target.empty())
         {
@@ -226,19 +228,19 @@ int LibVirtDriver::deployment_description_vmware(
         {
             file << "\t\t<disk type='block' device='disk'>" << endl;
             file << "\t\t\t<source file=[" <<  datastore << "] " << vm->get_oid()
-                << "/disk."  << i << "'/>"  << endl;
+                << "/disk."  << disk_id << "'/>"  << endl;
         }
         else if ( type == "CDROM" )
         {
             file << "\t\t<disk type='file' device='cdrom'>" << endl;
             file << "\t\t\t<source file=[" <<  datastore << "] " << vm->get_oid()
-                << "/disk."  << i << ".iso'/>"  << endl;
+                << "/disk."  << disk_id << ".iso'/>"  << endl;
         }
         else
         {
             file << "\t\t<disk type='file' device='disk'>" << endl
                  << "\t\t\t<source file='[" <<  datastore <<"] " << vm->get_oid()
-                 << "/disk." << i << "/disk.vmdk'/>" << endl;
+                 << "/disk." << disk_id << "/disk.vmdk'/>" << endl;
         }
 
         file << "\t\t\t<target dev='" << target << "'";

src/vmm/VirtualMachineManager.cc

@@ -166,6 +166,10 @@ void VirtualMachineManager::trigger(Actions action, int _vid)
         aname = "ATTACH";
         break;
 
+    case DETACH:
+        aname = "DETACH";
+        break;
+
     default:
         delete vid;
         return;
@@ -246,6 +250,10 @@ void VirtualMachineManager::do_action(const string &action, void * arg)
     {
         attach_action(vid);
     }
+    else if (action == "DETACH")
+    {
+        detach_action(vid);
+    }
     else if (action == ACTION_TIMER)
     {
         timer_action();
@@ -330,7 +338,7 @@ string * VirtualMachineManager::format_message(
 
     if ( !tm_command.empty() )
     {
-        oss << "<TM_COMMAND>"       << tm_command       << "</TM_COMMAND>"
+        oss << "<TM_COMMAND><![CDATA["  << tm_command   << "]]></TM_COMMAND>"
             << "<DISK_ID>"          << disk_id          << "</DISK_ID>"
             << "<DISK_TARGET_PATH>" << disk_target_path << "</DISK_TARGET_PATH>";
     }
@@ -1360,12 +1368,11 @@ void VirtualMachineManager::attach_action(
     system_tm_mad = nd.get_system_ds_tm_mad();
     opennebula_hostname = nd.get_nebula_hostname();
 
-    disk_id = disk->vector_value("DISK_ID", disk_id);
+    disk->vector_value("DISK_ID", disk_id);
 
     Nebula::instance().get_tm()->prolog_transfer_command(
             vm,
             disk,
-            disk_id,
             system_tm_mad,
             opennebula_hostname,
             os,
@@ -1428,6 +1435,121 @@ error_common:
     return;
 }
 
+/* -------------------------------------------------------------------------- */
+/* -------------------------------------------------------------------------- */
+
+void VirtualMachineManager::detach_action(
+    int vid)
+{
+    VirtualMachine *                    vm;
+    const VirtualMachineManagerDriver * vmd;
+
+    ostringstream os;
+    string        vm_tmpl;
+    string *      drv_msg;
+    string        tm_command;
+    string        system_tm_mad;
+    string        opennebula_hostname;
+    string        epilog_cmd;
+    string        disk_path;
+    string        error_str;
+
+    const VectorAttribute * disk;
+    int disk_id;
+
+    Nebula&          nd = Nebula::instance();
+
+    // Get the VM from the pool
+    vm = vmpool->get(vid,true);
+
+    if (vm == 0)
+    {
+        return;
+    }
+
+    if (!vm->hasHistory())
+    {
+        goto error_history;
+    }
+
+    // Get the driver for this VM
+    vmd = get(vm->get_vmm_mad());
+
+    if ( vmd == 0 )
+    {
+        goto error_driver;
+    }
+
+    disk = vm->get_attach_disk();
+
+    if ( disk == 0 )
+    {
+        goto error_disk;
+    }
+
+    system_tm_mad = nd.get_system_ds_tm_mad();
+    opennebula_hostname = nd.get_nebula_hostname();
+
+    disk->vector_value("DISK_ID", disk_id);
+
+    Nebula::instance().get_tm()->epilog_transfer_command(vm,disk,os,error_str);
+
+    epilog_cmd = os.str();
+
+    os.str("");
+    os << vm->get_remote_system_dir() << "/disk." << disk_id;
+
+    disk_path = os.str();
+
+    // Invoke driver method
+    drv_msg = format_message(
+        vm->get_hostname(),
+        vm->get_vnm_mad(),
+        "",
+        "",
+        vm->get_deploy_id(),
+        "",
+        "",
+        "",
+        disk_id,
+        epilog_cmd,
+        disk_path,
+        vm->to_xml(vm_tmpl));
+
+
+    vmd->detach(vid, *drv_msg);
+
+    delete drv_msg;
+
+    vm->unlock();
+
+    return;
+
+error_disk:
+    os.str("");
+    os << "detach_action, could not find disk to detach";
+    goto error_common;
+
+error_history:
+    os.str("");
+    os << "detach_action, VM has no history";
+    goto error_common;
+
+error_driver:
+    os.str("");
+    os << "detach_action, error getting driver " << vm->get_vmm_mad();
+    goto error_common;
+
+error_common:
+    Nebula              &ne = Nebula::instance();
+    LifeCycleManager *  lcm = ne.get_lcm();
+
+    lcm->trigger(LifeCycleManager::DETACH_FAILURE, vid);
+
+    vm->log("VMM", Log::ERROR, os);
+    vm->unlock();
+    return;
+}
 
 /* ************************************************************************** */
 /* MAD Loading                                                                */

src/vmm/VirtualMachineManagerDriver.cc

@@ -354,6 +354,25 @@ void VirtualMachineManagerDriver::protocol(
             lcm->trigger(LifeCycleManager::ATTACH_FAILURE, id);
         }
     }
+    else if ( action == "DETACH" )
+    {
+        Nebula              &ne  = Nebula::instance();
+        LifeCycleManager    *lcm = ne.get_lcm();
+
+        if ( result == "SUCCESS" )
+        {
+            vm->log("VMM",Log::ERROR,"VM Disk Successfully detached.");
+
+            lcm->trigger(LifeCycleManager::DETACH_SUCCESS, id);
+        }
+        else
+        {
+            log_error(vm,os,is,"Error detaching VM Disk");
+            vmpool->update(vm);
+
+            lcm->trigger(LifeCycleManager::DETACH_FAILURE, id);
+        }
+    }
     else if ( action == "POLL" )
     {
         if (result == "SUCCESS")

src/vmm/XenDriver.cc

@@ -50,6 +50,7 @@ int XenDriver::deployment_description(
     string ro         = "";
     string type       = "";
     string driver     = "";
+    int    disk_id;
     string default_driver = "";
     string mode;
 
@@ -237,6 +238,7 @@ int XenDriver::deployment_description(
         type   = disk->vector_value("TYPE");
         ro     = disk->vector_value("READONLY");
         driver = disk->vector_value("DRIVER");
+        disk->vector_value_str("DISK_ID", disk_id);
 
         if ( target.empty() )
         {
@@ -276,7 +278,7 @@ int XenDriver::deployment_description(
             }
         }
 
-        file << vm->get_remote_system_dir() << "/disk." << i << ","
+        file << vm->get_remote_system_dir() << "/disk." << disk_id << ","
              << target << ","
              << mode
              << "'," << endl;