GNOME.org
 

[gparted/psusi/refactor: 7/19] Add proper cancel support (#601239)

commit 1aad113dc1ba066f1f71d84d68399361b01736f9
Author: Phillip Susi <psusi ubuntu com>
Date:   Sun Jan 20 21:50:55 2013 -0500

    Add proper cancel support (#601239)
    
    Interested operations can now connect a signal to their OperationDetail
    to be notified of a cancelation request.  The internal copy/move code
    will now cleanly stop on cancelation, allowing the partition to be
    rolled back to its previous state.  This makes canceling a move
    perfectly safe.
    
    After clicking cancel, the button changes to "Force Cancel" and is
    disabled for 5 seconds.  Operations that are safe to cancel will do so
    and those that are not will continue to run.  Clicking force cancel
    asks operations to cancel, even if doing so is unsafe.  For the
    internal copy/move algorithm, canceling is always safe because an
    error results in a rollback operation.  Canceling the rollback is
    unsafe.  For external commands, filesystem modules may indicate
    that the command is safe to cancel or not.  Canceled commands will
    be terminated with SIGINT.
    
    As a result of the new safe cancel vs force cancel distinction, the
    scary warning about cancl causing corruption has been moved to
    after clicking the force cancel button.
    
    Part of Bug #601239 - Please allow 'Cancel after current operation'

 include/Copy_Blocks.h     |   31 +++++++++-------------
 include/Dialog_Progress.h |    3 ++
 include/FileSystem.h      |    3 +-
 include/GParted_Core.h    |    9 ++++--
 include/OperationDetail.h |    5 +++-
 src/Copy_Blocks.cc        |   63 ++++++++++++++++++++++++++++++++++++--------
 src/Dialog_Progress.cc    |   26 +++++++++++++++++-
 src/FileSystem.cc         |   14 +++++++++-
 src/GParted_Core.cc       |   39 ++++++++++++++++++----------
 src/OperationDetail.cc    |   12 ++++++++
 10 files changed, 153 insertions(+), 52 deletions(-)
---
diff --git a/include/Copy_Blocks.h b/include/Copy_Blocks.h
index c60b9d8..f5e622e 100644
--- a/include/Copy_Blocks.h
+++ b/include/Copy_Blocks.h
@@ -42,25 +42,20 @@ class copy_blocks {
 	Glib::ustring error_message;
 	bool set_progress_info();
 	void copy_thread();
+	bool cancel;
+	bool cancel_safe;
+	void set_cancel( bool force );
 public:
-copy_blocks( const Glib::ustring & in_src_device,
-	     const Glib::ustring & in_dst_device,
-	     Sector src_start,
-	     Sector dst_start,
-	     Byte_Value in_length,
-	     Byte_Value in_blocksize,
-	     OperationDetail & in_operationdetail,
-	     bool in_readonly,
-	     Byte_Value & in_total_done ) :
-	src_device( in_src_device ),
-		dst_device ( in_dst_device ),
-		length ( in_length ),
-		blocksize ( in_blocksize ),
-		operationdetail ( in_operationdetail ),
-		readonly ( in_readonly ),
-		total_done ( in_total_done ),
-		offset_src ( src_start ),
-		offset_dst ( dst_start ) {};
+	copy_blocks( const Glib::ustring & in_src_device,
+		     const Glib::ustring & in_dst_device,
+		     Sector src_start,
+		     Sector dst_start,
+		     Byte_Value in_length,
+		     Byte_Value in_blocksize,
+		     OperationDetail & in_operationdetail,
+		     bool in_readonly,
+		     Byte_Value & in_total_done,
+		     bool cancel_safe );
 	bool copy();
 	void copy_block();
 };
diff --git a/include/Dialog_Progress.h b/include/Dialog_Progress.h
index 12532ce..96ad082 100644
--- a/include/Dialog_Progress.h
+++ b/include/Dialog_Progress.h
@@ -97,6 +97,9 @@ private:
 	unsigned int t, warnings ;
 	sigc::connection pulsetimer;
 	Glib::ustring label_current_sub_text ;
+	unsigned int cancel_countdown;
+	sigc::connection canceltimer;
+	bool cancel_timeout();
 };
 
 }//GParted
