Refactor operation cases in apply_operation_to_disk() (#746559)

Background:

GParted_Core::calibrate_partition() reloads the partition path name and
boundary to ensure they are correct before the operation is performed.
(See comments in calibrate_partition() for the reasons why this is
necessary).  This also displays details of the partition being modified
in the operation details to inform the user.

The operation object contains these relevant member objects:

  * partition_original
    Partition before the operation is applied.

  * partition_new
    Partition as it is intended to be after the operation has been
    applied.

  * partition_copied (for the copy operation only)
    Source partition being copied.

Issues:

GParted_Core::apply_operation_to_disk() was always calibrating partition
object partition_original, but for about half the operations
partition_original was not used and partition_new is used, so should be
calibrated instead.

Copy into an existing partition calibrated three partitions, the source,
destination before and destination after the operation was applied.
This doesn't really make sense in the operation details to the user.
They would expect to only see the source and destination partitions and
don't care about the distinction between the before and after
representation of the destination.

Minor issues:

The previous fix had to copy the correct partition path from the
calibrated partition_original object to the used partition_new object
for the format, label file system, name partition and change uuid
operations.

Calibrate was called for the create operation too, even though the
partition didn't yet exist.  It was a no-operation.

Fix:

Stop always calibrating the partition_original object and instead
calibrate the correct partition object in each operation case.  For the
copy into existing partition operation only calibrate the right two
partition objects as the user would expect.

Bug 746559 - Various operations fail when following paste into existing
             partition
This commit is contained in:
Mike Fleetwood 2015-03-22 20:41:53 +00:00
parent d9993c21ba
commit cca8a55f51
1 changed files with 89 additions and 56 deletions

View File

@ -739,65 +739,98 @@ bool GParted_Core::snap_to_alignment( const Device & device, Partition & partiti
bool GParted_Core::apply_operation_to_disk( Operation * operation ) bool GParted_Core::apply_operation_to_disk( Operation * operation )
{ {
bool succes = false ; bool success = false;
libparted_messages .clear() ; libparted_messages .clear() ;
if ( calibrate_partition( operation ->partition_original, operation ->operation_detail ) ) switch ( operation->type )
switch ( operation ->type )
{ {
// Call calibrate_partition() first for each operation to ensure the
// correct partition path name and boundary is known before performing the
// actual modifications. (See comments in calibrate_partition() for
// reasons why this is necessary). Calibrate the most relevant partition
// object(s), either partition_original, partition_new or partition_copy,
// as required. Calibrate also displays details of the partition being
// modified in the operation results to inform the user.
case OPERATION_DELETE: case OPERATION_DELETE:
succes = remove_filesystem( operation ->partition_original, operation ->operation_detail ) && success = calibrate_partition( operation->partition_original, operation->operation_detail )
Delete( operation ->partition_original, operation ->operation_detail ) ; && remove_filesystem( operation->partition_original, operation->operation_detail )
break ; && Delete( operation->partition_original, operation->operation_detail );
break;
case OPERATION_CHECK: case OPERATION_CHECK:
succes = check_repair_filesystem( operation ->partition_original, operation ->operation_detail ) && success = calibrate_partition( operation->partition_original, operation->operation_detail )
maximize_filesystem( operation ->partition_original, operation ->operation_detail ) ; && check_repair_filesystem( operation->partition_original,
break ; operation->operation_detail )
&& maximize_filesystem( operation->partition_original, operation->operation_detail );
break;
case OPERATION_CREATE: case OPERATION_CREATE:
succes = create( operation->partition_new, operation->operation_detail ); // The partition doesn't exist yet so there's nothing to calibrate.
break ; success = create( operation->partition_new, operation->operation_detail );
break;
case OPERATION_RESIZE_MOVE: case OPERATION_RESIZE_MOVE:
//in case the to be resized/moved partition was a 'copy of..', we need a real path... success = calibrate_partition( operation->partition_original, operation->operation_detail );
operation ->partition_new .add_path( operation ->partition_original .get_path(), true ) ; if ( ! success )
succes = resize_move( operation ->partition_original, break;
operation ->partition_new,
operation ->operation_detail ); // Reset the new partition object's real path in case the name is
break ; // "copy of ..." from the previous operation.
case OPERATION_FORMAT:
// Reset real path in case the name is "copy of ..." from previous operation
operation->partition_new.add_path( operation->partition_original.get_path(), true ); operation->partition_new.add_path( operation->partition_original.get_path(), true );
succes = remove_filesystem( operation ->partition_original, operation ->operation_detail ) &&
format( operation ->partition_new, operation ->operation_detail ) ; success = resize_move( operation->partition_original,
break ; operation->partition_new,
operation->operation_detail );
break;
case OPERATION_FORMAT:
success = calibrate_partition( operation->partition_new, operation->operation_detail );
if ( ! success )
break;
// Reset the original partition object's real path in case the
// name is "copy of ..." from the previous operation.
operation->partition_original.add_path( operation->partition_new.get_path(), true );
success = remove_filesystem( operation->partition_original, operation->operation_detail )
&& format( operation->partition_new, operation->operation_detail );
break;
case OPERATION_COPY: case OPERATION_COPY:
//FIXME: in case of a new partition we should make sure the new partition is >= the source partition... //FIXME: in case of a new partition we should make sure the new partition is >= the source partition...
//i think it's best to do this in the dialog_paste //i think it's best to do this in the dialog_paste
succes = ( operation ->partition_original .type == TYPE_UNALLOCATED ||
calibrate_partition( operation ->partition_new, operation ->operation_detail ) ) &&
calibrate_partition( static_cast<OperationCopy*>( operation ) ->partition_copied, success = calibrate_partition( static_cast<OperationCopy*>( operation )->partition_copied,
operation ->operation_detail ) && operation->operation_detail )
remove_filesystem( operation ->partition_original, operation ->operation_detail ) && // Only calibrate the destination when pasting into an existing
copy( static_cast<OperationCopy*>( operation ) ->partition_copied, // partition, rather than when creating a new partition.
operation ->partition_new, && ( operation->partition_original.type == TYPE_UNALLOCATED ||
static_cast<OperationCopy*>( operation ) ->partition_copied .get_byte_length(), calibrate_partition( operation->partition_new, operation->operation_detail ) );
operation ->operation_detail ) ; if ( ! success )
break ;
case OPERATION_LABEL_FILESYSTEM:
// Reset real path in case the name is "copy of ..." from previous operation
operation->partition_new.add_path( operation->partition_original.get_path(), true );
succes = label_filesystem( operation->partition_new, operation->operation_detail );
break ;
case OPERATION_NAME_PARTITION:
// Reset real path in case the name is "copy of ..." from previous operation
operation->partition_new.add_path( operation->partition_original.get_path(), true );
succes = name_partition( operation->partition_new, operation->operation_detail );
break; break;
success = remove_filesystem( operation->partition_original, operation->operation_detail )
&& copy( static_cast<OperationCopy*>( operation )->partition_copied,
operation->partition_new,
static_cast<OperationCopy*>( operation )->partition_copied.get_byte_length(),
operation->operation_detail );
break;
case OPERATION_LABEL_FILESYSTEM:
success = calibrate_partition( operation->partition_new, operation->operation_detail )
&& label_filesystem( operation->partition_new, operation->operation_detail );
break;
case OPERATION_NAME_PARTITION:
success = calibrate_partition( operation->partition_new, operation->operation_detail )
&& name_partition( operation->partition_new, operation->operation_detail );
break;
case OPERATION_CHANGE_UUID: case OPERATION_CHANGE_UUID:
// Reset real path in case the name is "copy of ..." from previous operation success = calibrate_partition( operation->partition_new, operation->operation_detail )
operation->partition_new.add_path( operation->partition_original.get_path(), true ); && change_uuid( operation ->partition_new, operation ->operation_detail ) ;
succes = change_uuid( operation ->partition_new, operation ->operation_detail ) ; break;
break ;
} }
if ( libparted_messages .size() > 0 ) if ( libparted_messages .size() > 0 )
@ -809,7 +842,7 @@ bool GParted_Core::apply_operation_to_disk( Operation * operation )
OperationDetail( libparted_messages[ t ], STATUS_NONE, FONT_ITALIC ) ) ; OperationDetail( libparted_messages[ t ], STATUS_NONE, FONT_ITALIC ) ) ;
} }
return succes ; return success;
} }
bool GParted_Core::set_disklabel( const Device & device, const Glib::ustring & disklabel ) bool GParted_Core::set_disklabel( const Device & device, const Glib::ustring & disklabel )