diff --git a/include/FileSystem.h b/include/FileSystem.h
index 089a306..c26b3a1 100644
--- a/include/FileSystem.h
+++ b/include/FileSystem.h
@@ -59,7 +59,8 @@ public:
 	virtual bool remove( const Partition & partition, OperationDetail & operationdetail ) = 0 ;
 	bool success;
 protected:
-	int execute_command( const Glib::ustring & command, OperationDetail & operationdetail, bool checkstatus = false );
+	int execute_command( const Glib::ustring & command, OperationDetail & operationdetail,
+			     bool checkstatus = false, bool cancel_safe = false );
 	int execute_command_timed( const Glib::ustring & command, OperationDetail & operationdetail ) {
 		return execute_command( command, operationdetail, true ); }
 	void execute_command_eof();
diff --git a/include/GParted_Core.h b/include/GParted_Core.h
index bea5d88..6eb533e 100644
--- a/include/GParted_Core.h
+++ b/include/GParted_Core.h
@@ -146,11 +146,13 @@ private:
 	bool copy_filesystem( const Partition & partition_src,
 			      const Partition & partition_dst,
 			      OperationDetail & operationdetail,
-			      bool readonly = false ) ;
+			      bool readonly,
+			      bool cancel_safe );
 	bool copy_filesystem( const Partition & partition_src,
 			      const Partition & partition_dst,
 			      OperationDetail & operationdetail,
-			      Byte_Value & total_done ) ;
+			      Byte_Value & total_done,
+			      bool cancel_safe );
 	bool copy_filesystem( const Glib::ustring & src_device,
 			      const Glib::ustring & dst_device,
 			      Sector src_start,
@@ -160,7 +162,8 @@ private:
 			      Byte_Value src_length,
 			      OperationDetail & operationdetail,
 			      bool readonly,
-			      Byte_Value & total_done ) ;
+			      Byte_Value & total_done,
+			      bool cancel_safe ) ;
 	void rollback_transaction( const Partition & partition_src,
 				   const Partition & partition_dst,
 				   OperationDetail & operationdetail,
diff --git a/include/OperationDetail.h b/include/OperationDetail.h
index 159067f..0db4e15 100644
--- a/include/OperationDetail.h
+++ b/include/OperationDetail.h
@@ -48,6 +48,7 @@ class OperationDetail
 {
 public:	
 	OperationDetail() ;
+	~OperationDetail();
 	OperationDetail( const Glib::ustring & description,
 			 OperationDetailStatus status = STATUS_EXECUTE,
 			 Font font = FONT_NORMAL ) ;
@@ -68,9 +69,10 @@ public:
 	Glib::ustring progress_text ;
 	
 	sigc::signal< void, const OperationDetail & > signal_update ;
-
+	sigc::signal< void, bool > signal_cancel;
 private:
 	void on_update( const OperationDetail & operationdetail ) ;
+	void cancel( bool force );
 	
 	Glib::ustring description ;
 	OperationDetailStatus status ; 
@@ -79,6 +81,7 @@ private:
 	
 	std::vector<OperationDetail> sub_details ; 	
 	std::time_t time_start, time_elapsed ;
+	sigc::connection cancelconnection;
 };
 
 } //GParted
diff --git a/src/Copy_Blocks.cc b/src/Copy_Blocks.cc
index eb96933..4c6409c 100644
--- a/src/Copy_Blocks.cc
+++ b/src/Copy_Blocks.cc
@@ -23,6 +23,38 @@
 
 namespace GParted {
 
+void copy_blocks::set_cancel( bool force )
+{
+	if ( force || cancel_safe )
+		cancel = true;
+}
+
+copy_blocks::copy_blocks( const Glib::ustring & in_src_device,
+			  const Glib::ustring & in_dst_device,
+			  Sector src_start,
+			  Sector dst_start,
+			  Byte_Value in_length,
+			  Byte_Value in_blocksize,
+			  OperationDetail & in_operationdetail,
+			  bool in_readonly,
+			  Byte_Value & in_total_done,
+			  bool in_cancel_safe) :
+	src_device( in_src_device ),
+	dst_device ( in_dst_device ),
+	length ( in_length ),
+	blocksize ( in_blocksize ),
+	operationdetail ( in_operationdetail ),
+	readonly ( in_readonly ),
+	total_done ( in_total_done ),
+	offset_src ( src_start ),
+	offset_dst ( dst_start ),
+	cancel( false ),
+	cancel_safe ( in_cancel_safe )
+{
+	operationdetail.signal_cancel.connect(
+		sigc::mem_fun(*this, &copy_blocks::set_cancel));
+}
+
 bool copy_blocks::set_progress_info()
 {
 	Byte_Value done = llabs(this->done);
@@ -49,7 +81,7 @@ bool copy_blocks::set_progress_info()
 				_("%1 of %2 copied"),
 				Utils::format_size( done, 1 ), Utils::format_size( length, 1 ) ),
 				FONT_ITALIC );
-	if (done == length)
+	if (done == length || !success)
 		Gtk::Main::quit();
 	return false;
 }
@@ -73,7 +105,7 @@ void copy_blocks::copy_thread()
 	while( success && llabs( done ) < length )
 	{
 		copy_block();
-		if ( timer_progress_timeout.elapsed() >= 0.5 )
+		if ( timer_progress_timeout .elapsed() >= 0.5 )
 		{
 			Glib::signal_idle().connect( sigc::mem_fun(
 							     *this,
@@ -110,8 +142,8 @@ bool copy_blocks::copy()
 
 	if ( lp_device_src && lp_device_dst && ped_device_open( lp_device_src ) && ped_device_open( lp_device_dst ) )
 	{
-		Byte_Value src_sector_size = lp_device_src ->sector_size ;
-		Byte_Value dst_sector_size = lp_device_dst ->sector_size ;
+		Byte_Value src_sector_size = lp_device_src ->sector_size;
+		Byte_Value dst_sector_size = lp_device_dst ->sector_size;
 
 		//Handle situation where we need to perform the copy beginning
 		//  with the end of the partition and finishing with the start.
@@ -144,13 +176,13 @@ bool copy_blocks::copy()
 		//final description
 		operationdetail.get_last_child().get_last_child().set_description(
 			String::ucompose( readonly ?
-					/*TO TRANSLATORS: looks like  1.00 MiB of 16.00 MiB read */
-					_("%1 of %2 read") :
-					/*TO TRANSLATORS: looks like  1.00 MiB of 16.00 MiB copied */
-					_("%1 of %2 copied"),
-					Utils::format_size( llabs( done ), 1 ),
-					Utils::format_size( length, 1 ) ),
-					FONT_ITALIC );
+					  /*TO TRANSLATORS: looks like  1.00 MiB of 16.00 MiB read */
+					  _("%1 of %2 read") :
+					  /*TO TRANSLATORS: looks like  1.00 MiB of 16.00 MiB copied */
+					  _("%1 of %2 copied"),
+					  Utils::format_size( llabs( done ), 1 ),
+					  Utils::format_size( length, 1 ) ),
+			FONT_ITALIC );
 
 		if ( !success && !error_message.empty() )
 			operationdetail.get_last_child().add_child(
@@ -191,6 +223,13 @@ void copy_blocks::copy_block()
 		offset_dst += (blocksize / sector_size_dst);
 	}
 
+	if ( cancel )
+	{
+		error_message = _("Operation Canceled");
+		success = false;
+		return;
+	}
+
 	if ( blocksize != 0 )
 	{
 		if ( ped_device_read( lp_device_src, buf, offset_src, num_blocks_src ) )
@@ -203,7 +242,7 @@ void copy_blocks::copy_block()
 			}
 		}
 		else
-			error_message = String::ucompose( _("Error while reading block at sector %1"), offset_src );
+			error_message = String::ucompose( _("Error while reading block at sector %1"), offset_src ) ;
 	}
 	if ( blocksize > 0 )
 	{
diff --git a/src/Dialog_Progress.cc b/src/Dialog_Progress.cc
index 4560560..76b070e 100644
--- a/src/Dialog_Progress.cc
+++ b/src/Dialog_Progress.cc
@@ -223,6 +223,7 @@ void Dialog_Progress::on_signal_show()
 	this ->add_button( _("_Save Details"), Gtk::RESPONSE_OK ) ; //there's no enum for SAVE
 	
 	//replace 'cancel' with 'close'
+	canceltimer.disconnect();
 	delete cancelbutton;
 	cancelbutton = 0;
 	this ->add_button( Gtk::Stock::CLOSE, Gtk::RESPONSE_CLOSE );
@@ -292,6 +293,19 @@ void Dialog_Progress::on_cell_data_description( Gtk::CellRenderer * renderer, co
 		static_cast<Gtk::TreeRow>( *iter )[ treeview_operations_columns .operation_description ] ;
 }
 
+bool Dialog_Progress::cancel_timeout()
+{
+	if (--cancel_countdown)
+		cancelbutton->set_label( Glib::ustring::compose("Force Cancel (%1)", cancel_countdown ) );
+	else {
+		cancelbutton->set_label( "Force Cancel" );
+		canceltimer.disconnect();
+		cancelbutton->set_sensitive();
+		return false;
+	}
+	return true;
+}
+
 void Dialog_Progress::on_cancel()
 {
 	Gtk::MessageDialog dialog( *this,
@@ -306,10 +320,18 @@ void Dialog_Progress::on_cancel()
 	dialog .add_button( _("Continue Operation"), Gtk::RESPONSE_NONE ) ;
 	dialog .add_button( _("Cancel Operation"), Gtk::RESPONSE_CANCEL ) ;
 	
-	if ( dialog .run() == Gtk::RESPONSE_CANCEL )
+	if ( !cancel || dialog .run() == Gtk::RESPONSE_CANCEL )
 	{
-		cancel = true;
 		cancelbutton->set_sensitive( false );
+		if (!cancel) {
+			cancel_countdown = 5;
+			cancelbutton->set_label( Glib::ustring::compose("Force Cancel (%1)", cancel_countdown ) );
+			canceltimer = Glib::signal_timeout().connect(
+				sigc::mem_fun(*this, &Dialog_Progress::cancel_timeout), 1000 );
+		}
+		else cancelbutton->set_label( "Force Cancel" );
+		operations[t]->operation_detail.signal_cancel( cancel );
+		cancel = true;
 	}
 }
 
diff --git a/src/FileSystem.cc b/src/FileSystem.cc
index 775d5de..d3ad102 100644
--- a/src/FileSystem.cc
+++ b/src/FileSystem.cc
@@ -66,7 +66,14 @@ static void relay_update( OperationDetail *operationdetail, Glib::ustring *str )
 	operationdetail->set_description( *str, FONT_ITALIC );
 }
 
-int FileSystem::execute_command( const Glib::ustring & command, OperationDetail & operationdetail, bool checkstatus )
+static void cancel_command( bool force, Glib::Pid pid, bool cancel_safe )
+{
+	if( force || cancel_safe )
+		kill( pid, SIGINT );
+}
+
+int FileSystem::execute_command( const Glib::ustring & command, OperationDetail & operationdetail,
+				 bool checkstatus, bool cancel_safe )
 {
 	operationdetail .add_child( OperationDetail( command, checkstatus ? STATUS_EXECUTE : STATUS_NONE, FONT_BOLD_ITALIC ) ) ;
 	Glib::Pid pid;
@@ -112,6 +119,11 @@ int FileSystem::execute_command( const Glib::ustring & command, OperationDetail
 						 &(children[children.size() - 1]),
 						 &error ) );
 
+	operationdetail.get_last_child().signal_cancel.connect(
+		sigc::bind(
+			sigc::ptr_fun( cancel_command ),
+			pid,
+			cancel_safe ));
 	Gtk::Main::run();
 
 	if (checkstatus) {
diff --git a/src/GParted_Core.cc b/src/GParted_Core.cc
index 095a766..431a8bb 100644
--- a/src/GParted_Core.cc
+++ b/src/GParted_Core.cc
@@ -2137,6 +2137,7 @@ bool GParted_Core::move_filesystem( const Partition & partition_old,
 
 	bool succes = false ;
 	FileSystem* p_filesystem = NULL ;
+	Sector total_done = 0;
 	switch ( get_fs( partition_old .filesystem ) .move )
 	{
 		case GParted::FS::NONE:
@@ -2149,11 +2150,11 @@ bool GParted_Core::move_filesystem( const Partition & partition_old,
 				{
 					operationdetail .get_last_child() .add_child( OperationDetail( _("perform real move") ) ) ;
 					
-					Sector total_done ;
 					succes = copy_filesystem( partition_old,
 								  partition_new,
 								  operationdetail .get_last_child() .get_last_child(),
-								  total_done ) ;
+								  total_done,
+								  true ) ;
 					
 					operationdetail .get_last_child() .get_last_child() 
 						.set_status( succes ? STATUS_SUCCES : STATUS_ERROR ) ;
@@ -2169,7 +2170,8 @@ bool GParted_Core::move_filesystem( const Partition & partition_old,
 				}
 			}
 			else
-				succes = copy_filesystem( partition_old, partition_new, operationdetail .get_last_child() ) ;
+				succes = copy_filesystem( partition_old, partition_new, operationdetail .get_last_child(),
+							  total_done, true ) ;
 
 			break ;
 #ifdef HAVE_LIBPARTED_FS_RESIZE
@@ -2600,7 +2602,9 @@ bool GParted_Core::copy( const Partition & partition_src,
 				case GParted::FS::GPARTED :
 						succes = copy_filesystem( partition_src,
 									  partition_dst,
-									  operationdetail .get_last_child() ) ;
+									  operationdetail .get_last_child(),
+									  false,
+									  true ) ;
 						break ;
 
 #ifndef HAVE_LIBPARTED_3_0_0_PLUS
@@ -2646,7 +2650,7 @@ bool GParted_Core::copy_filesystem_simulation( const Partition & partition_src,
 {
 	operationdetail .add_child( OperationDetail( _("perform read-only test") ) ) ;
 	
-	bool succes = copy_filesystem( partition_src, partition_dst, operationdetail .get_last_child(), true ) ;
+	bool succes = copy_filesystem( partition_src, partition_dst, operationdetail .get_last_child(), true, true ) ;
 
 	operationdetail .get_last_child() .set_status( succes ? STATUS_SUCCES : STATUS_ERROR ) ;
 	return succes ;
@@ -2655,7 +2659,8 @@ bool GParted_Core::copy_filesystem_simulation( const Partition & partition_src,
 bool GParted_Core::copy_filesystem( const Partition & partition_src,
 				    const Partition & partition_dst,
 				    OperationDetail & operationdetail,
-				    bool readonly )
+				    bool readonly,
+				    bool cancel_safe )
 {
 	Sector dummy ;
 	return copy_filesystem( partition_src .device_path,
@@ -2667,13 +2672,15 @@ bool GParted_Core::copy_filesystem( const Partition & partition_src,
 				partition_src .get_byte_length(),
 				operationdetail,
 				readonly,
-				dummy ) ;
+				dummy,
+				cancel_safe ) ;
 }
 
 bool GParted_Core::copy_filesystem( const Partition & partition_src,
 				    const Partition & partition_dst,
 				    OperationDetail & operationdetail,
-				    Byte_Value & total_done ) 
+				    Byte_Value & total_done,
+				    bool cancel_safe )
 {
 	return copy_filesystem( partition_src .device_path,
 				partition_dst .device_path,
@@ -2684,7 +2691,8 @@ bool GParted_Core::copy_filesystem( const Partition & partition_src,
 				partition_src .get_byte_length(),
 				operationdetail,
 				false,
-				total_done ) ;
+				total_done,
+				cancel_safe ) ;
 }
 	
 bool GParted_Core::copy_filesystem( const Glib::ustring & src_device,
@@ -2696,7 +2704,8 @@ bool GParted_Core::copy_filesystem( const Glib::ustring & src_device,
 				    Byte_Value src_length,
 				    OperationDetail & operationdetail,
 				    bool readonly,
-				    Byte_Value & total_done ) 
+				    Byte_Value & total_done,
+				    bool cancel_safe )
 {
 	operationdetail .add_child( OperationDetail( _("using internal algorithm"), STATUS_NONE ) ) ;
 	operationdetail .add_child( OperationDetail( 
@@ -2744,7 +2753,8 @@ bool GParted_Core::copy_filesystem( const Glib::ustring & src_device,
 				      benchmark_blocksize,
 				      operationdetail .get_last_child(),
 				      readonly,
-				      total_done ).copy();
+				      total_done,
+				      cancel_safe ).copy();
 		timer.stop() ;
 
 		operationdetail .get_last_child() .get_last_child() .add_child( OperationDetail( 
@@ -2779,7 +2789,8 @@ bool GParted_Core::copy_filesystem( const Glib::ustring & src_device,
 				      optimal_blocksize,
 				      operationdetail,
 				      readonly,
-				      total_done ).copy();
+				      total_done,
+				      cancel_safe ).copy();
 
 	operationdetail .add_child( OperationDetail( 
 		String::ucompose( readonly ?
@@ -2816,7 +2827,7 @@ void GParted_Core::rollback_transaction( const Partition & partition_src,
 		operationdetail.add_child( OperationDetail( _("roll back last transaction") ) );
 
 		//and copy it back (NOTE the reversed dst and src)
-		bool succes = copy_filesystem( temp_dst, temp_src, operationdetail .get_last_child() ) ;
+		bool succes = copy_filesystem( temp_dst, temp_src, operationdetail .get_last_child(), false, false ) ;
 
 		operationdetail .get_last_child() .set_status( succes ? STATUS_SUCCES : STATUS_ERROR ) ;
 	}
@@ -2935,7 +2946,7 @@ bool GParted_Core::set_partition_type( const Partition & partition, OperationDet
 	operationdetail .get_last_child() .set_status( return_value ? STATUS_SUCCES : STATUS_ERROR ) ;
 	return return_value ;
 }
-	
+
 bool GParted_Core::calibrate_partition( Partition & partition, OperationDetail & operationdetail ) 
 {
 	if ( partition .type == TYPE_PRIMARY || partition .type == TYPE_LOGICAL || partition .type == TYPE_EXTENDED )
diff --git a/src/OperationDetail.cc b/src/OperationDetail.cc
index dc7e96c..4c9fa3f 100644
--- a/src/OperationDetail.cc
+++ b/src/OperationDetail.cc
@@ -40,6 +40,11 @@ OperationDetail::OperationDetail( const Glib::ustring & description, OperationDe
 	time_elapsed = -1 ;
 }
 
+OperationDetail::~OperationDetail()
+{
+	cancelconnection.disconnect();
+}
+
 void OperationDetail::set_description( const Glib::ustring & description, Font font )
 {
 	try
@@ -126,6 +131,8 @@ void OperationDetail::add_child( const OperationDetail & operationdetail )
 
 	sub_details .back() .set_treepath( treepath + ":" + Utils::num_to_str( sub_details .size() -1 ) ) ;
 	sub_details .back() .signal_update .connect( sigc::mem_fun( this, &OperationDetail::on_update ) ) ;
+	sub_details.back().cancelconnection = signal_cancel.connect(
+				sigc::mem_fun( &sub_details.back(), &OperationDetail::cancel ) );
 	
 	on_update( sub_details .back() ) ;
 }
@@ -155,4 +162,9 @@ void OperationDetail::on_update( const OperationDetail & operationdetail )
 		signal_update .emit( operationdetail ) ;
 }
 
+void OperationDetail::cancel( bool force )
+{
+	signal_cancel(force);
+}
+
 } //GParted
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]