Package: parted;
Reported by: Phillip Susi <psusi <at> ubuntu.com>
Date: Mon, 6 Jan 2014 04:05:02 UTC
Severity: normal
Tags: patch
Done: Phillip Susi <psusi <at> ubuntu.com>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 16370 in the body.
You can then email your comments to 16370 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
bug-parted <at> gnu.org
:bug#16370
; Package parted
.
(Mon, 06 Jan 2014 04:05:02 GMT) Full text and rfc822 format available.Phillip Susi <psusi <at> ubuntu.com>
:bug-parted <at> gnu.org
.
(Mon, 06 Jan 2014 04:05:03 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: bug-parted <at> gnu.org Subject: [PATCH] parted: don't reload partition table on every command Date: Sun, 5 Jan 2014 23:03:55 -0500
gpt was using a static local variable to suppress repeatedly reporting an error if you chose to ignore it. This is incorrect as the variable is global to all disks, and ignoring the error on one should not suppress its reporting on another. Moving the flag to the PedDisk object made it effectively useless because parted was destroying the PedDisk and reloading the partition table on every command. Parted has been reworked to cache the PedDisk once loaded, and only discard it when changing disks, or creating a new disklabel. --- NEWS | 5 + libparted/labels/gpt.c | 7 +- parted/command.c | 6 +- parted/command.h | 6 +- parted/parted.c | 257 +++++++++++++++++++++++------------------------ parted/ui.c | 8 +- parted/ui.h | 8 +- tests/t3300-palo-prep.sh | 8 +- 8 files changed, 153 insertions(+), 152 deletions(-) diff --git a/NEWS b/NEWS index bc948bd..39f91f3 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,11 @@ GNU parted NEWS -*- outline -*- ** Bug Fixes + libparted: previously if you chose to ignore the warning about + the gpt thinking the disk was smaller than it appears to be on + on disk, subsequent warnings on other disks would be suppressed. + Now parted will warn once per disk. + Fix filesystem detection on non 512 byte sector sizes Fix several bugs with loop labels ( whole disk filesystems ) diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c index dce89b1..75b1ef8 100644 --- a/libparted/labels/gpt.c +++ b/libparted/labels/gpt.c @@ -695,7 +695,6 @@ _parse_header (PedDisk *disk, const GuidPartitionTableHeader_t *gpt, PedSector first_usable; PedSector last_usable; PedSector last_usable_if_grown, last_usable_min_default; - static int asked_already; #ifndef DISCOVER_ONLY if (PED_LE32_TO_CPU (gpt->Revision) > GPT_HEADER_REVISION_V1_02) @@ -742,7 +741,7 @@ _parse_header (PedDisk *disk, const GuidPartitionTableHeader_t *gpt, || disk->dev->length < last_usable_if_grown) return 0; - if (!asked_already && last_usable < last_usable_if_grown) + if (last_usable < last_usable_if_grown) { PedExceptionOption q; @@ -765,10 +764,6 @@ _parse_header (PedDisk *disk, const GuidPartitionTableHeader_t *gpt, last_usable = last_usable_if_grown; *update_needed = 1; } - else if (q != PED_EXCEPTION_UNHANDLED) - { - asked_already = 1; - } } ped_geometry_init (&gpt_disk_data->data_area, disk->dev, diff --git a/parted/command.c b/parted/command.c index e91c0c6..1b84e44 100644 --- a/parted/command.c +++ b/parted/command.c @@ -28,7 +28,7 @@ Command* command_create (const StrList* names, - int (*method) (PedDevice** dev), + int (*method) (PedDevice** dev, PedDisk** disk), const StrList* summary, const StrList* help, const int non_interactive) @@ -134,7 +134,7 @@ command_print_help (Command* cmd) } int -command_run (Command* cmd, PedDevice** dev) +command_run (Command* cmd, PedDevice** dev, PedDisk** disk) { - return cmd->method (dev); + return cmd->method (dev, disk); } diff --git a/parted/command.h b/parted/command.h index fed7a23..6a6a075 100644 --- a/parted/command.h +++ b/parted/command.h @@ -24,14 +24,14 @@ typedef struct { StrList* names; - int (*method) (PedDevice** dev); + int (*method) (PedDevice** dev, PedDisk** disk); StrList* summary; StrList* help; int non_interactive:1; } Command; extern Command* command_create (const StrList* names, - int (*method) (PedDevice** dev), + int (*method) (PedDevice** dev, PedDisk** disk), const StrList* summary, const StrList* help, int non_interactive); @@ -42,6 +42,6 @@ extern Command* command_get (Command** list, char* name); extern StrList* command_get_names (Command** list); extern void command_print_summary (Command* cmd); extern void command_print_help (Command* cmd); -extern int command_run (Command* cmd, PedDevice** dev); +extern int command_run (Command* cmd, PedDevice** dev, PedDisk** disk); #endif /* COMMAND_H_INCLUDED */ diff --git a/parted/parted.c b/parted/parted.c index b20d432..5ff3dc7 100644 --- a/parted/parted.c +++ b/parted/parted.c @@ -176,7 +176,7 @@ static PedTimer* g_timer; static TimerContext timer_context; static int _print_list (); -static void _done (PedDevice* dev); +static void _done (PedDevice* dev, PedDisk *disk); static bool partition_align_check (PedDisk const *disk, PedPartition const *part, enum AlignmentType a_type); @@ -469,7 +469,7 @@ print_options_help () } int -do_help (PedDevice** dev) +do_help (PedDevice** dev, PedDisk** disk) { if (command_line_get_word_count ()) { char* word = command_line_pop_word (); @@ -484,26 +484,28 @@ do_help (PedDevice** dev) } static int -do_mklabel (PedDevice** dev) +do_mklabel (PedDevice** dev, PedDisk** diskp) { PedDisk* disk; const PedDiskType* type = NULL; - ped_exception_fetch_all (); - disk = ped_disk_new (*dev); - if (!disk) ped_exception_catch (); - ped_exception_leave_all (); + if (*diskp) + disk = *diskp; + else { + ped_exception_fetch_all (); + disk = ped_disk_new (*dev); + if (!disk) ped_exception_catch (); + ped_exception_leave_all (); + } if (!command_line_get_disk_type (_("New disk label type?"), &type)) goto error; if (disk) { if (!_disk_warn_busy (disk)) - goto error_destroy_disk; + goto error; if (!opt_script_mode && !_disk_warn_loss (disk)) - goto error_destroy_disk; - - ped_disk_destroy (disk); + goto error; } disk = ped_disk_new_fresh (*dev, type); @@ -512,15 +514,17 @@ do_mklabel (PedDevice** dev) if (!ped_disk_commit (disk)) goto error_destroy_disk; - ped_disk_destroy (disk); if ((*dev)->type != PED_DEVICE_FILE) disk_is_modified = 1; - + if (*diskp) + ped_disk_destroy (*diskp); + *diskp = disk; return 1; error_destroy_disk: ped_disk_destroy (disk); + *diskp = 0; error: return 0; } @@ -598,7 +602,7 @@ _adjust_end_if_iec (PedSector* start, PedSector* end, } static int -do_mkpart (PedDevice** dev) +do_mkpart (PedDevice** dev, PedDisk** diskp) { PedDisk* disk; PedPartition* part; @@ -614,21 +618,26 @@ do_mkpart (PedDevice** dev) char *start_usr = NULL, *end_usr = NULL; char *start_sol = NULL, *end_sol = NULL; - disk = ped_disk_new (*dev); + if (*diskp) + disk = *diskp; + else { + disk = ped_disk_new (*dev); + *diskp = disk; + } if (!disk) goto error; if (ped_disk_is_flag_available(disk, PED_DISK_CYLINDER_ALIGNMENT)) if (!ped_disk_set_flag(disk, PED_DISK_CYLINDER_ALIGNMENT, alignment == ALIGNMENT_CYLINDER)) - goto error_destroy_disk; + goto error; if (!ped_disk_type_check_feature (disk->type, PED_DISK_TYPE_EXTENDED)) { part_type = PED_PARTITION_NORMAL; } else { if (!command_line_get_part_type (_("Partition type?"), disk, &part_type)) - goto error_destroy_disk; + goto error; } /* The undocumented feature that mkpart sometimes takes a @@ -654,15 +663,15 @@ do_mkpart (PedDevice** dev) } else { if (!command_line_get_fs_type (_("File system type?"), &fs_type)) - goto error_destroy_disk; + goto error; } free (peek_word); if (!command_line_get_sector (_("Start?"), *dev, &start, &range_start, NULL)) - goto error_destroy_disk; + goto error; char *end_input; if (!command_line_get_sector (_("End?"), *dev, &end, &range_end, &end_input)) - goto error_destroy_disk; + goto error; _adjust_end_if_iec(&start, &end, range_end, end_input); free(end_input); @@ -670,7 +679,7 @@ do_mkpart (PedDevice** dev) /* processing starts here */ part = ped_partition_new (disk, part_type, fs_type, start, end); if (!part) - goto error_destroy_disk; + goto error; snap_to_boundaries (&part->geom, NULL, disk, range_start, range_end); @@ -778,16 +787,14 @@ do_mkpart (PedDevice** dev) free (part_name); /* avoid double-free upon failure */ part_name = NULL; if (!ped_partition_set_system (part, fs_type)) - goto error_destroy_disk; + goto error; if (ped_partition_is_flag_available (part, PED_PARTITION_LBA)) ped_partition_set_flag (part, PED_PARTITION_LBA, 1); if (!ped_disk_commit (disk)) - goto error_destroy_disk; + goto error; /* clean up */ - ped_disk_destroy (disk); - if (range_start != NULL) ped_geometry_destroy (range_start); if (range_end != NULL) @@ -807,10 +814,8 @@ error_remove_part: ped_disk_remove_partition (disk, part); error_destroy_simple_constraints: ped_partition_destroy (part); -error_destroy_disk: - free (part_name); - ped_disk_destroy (disk); error: + free (part_name); if (range_start != NULL) ped_geometry_destroy (range_start); if (range_end != NULL) @@ -825,36 +830,33 @@ error: } static int -do_name (PedDevice** dev) +do_name (PedDevice** dev, PedDisk** disk) { - PedDisk* disk; PedPartition* part = NULL; char* name; - disk = ped_disk_new (*dev); + if (!*disk) + *disk = ped_disk_new (*dev); if (!disk) goto error; - if (!command_line_get_partition (_("Partition number?"), disk, &part)) - goto error_destroy_disk; + if (!command_line_get_partition (_("Partition number?"), *disk, &part)) + goto error; name = command_line_get_word (_("Partition name?"), ped_partition_get_name (part), NULL, 0); if (!name) - goto error_destroy_disk; + goto error; if (!ped_partition_set_name (part, name)) goto error_free_name; free (name); - if (!ped_disk_commit (disk)) - goto error_destroy_disk; - ped_disk_destroy (disk); + if (!ped_disk_commit (*disk)) + goto error; return 1; error_free_name: free (name); -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } @@ -995,9 +997,8 @@ _print_disk_info (const PedDevice *dev, const PedDisk *disk) } static int -do_print (PedDevice** dev) +do_print (PedDevice** dev, PedDisk** disk) { - PedDisk* disk = NULL; Table* table; int has_extended; int has_name; @@ -1039,22 +1040,23 @@ do_print (PedDevice** dev) } if (!has_devices_arg && !has_list_arg) { - disk = ped_disk_new (*dev); + if (!*disk) + *disk = ped_disk_new (*dev); /* Returning NULL here is an indication of failure, when in script mode. Otherwise (interactive mode) it may indicate a real error, but it may also indicate that the user declined when asked to perform some operation. FIXME: what this really needs is an API change, but a reliable exit code is less important in interactive mode. */ - if (disk == NULL && opt_script_mode) + if (*disk == NULL && opt_script_mode) ok = 0; } - if (disk && - ped_disk_is_flag_available(disk, PED_DISK_CYLINDER_ALIGNMENT)) - if (!ped_disk_set_flag(disk, PED_DISK_CYLINDER_ALIGNMENT, + if (*disk && + ped_disk_is_flag_available(*disk, PED_DISK_CYLINDER_ALIGNMENT)) + if (!ped_disk_set_flag(*disk, PED_DISK_CYLINDER_ALIGNMENT, alignment == ALIGNMENT_CYLINDER)) - goto error_destroy_disk; + return 0; if (has_devices_arg) { char* dev_name; @@ -1087,24 +1089,23 @@ do_print (PedDevice** dev) else if (has_list_arg) return _print_list (); - else if (disk && has_num_arg) { + else if (*disk && has_num_arg) { PedPartition* part = NULL; int status = 0; - if (command_line_get_partition ("", disk, &part)) + if (command_line_get_partition ("", *disk, &part)) status = partition_print (part); - ped_disk_destroy (disk); return status; } - _print_disk_info (*dev, disk); - if (!disk) + _print_disk_info (*dev, *disk); + if (!*disk) goto nopt; if (!opt_machine_mode) putchar ('\n'); - has_extended = ped_disk_type_check_feature (disk->type, + has_extended = ped_disk_type_check_feature ((*disk)->type, PED_DISK_TYPE_EXTENDED); - has_name = ped_disk_type_check_feature (disk->type, + has_name = ped_disk_type_check_feature ((*disk)->type, PED_DISK_TYPE_PARTITION_NAME); PedPartition* part; @@ -1134,8 +1135,8 @@ do_print (PedDevice** dev) table_add_row_from_strlist (table, row1); - for (part = ped_disk_next_partition (disk, NULL); part; - part = ped_disk_next_partition (disk, part)) { + for (part = ped_disk_next_partition (*disk, NULL); part; + part = ped_disk_next_partition (*disk, part)) { if ((!has_free_arg && !ped_partition_is_active(part)) || part->type & PED_PARTITION_METADATA) @@ -1210,8 +1211,8 @@ do_print (PedDevice** dev) } else { - for (part = ped_disk_next_partition (disk, NULL); part; - part = ped_disk_next_partition (disk, part)) { + for (part = ped_disk_next_partition (*disk, NULL); part; + part = ped_disk_next_partition (*disk, part)) { if ((!has_free_arg && !ped_partition_is_active(part)) || part->type & PED_PARTITION_METADATA) @@ -1259,13 +1260,8 @@ do_print (PedDevice** dev) } } - ped_disk_destroy (disk); - return ok; -error_destroy_disk: - ped_disk_destroy (disk); - return 0; nopt: return ok; } @@ -1274,11 +1270,15 @@ static int _print_list () { PedDevice *current_dev = NULL; + PedDisk *disk = 0; ped_device_probe_all(); while ((current_dev = ped_device_get_next(current_dev))) { - do_print (¤t_dev); + do_print (¤t_dev, &disk); + if (disk) + ped_disk_destroy (disk); + disk = 0; putchar ('\n'); } @@ -1286,9 +1286,9 @@ _print_list () } static int -do_quit (PedDevice** dev) +do_quit (PedDevice** dev, PedDisk **disk) { - _done (*dev); + _done (*dev, *disk); exit (EXIT_SUCCESS); } @@ -1434,7 +1434,7 @@ error_remove_partition: } static int -do_rescue (PedDevice** dev) +do_rescue (PedDevice** dev, PedDisk** diskp) { PedDisk* disk; PedSector start = 0, end = 0; @@ -1442,6 +1442,10 @@ do_rescue (PedDevice** dev) PedGeometry probe_start_region; PedGeometry probe_end_region; + if (*diskp) { + ped_disk_destroy (*diskp); + *diskp = 0; + } disk = ped_disk_new (*dev); if (!disk) goto error; @@ -1478,37 +1482,34 @@ error: } static int -do_rm (PedDevice** dev) +do_rm (PedDevice** dev, PedDisk** disk) { - PedDisk* disk; PedPartition* part = NULL; - disk = ped_disk_new (*dev); - if (!disk) + if (!*disk) + *disk = ped_disk_new (*dev); + if (!*disk) goto error; - if (!command_line_get_partition (_("Partition number?"), disk, &part)) - goto error_destroy_disk; + if (!command_line_get_partition (_("Partition number?"), *disk, &part)) + goto error; if (!_partition_warn_busy (part)) - goto error_destroy_disk; + goto error; - ped_disk_delete_partition (disk, part); - ped_disk_commit (disk); - ped_disk_destroy (disk); + ped_disk_delete_partition (*disk, part); + ped_disk_commit (*disk); if ((*dev)->type != PED_DEVICE_FILE) disk_is_modified = 1; return 1; -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } static int -do_select (PedDevice** dev) +do_select (PedDevice** dev, PedDisk** disk) { PedDevice* new_dev = *dev; @@ -1518,6 +1519,10 @@ do_select (PedDevice** dev) return 0; ped_device_close (*dev); + if (*disk) { + ped_disk_destroy (*disk); + *disk = 0; + } *dev = new_dev; print_using_dev (*dev); return 1; @@ -1548,26 +1553,25 @@ partition_align_check (PedDisk const *disk, PedPartition const *part, } static int -do_align_check (PedDevice **dev) +do_align_check (PedDevice **dev, PedDisk** disk) { - PedDisk *disk = ped_disk_new (*dev); - if (!disk) + if (!*disk) + *disk = ped_disk_new (*dev); + if (!*disk) goto error; enum AlignmentType align_type = PA_OPTIMUM; PedPartition *part = NULL; if (!command_line_get_align_type (_("alignment type(min/opt)"), &align_type)) - goto error_destroy_disk; - if (!command_line_get_partition (_("Partition number?"), disk, &part)) - goto error_destroy_disk; + goto error; + if (!command_line_get_partition (_("Partition number?"), *disk, &part)) + goto error; - bool aligned = partition_align_check (disk, part, align_type); + bool aligned = partition_align_check (*disk, part, align_type); if (!opt_script_mode) printf(aligned ? _("%d aligned\n") : _("%d not aligned\n"), part->num); - ped_disk_destroy (disk); - if (opt_script_mode) return aligned ? 1 : 0; @@ -1575,115 +1579,107 @@ do_align_check (PedDevice **dev) with the other modes. */ return 1; -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } static int -do_disk_set (PedDevice** dev) +do_disk_set (PedDevice** dev, PedDisk** disk) { - PedDisk* disk; PedDiskFlag flag; int state; - disk = ped_disk_new (*dev); + if (!*disk) + *disk = ped_disk_new (*dev); if (!disk) goto error; - if (!command_line_get_disk_flag (_("Flag to Invert?"), disk, &flag)) - goto error_destroy_disk; - state = (ped_disk_get_flag (disk, flag) == 0 ? 1 : 0); + if (!command_line_get_disk_flag (_("Flag to Invert?"), *disk, &flag)) + goto error; + state = (ped_disk_get_flag (*disk, flag) == 0 ? 1 : 0); if (!is_toggle_mode) { if (!command_line_get_state (_("New state?"), &state)) - goto error_destroy_disk; + goto error; } - if (!ped_disk_set_flag (disk, flag, state)) - goto error_destroy_disk; - if (!ped_disk_commit (disk)) - goto error_destroy_disk; - ped_disk_destroy (disk); + if (!ped_disk_set_flag (*disk, flag, state)) + goto error; + if (!ped_disk_commit (*disk)) + goto error; if ((*dev)->type != PED_DEVICE_FILE) disk_is_modified = 1; return 1; -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } static int -do_set (PedDevice** dev) +do_set (PedDevice** dev, PedDisk **disk) { - PedDisk* disk; PedPartition* part = NULL; PedPartitionFlag flag; int state; - disk = ped_disk_new (*dev); - if (!disk) + if (*disk == 0) + *disk = ped_disk_new (*dev); + if (!*disk) goto error; - if (!command_line_get_partition (_("Partition number?"), disk, &part)) - goto error_destroy_disk; + if (!command_line_get_partition (_("Partition number?"), *disk, &part)) + goto error; if (!command_line_get_part_flag (_("Flag to Invert?"), part, &flag)) - goto error_destroy_disk; + goto error; state = (ped_partition_get_flag (part, flag) == 0 ? 1 : 0); if (!is_toggle_mode) { if (!command_line_get_state (_("New state?"), &state)) - goto error_destroy_disk; + goto error; } if (!ped_partition_set_flag (part, flag, state)) - goto error_destroy_disk; - if (!ped_disk_commit (disk)) - goto error_destroy_disk; - ped_disk_destroy (disk); + goto error; + if (!ped_disk_commit (*disk)) + goto error; if ((*dev)->type != PED_DEVICE_FILE) disk_is_modified = 1; - return 1; + return 1; -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } static int -do_disk_toggle (PedDevice **dev) +do_disk_toggle (PedDevice **dev, PedDisk** disk) { int result; is_toggle_mode = 1; - result = do_disk_set (dev); + result = do_disk_set (dev, disk); is_toggle_mode = 0; return result; } static int -do_toggle (PedDevice **dev) +do_toggle (PedDevice **dev, PedDisk** disk) { int result; is_toggle_mode = 1; - result = do_set (dev); + result = do_set (dev, disk); is_toggle_mode = 0; return result; } static int -do_unit (PedDevice** dev) +do_unit (PedDevice** dev, PedDisk** disk) { PedUnit unit = ped_unit_get_default (); if (!command_line_get_unit (_("Unit?"), &unit)) @@ -2131,8 +2127,10 @@ return NULL; } static void -_done (PedDevice* dev) +_done (PedDevice* dev, PedDisk* disk) { +if (disk) + ped_disk_destroy (disk); if (dev->boot_dirty && dev->type != PED_DEVICE_FILE) { ped_exception_throw ( PED_EXCEPTION_WARNING, @@ -2159,6 +2157,7 @@ int main (int argc, char** argv) { PedDevice* dev; + PedDisk* disk = 0; int status; set_program_name (argv[0]); @@ -2169,11 +2168,11 @@ main (int argc, char** argv) return 1; if (argc || opt_script_mode) - status = non_interactive_mode (&dev, commands, argc, argv); + status = non_interactive_mode (&dev, &disk, commands, argc, argv); else - status = interactive_mode (&dev, commands); + status = interactive_mode (&dev, &disk, commands); - _done (dev); + _done (dev, disk); return !status; } diff --git a/parted/ui.c b/parted/ui.c index 786deed..e0355ff 100644 --- a/parted/ui.c +++ b/parted/ui.c @@ -1557,7 +1557,7 @@ print_using_dev (PedDevice* dev) } int -interactive_mode (PedDevice** dev, Command* cmd_list[]) +interactive_mode (PedDevice** dev, PedDisk** disk, Command* cmd_list[]) { StrList* list; StrList* command_names = command_get_names (cmd_list); @@ -1590,7 +1590,7 @@ interactive_mode (PedDevice** dev, Command* cmd_list[]) cmd = command_get (commands, word); free (word); if (cmd) { - if (!command_run (cmd, dev)) + if (!command_run (cmd, dev, disk)) command_line_flush (); } else print_commands_help (); @@ -1602,7 +1602,7 @@ interactive_mode (PedDevice** dev, Command* cmd_list[]) int -non_interactive_mode (PedDevice** dev, Command* cmd_list[], +non_interactive_mode (PedDevice** dev, PedDisk **disk, Command* cmd_list[], int argc, char* argv[]) { int i; @@ -1633,7 +1633,7 @@ non_interactive_mode (PedDevice** dev, Command* cmd_list[], goto error; } - if (!command_run (cmd, dev)) + if (!command_run (cmd, dev, disk)) goto error; } return 1; diff --git a/parted/ui.h b/parted/ui.h index 3c6ebc0..260ce37 100644 --- a/parted/ui.h +++ b/parted/ui.h @@ -31,9 +31,11 @@ extern const char *prog_name; extern int init_ui (); extern int init_readline (); -extern int non_interactive_mode (PedDevice** dev, Command* cmd_list[], - int argc, char* argv[]); -extern int interactive_mode (PedDevice** dev, Command* cmd_list[]); +extern int non_interactive_mode (PedDevice** dev, PedDisk **disk, + Command* cmd_list[], int argc, + char* argv[]); +extern int interactive_mode (PedDevice** dev, PedDisk **disk, + Command* cmd_list[]); extern void done_ui (); extern int screen_width (); diff --git a/tests/t3300-palo-prep.sh b/tests/t3300-palo-prep.sh index 4050414..88b2c55 100755 --- a/tests/t3300-palo-prep.sh +++ b/tests/t3300-palo-prep.sh @@ -20,9 +20,9 @@ ss=$sector_size_ cat > exp <<EOF || framework_failure -1:2048s:4095s:2048s:::palo; -1:2048s:4095s:2048s:::prep; -1:2048s:4095s:2048s:::palo; +1:2048s:4095s:2048s:ext2::lba, palo; +1:2048s:4095s:2048s:ext2::lba, prep; +1:2048s:4095s:2048s:ext2::lba, palo; EOF dev=dev-file @@ -37,7 +37,7 @@ parted -m -s $dev mklabel msdos \ set 1 palo on u s print \ > out 2> err || fail=1 -grep -E '^1:2048s:4095s:2048s:::p...;$' out > k; mv k out +grep -E '^1:2048s:4095s:2048s:ext2::lba, p...;$' out > k; mv k out compare exp out || fail=1 -- 1.8.3.2
bug-parted <at> gnu.org
:bug#16370
; Package parted
.
(Mon, 03 Mar 2014 20:08:02 GMT) Full text and rfc822 format available.Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):
From: "Brian C. Lane" <bcl <at> redhat.com> To: bug-parted <at> gnu.org Subject: Re: bug#16370: [PATCH] parted: don't reload partition table on every command Date: Mon, 3 Mar 2014 12:06:43 -0800
On Sun, Jan 05, 2014 at 11:03:55PM -0500, Phillip Susi wrote: > Command* > command_create (const StrList* names, > - int (*method) (PedDevice** dev), > + int (*method) (PedDevice** dev, PedDisk** disk), May as well call the argument diskp everywhere to be consistent. > @@ -1274,11 +1270,15 @@ static int > _print_list () > { > PedDevice *current_dev = NULL; > + PedDisk *disk = 0; Why do you use 0 instead of NULL? To me seeing a 0 makes me think it is supposed to have a value whereas NULL makes it clear it is uninitialized. > - disk = ped_disk_new (*dev); > - if (!disk) > + if (*disk == 0) No need to use == 0 here. > static void > -_done (PedDevice* dev) > +_done (PedDevice* dev, PedDisk* disk) > { > +if (disk) > + ped_disk_destroy (disk); > if (dev->boot_dirty && dev->type != PED_DEVICE_FILE) { > ped_exception_throw ( > PED_EXCEPTION_WARNING, Looks like _done could use some indentation fixes, may as well clean it up while you're changing it. -- Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)
bug-parted <at> gnu.org
:bug#16370
; Package parted
.
(Tue, 04 Mar 2014 00:47:01 GMT) Full text and rfc822 format available.Message #11 received at 16370 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: "Brian C. Lane" <bcl <at> redhat.com>, 16370 <at> debbugs.gnu.org Subject: Re: bug#16370: [PATCH] parted: don't reload partition table on every command Date: Mon, 03 Mar 2014 19:46:21 -0500
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On 03/03/2014 03:06 PM, Brian C. Lane wrote: > On Sun, Jan 05, 2014 at 11:03:55PM -0500, Phillip Susi wrote: >> Command* command_create (const StrList* names, - int (*method) >> (PedDevice** dev), + int (*method) (PedDevice** dev, PedDisk** >> disk), > > May as well call the argument diskp everywhere to be consistent. Good point. >> @@ -1274,11 +1270,15 @@ static int _print_list () { PedDevice >> *current_dev = NULL; + PedDisk *disk = 0; > > Why do you use 0 instead of NULL? To me seeing a 0 makes me think > it is supposed to have a value whereas NULL makes it clear it is > uninitialized. Because programmers are notoriously lazy typists ;) Also at least at some time in the past on certain compilers in C++, NULL could cause errors where 0 wouldn't so I guess I got into the habit. I suppose I can change it since I'm making changes anyhow. >> - disk = ped_disk_new (*dev); - if (!disk) + >> if (*disk == 0) > > No need to use == 0 here. I suppose !*disk works too, though it's a bit less readable. >> static void -_done (PedDevice* dev) +_done (PedDevice* dev, >> PedDisk* disk) { +if (disk) + ped_disk_destroy (disk); if >> (dev->boot_dirty && dev->type != PED_DEVICE_FILE) { >> ped_exception_throw ( PED_EXCEPTION_WARNING, > > Looks like _done could use some indentation fixes, may as well > clean it up while you're changing it. I didn't want the diff being larger than needed, but yea, it should be fixed while I'm in the area. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJTFSJcAAoJEI5FoCIzSKrwdGwH/Rn8XAbq84BGR2i70tdGas1Q sBhQxmUEBgSzdT8GG1GmErZVj3jINNO72TP9e+q6+9Kx5slVvt+mRHgXttN3317s 3fGLgCWBgrgsIkOih6dBqib5qxGYrghcJj3id/5q4K0zF2hREJ89dP53gv1h15vt unpRjfvEsJTpnC/hC5wvLV2pXQqf0LOlSVgJn0isckyGp2SOo8JkmCpLBt+wk+kP qhp1wcjl7Vb0nq9JRJmvdK2ZS8aXJJpuENPFx0v627tRSq8vqmwWukF6TlVTkIqT Ld2x+0pPIVtC8Oomh3aAdv0ZR+R2t3ZI21EWhpJNgC0vyFfG7oI8pjov1swQoAU= =cIMY -----END PGP SIGNATURE-----
bug-parted <at> gnu.org
:bug#16370
; Package parted
.
(Sat, 29 Mar 2014 17:51:01 GMT) Full text and rfc822 format available.Message #14 received at 16370 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: bcl <at> redhat.com Cc: 16370 <at> debbugs.gnu.org Subject: [PATCH] parted: don't reload partition table on every command Date: Sat, 29 Mar 2014 13:50:12 -0400
gpt was using a static local variable to suppress repeatedly reporting an error if you chose to ignore it. This is incorrect as the variable is global to all disks, and ignoring the error on one should not suppress its reporting on another. Moving the flag to the PedDisk object made it effectively useless because parted was destroying the PedDisk and reloading the partition table on every command. Parted has been reworked to cache the PedDisk once loaded, and only discard it when changing disks, or creating a new disklabel. --- NEWS | 5 + libparted/labels/gpt.c | 7 +- parted/command.c | 6 +- parted/command.h | 6 +- parted/parted.c | 291 +++++++++++++++++++++++------------------------ parted/ui.c | 8 +- parted/ui.h | 8 +- tests/t3300-palo-prep.sh | 8 +- 8 files changed, 170 insertions(+), 169 deletions(-) diff --git a/NEWS b/NEWS index 523fd9e..94a201a 100644 --- a/NEWS +++ b/NEWS @@ -23,6 +23,11 @@ GNU parted NEWS -*- outline -*- ** Bug Fixes + libparted: previously if you chose to ignore the warning about + the gpt thinking the disk was smaller than it appears to be on + on disk, subsequent warnings on other disks would be suppressed. + Now parted will warn once per disk. + Fix filesystem detection on non 512 byte sector sizes Fix several bugs with loop labels ( whole disk filesystems ) diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c index 42b0360..b7f2a11 100644 --- a/libparted/labels/gpt.c +++ b/libparted/labels/gpt.c @@ -701,7 +701,6 @@ _parse_header (PedDisk *disk, const GuidPartitionTableHeader_t *gpt, PedSector first_usable; PedSector last_usable; PedSector last_usable_if_grown, last_usable_min_default; - static int asked_already; #ifndef DISCOVER_ONLY if (PED_LE32_TO_CPU (gpt->Revision) > GPT_HEADER_REVISION_V1_02) @@ -748,7 +747,7 @@ _parse_header (PedDisk *disk, const GuidPartitionTableHeader_t *gpt, || disk->dev->length < last_usable_if_grown) return 0; - if (!asked_already && last_usable < last_usable_if_grown) + if (last_usable < last_usable_if_grown) { PedExceptionOption q; @@ -771,10 +770,6 @@ _parse_header (PedDisk *disk, const GuidPartitionTableHeader_t *gpt, last_usable = last_usable_if_grown; *update_needed = 1; } - else if (q != PED_EXCEPTION_UNHANDLED) - { - asked_already = 1; - } } ped_geometry_init (&gpt_disk_data->data_area, disk->dev, diff --git a/parted/command.c b/parted/command.c index e91c0c6..c592248 100644 --- a/parted/command.c +++ b/parted/command.c @@ -28,7 +28,7 @@ Command* command_create (const StrList* names, - int (*method) (PedDevice** dev), + int (*method) (PedDevice** dev, PedDisk** diskp), const StrList* summary, const StrList* help, const int non_interactive) @@ -134,7 +134,7 @@ command_print_help (Command* cmd) } int -command_run (Command* cmd, PedDevice** dev) +command_run (Command* cmd, PedDevice** dev, PedDisk** diskp) { - return cmd->method (dev); + return cmd->method (dev, diskp); } diff --git a/parted/command.h b/parted/command.h index fed7a23..0fe43aa 100644 --- a/parted/command.h +++ b/parted/command.h @@ -24,14 +24,14 @@ typedef struct { StrList* names; - int (*method) (PedDevice** dev); + int (*method) (PedDevice** dev, PedDisk** diskp); StrList* summary; StrList* help; int non_interactive:1; } Command; extern Command* command_create (const StrList* names, - int (*method) (PedDevice** dev), + int (*method) (PedDevice** dev, PedDisk** diskp), const StrList* summary, const StrList* help, int non_interactive); @@ -42,6 +42,6 @@ extern Command* command_get (Command** list, char* name); extern StrList* command_get_names (Command** list); extern void command_print_summary (Command* cmd); extern void command_print_help (Command* cmd); -extern int command_run (Command* cmd, PedDevice** dev); +extern int command_run (Command* cmd, PedDevice** dev, PedDisk** diskp); #endif /* COMMAND_H_INCLUDED */ diff --git a/parted/parted.c b/parted/parted.c index a7d9363..7a53586 100644 --- a/parted/parted.c +++ b/parted/parted.c @@ -178,7 +178,7 @@ static PedTimer* g_timer; static TimerContext timer_context; static int _print_list (); -static void _done (PedDevice* dev); +static void _done (PedDevice* dev, PedDisk *diskp); static bool partition_align_check (PedDisk const *disk, PedPartition const *part, enum AlignmentType a_type); @@ -471,7 +471,7 @@ print_options_help () } int -do_help (PedDevice** dev) +do_help (PedDevice** dev, PedDisk** diskp) { if (command_line_get_word_count ()) { char* word = command_line_pop_word (); @@ -486,26 +486,28 @@ do_help (PedDevice** dev) } static int -do_mklabel (PedDevice** dev) +do_mklabel (PedDevice** dev, PedDisk** diskp) { PedDisk* disk; const PedDiskType* type = NULL; - ped_exception_fetch_all (); - disk = ped_disk_new (*dev); - if (!disk) ped_exception_catch (); - ped_exception_leave_all (); + if (*diskp) + disk = *diskp; + else { + ped_exception_fetch_all (); + disk = ped_disk_new (*dev); + if (!disk) ped_exception_catch (); + ped_exception_leave_all (); + } if (!command_line_get_disk_type (_("New disk label type?"), &type)) goto error; if (disk) { if (!_disk_warn_busy (disk)) - goto error_destroy_disk; + goto error; if (!opt_script_mode && !_disk_warn_loss (disk)) - goto error_destroy_disk; - - ped_disk_destroy (disk); + goto error; } disk = ped_disk_new_fresh (*dev, type); @@ -514,15 +516,17 @@ do_mklabel (PedDevice** dev) if (!ped_disk_commit (disk)) goto error_destroy_disk; - ped_disk_destroy (disk); if ((*dev)->type != PED_DEVICE_FILE) disk_is_modified = 1; - + if (*diskp) + ped_disk_destroy (*diskp); + *diskp = disk; return 1; error_destroy_disk: ped_disk_destroy (disk); + *diskp = 0; error: return 0; } @@ -600,7 +604,7 @@ _adjust_end_if_iec (PedSector* start, PedSector* end, } static int -do_mkpart (PedDevice** dev) +do_mkpart (PedDevice** dev, PedDisk** diskp) { PedDisk* disk; PedPartition* part; @@ -616,21 +620,26 @@ do_mkpart (PedDevice** dev) char *start_usr = NULL, *end_usr = NULL; char *start_sol = NULL, *end_sol = NULL; - disk = ped_disk_new (*dev); + if (*diskp) + disk = *diskp; + else { + disk = ped_disk_new (*dev); + *diskp = disk; + } if (!disk) goto error; if (ped_disk_is_flag_available(disk, PED_DISK_CYLINDER_ALIGNMENT)) if (!ped_disk_set_flag(disk, PED_DISK_CYLINDER_ALIGNMENT, alignment == ALIGNMENT_CYLINDER)) - goto error_destroy_disk; + goto error; if (!ped_disk_type_check_feature (disk->type, PED_DISK_TYPE_EXTENDED)) { part_type = PED_PARTITION_NORMAL; } else { if (!command_line_get_part_type (_("Partition type?"), disk, &part_type)) - goto error_destroy_disk; + goto error; } /* The undocumented feature that mkpart sometimes takes a @@ -656,15 +665,15 @@ do_mkpart (PedDevice** dev) } else { if (!command_line_get_fs_type (_("File system type?"), &fs_type)) - goto error_destroy_disk; + goto error; } free (peek_word); if (!command_line_get_sector (_("Start?"), *dev, &start, &range_start, NULL)) - goto error_destroy_disk; + goto error; char *end_input; if (!command_line_get_sector (_("End?"), *dev, &end, &range_end, &end_input)) - goto error_destroy_disk; + goto error; _adjust_end_if_iec(&start, &end, range_end, end_input); free(end_input); @@ -672,7 +681,7 @@ do_mkpart (PedDevice** dev) /* processing starts here */ part = ped_partition_new (disk, part_type, fs_type, start, end); if (!part) - goto error_destroy_disk; + goto error; snap_to_boundaries (&part->geom, NULL, disk, range_start, range_end); @@ -780,16 +789,14 @@ do_mkpart (PedDevice** dev) free (part_name); /* avoid double-free upon failure */ part_name = NULL; if (!ped_partition_set_system (part, fs_type)) - goto error_destroy_disk; + goto error; if (ped_partition_is_flag_available (part, PED_PARTITION_LBA)) ped_partition_set_flag (part, PED_PARTITION_LBA, 1); if (!ped_disk_commit (disk)) - goto error_destroy_disk; + goto error; /* clean up */ - ped_disk_destroy (disk); - if (range_start != NULL) ped_geometry_destroy (range_start); if (range_end != NULL) @@ -809,10 +816,8 @@ error_remove_part: ped_disk_remove_partition (disk, part); error_destroy_simple_constraints: ped_partition_destroy (part); -error_destroy_disk: - free (part_name); - ped_disk_destroy (disk); error: + free (part_name); if (range_start != NULL) ped_geometry_destroy (range_start); if (range_end != NULL) @@ -827,36 +832,33 @@ error: } static int -do_name (PedDevice** dev) +do_name (PedDevice** dev, PedDisk** diskp) { - PedDisk* disk; PedPartition* part = NULL; char* name; - disk = ped_disk_new (*dev); - if (!disk) + if (!*diskp) + *diskp = ped_disk_new (*dev); + if (!diskp) goto error; - if (!command_line_get_partition (_("Partition number?"), disk, &part)) - goto error_destroy_disk; + if (!command_line_get_partition (_("Partition number?"), *diskp, &part)) + goto error; name = command_line_get_word (_("Partition name?"), ped_partition_get_name (part), NULL, 0); if (!name) - goto error_destroy_disk; + goto error; if (!ped_partition_set_name (part, name)) goto error_free_name; free (name); - if (!ped_disk_commit (disk)) - goto error_destroy_disk; - ped_disk_destroy (disk); + if (!ped_disk_commit (*diskp)) + goto error; return 1; error_free_name: free (name); -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } @@ -943,7 +945,7 @@ _print_disk_geometry (const PedDevice *dev) } static void -_print_disk_info (const PedDevice *dev, const PedDisk *disk) +_print_disk_info (const PedDevice *dev, const PedDisk *diskp) { char const *const transport[] = {"unknown", "scsi", "ide", "dac960", "cpqarray", "file", "ataraid", "i2o", @@ -957,8 +959,8 @@ _print_disk_info (const PedDevice *dev, const PedDisk *disk) - (default_unit == PED_UNIT_CHS || default_unit == PED_UNIT_CYLINDER)); - const char* pt_name = disk ? disk->type->name : "unknown"; - char *disk_flags = disk_print_flags (disk); + const char* pt_name = diskp ? diskp->type->name : "unknown"; + char *disk_flags = disk_print_flags (diskp); if (opt_machine_mode) { switch (default_unit) { @@ -997,9 +999,8 @@ _print_disk_info (const PedDevice *dev, const PedDisk *disk) } static int -do_print (PedDevice** dev) +do_print (PedDevice** dev, PedDisk** diskp) { - PedDisk* disk = NULL; Table* table; int has_extended; int has_name; @@ -1041,22 +1042,23 @@ do_print (PedDevice** dev) } if (!has_devices_arg && !has_list_arg) { - disk = ped_disk_new (*dev); + if (!*diskp) + *diskp = ped_disk_new (*dev); /* Returning NULL here is an indication of failure, when in script mode. Otherwise (interactive mode) it may indicate a real error, but it may also indicate that the user declined when asked to perform some operation. FIXME: what this really needs is an API change, but a reliable exit code is less important in interactive mode. */ - if (disk == NULL && opt_script_mode) + if (*diskp == NULL && opt_script_mode) ok = 0; } - if (disk && - ped_disk_is_flag_available(disk, PED_DISK_CYLINDER_ALIGNMENT)) - if (!ped_disk_set_flag(disk, PED_DISK_CYLINDER_ALIGNMENT, + if (*diskp && + ped_disk_is_flag_available(*diskp, PED_DISK_CYLINDER_ALIGNMENT)) + if (!ped_disk_set_flag(*diskp, PED_DISK_CYLINDER_ALIGNMENT, alignment == ALIGNMENT_CYLINDER)) - goto error_destroy_disk; + return 0; if (has_devices_arg) { char* dev_name; @@ -1089,24 +1091,23 @@ do_print (PedDevice** dev) else if (has_list_arg) return _print_list (); - else if (disk && has_num_arg) { + else if (*diskp && has_num_arg) { PedPartition* part = NULL; int status = 0; - if (command_line_get_partition ("", disk, &part)) + if (command_line_get_partition ("", *diskp, &part)) status = partition_print (part); - ped_disk_destroy (disk); return status; } - _print_disk_info (*dev, disk); - if (!disk) + _print_disk_info (*dev, *diskp); + if (!*diskp) goto nopt; if (!opt_machine_mode) putchar ('\n'); - has_extended = ped_disk_type_check_feature (disk->type, + has_extended = ped_disk_type_check_feature ((*diskp)->type, PED_DISK_TYPE_EXTENDED); - has_name = ped_disk_type_check_feature (disk->type, + has_name = ped_disk_type_check_feature ((*diskp)->type, PED_DISK_TYPE_PARTITION_NAME); PedPartition* part; @@ -1136,8 +1137,8 @@ do_print (PedDevice** dev) table_add_row_from_strlist (table, row1); - for (part = ped_disk_next_partition (disk, NULL); part; - part = ped_disk_next_partition (disk, part)) { + for (part = ped_disk_next_partition (*diskp, NULL); part; + part = ped_disk_next_partition (*diskp, part)) { if ((!has_free_arg && !ped_partition_is_active(part)) || part->type & PED_PARTITION_METADATA) @@ -1212,8 +1213,8 @@ do_print (PedDevice** dev) } else { - for (part = ped_disk_next_partition (disk, NULL); part; - part = ped_disk_next_partition (disk, part)) { + for (part = ped_disk_next_partition (*diskp, NULL); part; + part = ped_disk_next_partition (*diskp, part)) { if ((!has_free_arg && !ped_partition_is_active(part)) || part->type & PED_PARTITION_METADATA) @@ -1261,13 +1262,8 @@ do_print (PedDevice** dev) } } - ped_disk_destroy (disk); - return ok; -error_destroy_disk: - ped_disk_destroy (disk); - return 0; nopt: return ok; } @@ -1276,11 +1272,15 @@ static int _print_list () { PedDevice *current_dev = NULL; + PedDisk *diskp = NULL; ped_device_probe_all(); while ((current_dev = ped_device_get_next(current_dev))) { - do_print (¤t_dev); + do_print (¤t_dev, &diskp); + if (diskp) + ped_disk_destroy (diskp); + diskp = 0; putchar ('\n'); } @@ -1288,9 +1288,9 @@ _print_list () } static int -do_quit (PedDevice** dev) +do_quit (PedDevice** dev, PedDisk **diskp) { - _done (*dev); + _done (*dev, *diskp); exit (EXIT_SUCCESS); } @@ -1436,7 +1436,7 @@ error_remove_partition: } static int -do_rescue (PedDevice** dev) +do_rescue (PedDevice** dev, PedDisk** diskp) { PedDisk* disk; PedSector start = 0, end = 0; @@ -1444,6 +1444,10 @@ do_rescue (PedDevice** dev) PedGeometry probe_start_region; PedGeometry probe_end_region; + if (*diskp) { + ped_disk_destroy (*diskp); + *diskp = 0; + } disk = ped_disk_new (*dev); if (!disk) goto error; @@ -1480,37 +1484,34 @@ error: } static int -do_rm (PedDevice** dev) +do_rm (PedDevice** dev, PedDisk** diskp) { - PedDisk* disk; PedPartition* part = NULL; - disk = ped_disk_new (*dev); - if (!disk) + if (!*diskp) + *diskp = ped_disk_new (*dev); + if (!*diskp) goto error; - if (!command_line_get_partition (_("Partition number?"), disk, &part)) - goto error_destroy_disk; + if (!command_line_get_partition (_("Partition number?"), *diskp, &part)) + goto error; if (!_partition_warn_busy (part)) - goto error_destroy_disk; + goto error; - ped_disk_delete_partition (disk, part); - ped_disk_commit (disk); - ped_disk_destroy (disk); + ped_disk_delete_partition (*diskp, part); + ped_disk_commit (*diskp); if ((*dev)->type != PED_DEVICE_FILE) disk_is_modified = 1; return 1; -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } static int -do_select (PedDevice** dev) +do_select (PedDevice** dev, PedDisk** diskp) { PedDevice* new_dev = *dev; @@ -1520,6 +1521,10 @@ do_select (PedDevice** dev) return 0; ped_device_close (*dev); + if (*diskp) { + ped_disk_destroy (*diskp); + *diskp = 0; + } *dev = new_dev; print_using_dev (*dev); return 1; @@ -1550,26 +1555,25 @@ partition_align_check (PedDisk const *disk, PedPartition const *part, } static int -do_align_check (PedDevice **dev) +do_align_check (PedDevice **dev, PedDisk** diskp) { - PedDisk *disk = ped_disk_new (*dev); - if (!disk) + if (!*diskp) + *diskp = ped_disk_new (*dev); + if (!*diskp) goto error; enum AlignmentType align_type = PA_OPTIMUM; PedPartition *part = NULL; if (!command_line_get_align_type (_("alignment type(min/opt)"), &align_type)) - goto error_destroy_disk; - if (!command_line_get_partition (_("Partition number?"), disk, &part)) - goto error_destroy_disk; + goto error; + if (!command_line_get_partition (_("Partition number?"), *diskp, &part)) + goto error; - bool aligned = partition_align_check (disk, part, align_type); + bool aligned = partition_align_check (*diskp, part, align_type); if (!opt_script_mode) printf(aligned ? _("%d aligned\n") : _("%d not aligned\n"), part->num); - ped_disk_destroy (disk); - if (opt_script_mode) return aligned ? 1 : 0; @@ -1577,115 +1581,107 @@ do_align_check (PedDevice **dev) with the other modes. */ return 1; -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } static int -do_disk_set (PedDevice** dev) +do_disk_set (PedDevice** dev, PedDisk** diskp) { - PedDisk* disk; PedDiskFlag flag; int state; - disk = ped_disk_new (*dev); - if (!disk) + if (!*diskp) + *diskp = ped_disk_new (*dev); + if (!diskp) goto error; - if (!command_line_get_disk_flag (_("Flag to Invert?"), disk, &flag)) - goto error_destroy_disk; - state = (ped_disk_get_flag (disk, flag) == 0 ? 1 : 0); + if (!command_line_get_disk_flag (_("Flag to Invert?"), *diskp, &flag)) + goto error; + state = (ped_disk_get_flag (*diskp, flag) == 0 ? 1 : 0); if (!is_toggle_mode) { if (!command_line_get_state (_("New state?"), &state)) - goto error_destroy_disk; + goto error; } - if (!ped_disk_set_flag (disk, flag, state)) - goto error_destroy_disk; - if (!ped_disk_commit (disk)) - goto error_destroy_disk; - ped_disk_destroy (disk); + if (!ped_disk_set_flag (*diskp, flag, state)) + goto error; + if (!ped_disk_commit (*diskp)) + goto error; if ((*dev)->type != PED_DEVICE_FILE) disk_is_modified = 1; return 1; -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } static int -do_set (PedDevice** dev) +do_set (PedDevice** dev, PedDisk **diskp) { - PedDisk* disk; PedPartition* part = NULL; PedPartitionFlag flag; int state; - disk = ped_disk_new (*dev); - if (!disk) + if (*diskp == 0) + *diskp = ped_disk_new (*dev); + if (!*diskp) goto error; - if (!command_line_get_partition (_("Partition number?"), disk, &part)) - goto error_destroy_disk; + if (!command_line_get_partition (_("Partition number?"), *diskp, &part)) + goto error; if (!command_line_get_part_flag (_("Flag to Invert?"), part, &flag)) - goto error_destroy_disk; + goto error; state = (ped_partition_get_flag (part, flag) == 0 ? 1 : 0); if (!is_toggle_mode) { if (!command_line_get_state (_("New state?"), &state)) - goto error_destroy_disk; + goto error; } if (!ped_partition_set_flag (part, flag, state)) - goto error_destroy_disk; - if (!ped_disk_commit (disk)) - goto error_destroy_disk; - ped_disk_destroy (disk); + goto error; + if (!ped_disk_commit (*diskp)) + goto error; if ((*dev)->type != PED_DEVICE_FILE) disk_is_modified = 1; - return 1; + return 1; -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } static int -do_disk_toggle (PedDevice **dev) +do_disk_toggle (PedDevice **dev, PedDisk** diskp) { int result; is_toggle_mode = 1; - result = do_disk_set (dev); + result = do_disk_set (dev, diskp); is_toggle_mode = 0; return result; } static int -do_toggle (PedDevice **dev) +do_toggle (PedDevice **dev, PedDisk** diskp) { int result; is_toggle_mode = 1; - result = do_set (dev); + result = do_set (dev, diskp); is_toggle_mode = 0; return result; } static int -do_unit (PedDevice** dev) +do_unit (PedDevice** dev, PedDisk** diskp) { PedUnit unit = ped_unit_get_default (); if (!command_line_get_unit (_("Unit?"), &unit)) @@ -2150,20 +2146,22 @@ return NULL; } static void -_done (PedDevice* dev) +_done (PedDevice* dev, PedDisk* diskp) { -if (dev->boot_dirty && dev->type != PED_DEVICE_FILE) { - ped_exception_throw ( - PED_EXCEPTION_WARNING, - PED_EXCEPTION_OK, - _("You should reinstall your boot loader before " - "rebooting. Read section 4 of the Parted User " - "documentation for more information.")); -} -if (!opt_script_mode && !opt_machine_mode && disk_is_modified) { - ped_exception_throw ( - PED_EXCEPTION_INFORMATION, PED_EXCEPTION_OK, - _("You may need to update /etc/fstab.\n")); + if (diskp) + ped_disk_destroy (diskp); + if (dev->boot_dirty && dev->type != PED_DEVICE_FILE) { + ped_exception_throw ( + PED_EXCEPTION_WARNING, + PED_EXCEPTION_OK, + _("You should reinstall your boot loader before " + "rebooting. Read section 4 of the Parted User " + "documentation for more information.")); + } + if (!opt_script_mode && !opt_machine_mode && disk_is_modified) { + ped_exception_throw ( + PED_EXCEPTION_INFORMATION, PED_EXCEPTION_OK, + _("You may need to update /etc/fstab.\n")); } ped_device_close (dev); @@ -2178,6 +2176,7 @@ int main (int argc, char** argv) { PedDevice* dev; + PedDisk* diskp = 0; int status; set_program_name (argv[0]); @@ -2188,11 +2187,11 @@ main (int argc, char** argv) return 1; if (argc || opt_script_mode) - status = non_interactive_mode (&dev, commands, argc, argv); + status = non_interactive_mode (&dev, &diskp, commands, argc, argv); else - status = interactive_mode (&dev, commands); + status = interactive_mode (&dev, &diskp, commands); - _done (dev); + _done (dev, diskp); return !status; } diff --git a/parted/ui.c b/parted/ui.c index b33f6fc..925110d 100644 --- a/parted/ui.c +++ b/parted/ui.c @@ -1557,7 +1557,7 @@ print_using_dev (PedDevice* dev) } int -interactive_mode (PedDevice** dev, Command* cmd_list[]) +interactive_mode (PedDevice** dev, PedDisk** disk, Command* cmd_list[]) { StrList* list; StrList* command_names = command_get_names (cmd_list); @@ -1590,7 +1590,7 @@ interactive_mode (PedDevice** dev, Command* cmd_list[]) cmd = command_get (commands, word); free (word); if (cmd) { - if (!command_run (cmd, dev)) + if (!command_run (cmd, dev, disk)) command_line_flush (); } else print_commands_help (); @@ -1602,7 +1602,7 @@ interactive_mode (PedDevice** dev, Command* cmd_list[]) int -non_interactive_mode (PedDevice** dev, Command* cmd_list[], +non_interactive_mode (PedDevice** dev, PedDisk **disk, Command* cmd_list[], int argc, char* argv[]) { int i; @@ -1633,7 +1633,7 @@ non_interactive_mode (PedDevice** dev, Command* cmd_list[], goto error; } - if (!command_run (cmd, dev)) + if (!command_run (cmd, dev, disk)) goto error; } return 1; diff --git a/parted/ui.h b/parted/ui.h index 3c6ebc0..260ce37 100644 --- a/parted/ui.h +++ b/parted/ui.h @@ -31,9 +31,11 @@ extern const char *prog_name; extern int init_ui (); extern int init_readline (); -extern int non_interactive_mode (PedDevice** dev, Command* cmd_list[], - int argc, char* argv[]); -extern int interactive_mode (PedDevice** dev, Command* cmd_list[]); +extern int non_interactive_mode (PedDevice** dev, PedDisk **disk, + Command* cmd_list[], int argc, + char* argv[]); +extern int interactive_mode (PedDevice** dev, PedDisk **disk, + Command* cmd_list[]); extern void done_ui (); extern int screen_width (); diff --git a/tests/t3300-palo-prep.sh b/tests/t3300-palo-prep.sh index 4050414..88b2c55 100755 --- a/tests/t3300-palo-prep.sh +++ b/tests/t3300-palo-prep.sh @@ -20,9 +20,9 @@ ss=$sector_size_ cat > exp <<EOF || framework_failure -1:2048s:4095s:2048s:::palo; -1:2048s:4095s:2048s:::prep; -1:2048s:4095s:2048s:::palo; +1:2048s:4095s:2048s:ext2::lba, palo; +1:2048s:4095s:2048s:ext2::lba, prep; +1:2048s:4095s:2048s:ext2::lba, palo; EOF dev=dev-file @@ -37,7 +37,7 @@ parted -m -s $dev mklabel msdos \ set 1 palo on u s print \ > out 2> err || fail=1 -grep -E '^1:2048s:4095s:2048s:::p...;$' out > k; mv k out +grep -E '^1:2048s:4095s:2048s:ext2::lba, p...;$' out > k; mv k out compare exp out || fail=1 -- 1.8.3.2
bug-parted <at> gnu.org
:bug#16370
; Package parted
.
(Thu, 17 Apr 2014 22:14:02 GMT) Full text and rfc822 format available.Message #17 received at 16370 <at> debbugs.gnu.org (full text, mbox):
From: "Brian C. Lane" <bcl <at> redhat.com> To: Phillip Susi <psusi <at> ubuntu.com> Cc: 16370 <at> debbugs.gnu.org Subject: Re: [PATCH] parted: don't reload partition table on every command Date: Thu, 17 Apr 2014 15:13:07 -0700
On Sat, Mar 29, 2014 at 01:50:12PM -0400, Phillip Susi wrote: > gpt was using a static local variable to suppress repeatedly reporting > an error if you chose to ignore it. This is incorrect as the variable is > global to all disks, and ignoring the error on one should not suppress its > reporting on another. Moving the flag to the PedDisk object made it > effectively useless because parted was destroying the PedDisk and reloading > the partition table on every command. > > Parted has been reworked to cache the PedDisk once loaded, and only discard > it when changing disks, or creating a new disklabel. > --- > NEWS | 5 + > libparted/labels/gpt.c | 7 +- > parted/command.c | 6 +- > parted/command.h | 6 +- > parted/parted.c | 291 +++++++++++++++++++++++------------------------ > parted/ui.c | 8 +- > parted/ui.h | 8 +- > tests/t3300-palo-prep.sh | 8 +- > 8 files changed, 170 insertions(+), 169 deletions(-) This makes t0283 start failing, it stops showing the error about overlapping partitions. -- Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)
bug-parted <at> gnu.org
:bug#16370
; Package parted
.
(Fri, 18 Apr 2014 22:56:01 GMT) Full text and rfc822 format available.Message #20 received at 16370 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16370 <at> debbugs.gnu.org Cc: bcl <at> redhat.com Subject: [PATCH] parted: don't reload partition table on every command Date: Fri, 18 Apr 2014 18:55:06 -0400
I could have sworn I fixed that test before, but here it is 8<---------------------------->8 gpt was using a static local variable to suppress repeatedly reporting an error if you chose to ignore it. This is incorrect as the variable is global to all disks, and ignoring the error on one should not suppress its reporting on another. Moving the flag to the PedDisk object made it effectively useless because parted was destroying the PedDisk and reloading the partition table on every command. Parted has been reworked to cache the PedDisk once loaded, and only discard it when changing disks, or creating a new disklabel. --- NEWS | 5 + libparted/labels/gpt.c | 7 +- parted/command.c | 6 +- parted/command.h | 6 +- parted/parted.c | 291 +++++++++++++++++++------------------- parted/ui.c | 8 +- parted/ui.h | 8 +- tests/t0283-overlap-partitions.sh | 6 - tests/t3300-palo-prep.sh | 8 +- 9 files changed, 170 insertions(+), 175 deletions(-) diff --git a/NEWS b/NEWS index ae65106..61a896c 100644 --- a/NEWS +++ b/NEWS @@ -23,6 +23,11 @@ GNU parted NEWS -*- outline -*- ** Bug Fixes + libparted: previously if you chose to ignore the warning about + the gpt thinking the disk was smaller than it appears to be on + on disk, subsequent warnings on other disks would be suppressed. + Now parted will warn once per disk. + Fix filesystem detection on non 512 byte sector sizes Fix linux partition sync code to flush partitions > 16 diff --git a/libparted/labels/gpt.c b/libparted/labels/gpt.c index 5c8df59..5e95f6f 100644 --- a/libparted/labels/gpt.c +++ b/libparted/labels/gpt.c @@ -724,7 +724,6 @@ _parse_header (PedDisk *disk, const GuidPartitionTableHeader_t *gpt, PedSector first_usable; PedSector last_usable; PedSector last_usable_if_grown; - static int asked_already; #ifndef DISCOVER_ONLY if (PED_LE32_TO_CPU (gpt->Revision) > GPT_HEADER_REVISION_V1_02) @@ -761,7 +760,7 @@ _parse_header (PedDisk *disk, const GuidPartitionTableHeader_t *gpt, || disk->dev->length < last_usable_if_grown) return 0; - if (!asked_already && last_usable < last_usable_if_grown) + if (last_usable < last_usable_if_grown) { PedExceptionOption q; @@ -783,10 +782,6 @@ _parse_header (PedDisk *disk, const GuidPartitionTableHeader_t *gpt, gpt_disk_data->AlternateLBA = disk->dev->length - 1; *update_needed = 1; } - else if (q != PED_EXCEPTION_UNHANDLED) - { - asked_already = 1; - } } ped_geometry_init (&gpt_disk_data->data_area, disk->dev, diff --git a/parted/command.c b/parted/command.c index e91c0c6..c592248 100644 --- a/parted/command.c +++ b/parted/command.c @@ -28,7 +28,7 @@ Command* command_create (const StrList* names, - int (*method) (PedDevice** dev), + int (*method) (PedDevice** dev, PedDisk** diskp), const StrList* summary, const StrList* help, const int non_interactive) @@ -134,7 +134,7 @@ command_print_help (Command* cmd) } int -command_run (Command* cmd, PedDevice** dev) +command_run (Command* cmd, PedDevice** dev, PedDisk** diskp) { - return cmd->method (dev); + return cmd->method (dev, diskp); } diff --git a/parted/command.h b/parted/command.h index fed7a23..0fe43aa 100644 --- a/parted/command.h +++ b/parted/command.h @@ -24,14 +24,14 @@ typedef struct { StrList* names; - int (*method) (PedDevice** dev); + int (*method) (PedDevice** dev, PedDisk** diskp); StrList* summary; StrList* help; int non_interactive:1; } Command; extern Command* command_create (const StrList* names, - int (*method) (PedDevice** dev), + int (*method) (PedDevice** dev, PedDisk** diskp), const StrList* summary, const StrList* help, int non_interactive); @@ -42,6 +42,6 @@ extern Command* command_get (Command** list, char* name); extern StrList* command_get_names (Command** list); extern void command_print_summary (Command* cmd); extern void command_print_help (Command* cmd); -extern int command_run (Command* cmd, PedDevice** dev); +extern int command_run (Command* cmd, PedDevice** dev, PedDisk** diskp); #endif /* COMMAND_H_INCLUDED */ diff --git a/parted/parted.c b/parted/parted.c index a7d9363..7a53586 100644 --- a/parted/parted.c +++ b/parted/parted.c @@ -178,7 +178,7 @@ static PedTimer* g_timer; static TimerContext timer_context; static int _print_list (); -static void _done (PedDevice* dev); +static void _done (PedDevice* dev, PedDisk *diskp); static bool partition_align_check (PedDisk const *disk, PedPartition const *part, enum AlignmentType a_type); @@ -471,7 +471,7 @@ print_options_help () } int -do_help (PedDevice** dev) +do_help (PedDevice** dev, PedDisk** diskp) { if (command_line_get_word_count ()) { char* word = command_line_pop_word (); @@ -486,26 +486,28 @@ do_help (PedDevice** dev) } static int -do_mklabel (PedDevice** dev) +do_mklabel (PedDevice** dev, PedDisk** diskp) { PedDisk* disk; const PedDiskType* type = NULL; - ped_exception_fetch_all (); - disk = ped_disk_new (*dev); - if (!disk) ped_exception_catch (); - ped_exception_leave_all (); + if (*diskp) + disk = *diskp; + else { + ped_exception_fetch_all (); + disk = ped_disk_new (*dev); + if (!disk) ped_exception_catch (); + ped_exception_leave_all (); + } if (!command_line_get_disk_type (_("New disk label type?"), &type)) goto error; if (disk) { if (!_disk_warn_busy (disk)) - goto error_destroy_disk; + goto error; if (!opt_script_mode && !_disk_warn_loss (disk)) - goto error_destroy_disk; - - ped_disk_destroy (disk); + goto error; } disk = ped_disk_new_fresh (*dev, type); @@ -514,15 +516,17 @@ do_mklabel (PedDevice** dev) if (!ped_disk_commit (disk)) goto error_destroy_disk; - ped_disk_destroy (disk); if ((*dev)->type != PED_DEVICE_FILE) disk_is_modified = 1; - + if (*diskp) + ped_disk_destroy (*diskp); + *diskp = disk; return 1; error_destroy_disk: ped_disk_destroy (disk); + *diskp = 0; error: return 0; } @@ -600,7 +604,7 @@ _adjust_end_if_iec (PedSector* start, PedSector* end, } static int -do_mkpart (PedDevice** dev) +do_mkpart (PedDevice** dev, PedDisk** diskp) { PedDisk* disk; PedPartition* part; @@ -616,21 +620,26 @@ do_mkpart (PedDevice** dev) char *start_usr = NULL, *end_usr = NULL; char *start_sol = NULL, *end_sol = NULL; - disk = ped_disk_new (*dev); + if (*diskp) + disk = *diskp; + else { + disk = ped_disk_new (*dev); + *diskp = disk; + } if (!disk) goto error; if (ped_disk_is_flag_available(disk, PED_DISK_CYLINDER_ALIGNMENT)) if (!ped_disk_set_flag(disk, PED_DISK_CYLINDER_ALIGNMENT, alignment == ALIGNMENT_CYLINDER)) - goto error_destroy_disk; + goto error; if (!ped_disk_type_check_feature (disk->type, PED_DISK_TYPE_EXTENDED)) { part_type = PED_PARTITION_NORMAL; } else { if (!command_line_get_part_type (_("Partition type?"), disk, &part_type)) - goto error_destroy_disk; + goto error; } /* The undocumented feature that mkpart sometimes takes a @@ -656,15 +665,15 @@ do_mkpart (PedDevice** dev) } else { if (!command_line_get_fs_type (_("File system type?"), &fs_type)) - goto error_destroy_disk; + goto error; } free (peek_word); if (!command_line_get_sector (_("Start?"), *dev, &start, &range_start, NULL)) - goto error_destroy_disk; + goto error; char *end_input; if (!command_line_get_sector (_("End?"), *dev, &end, &range_end, &end_input)) - goto error_destroy_disk; + goto error; _adjust_end_if_iec(&start, &end, range_end, end_input); free(end_input); @@ -672,7 +681,7 @@ do_mkpart (PedDevice** dev) /* processing starts here */ part = ped_partition_new (disk, part_type, fs_type, start, end); if (!part) - goto error_destroy_disk; + goto error; snap_to_boundaries (&part->geom, NULL, disk, range_start, range_end); @@ -780,16 +789,14 @@ do_mkpart (PedDevice** dev) free (part_name); /* avoid double-free upon failure */ part_name = NULL; if (!ped_partition_set_system (part, fs_type)) - goto error_destroy_disk; + goto error; if (ped_partition_is_flag_available (part, PED_PARTITION_LBA)) ped_partition_set_flag (part, PED_PARTITION_LBA, 1); if (!ped_disk_commit (disk)) - goto error_destroy_disk; + goto error; /* clean up */ - ped_disk_destroy (disk); - if (range_start != NULL) ped_geometry_destroy (range_start); if (range_end != NULL) @@ -809,10 +816,8 @@ error_remove_part: ped_disk_remove_partition (disk, part); error_destroy_simple_constraints: ped_partition_destroy (part); -error_destroy_disk: - free (part_name); - ped_disk_destroy (disk); error: + free (part_name); if (range_start != NULL) ped_geometry_destroy (range_start); if (range_end != NULL) @@ -827,36 +832,33 @@ error: } static int -do_name (PedDevice** dev) +do_name (PedDevice** dev, PedDisk** diskp) { - PedDisk* disk; PedPartition* part = NULL; char* name; - disk = ped_disk_new (*dev); - if (!disk) + if (!*diskp) + *diskp = ped_disk_new (*dev); + if (!diskp) goto error; - if (!command_line_get_partition (_("Partition number?"), disk, &part)) - goto error_destroy_disk; + if (!command_line_get_partition (_("Partition number?"), *diskp, &part)) + goto error; name = command_line_get_word (_("Partition name?"), ped_partition_get_name (part), NULL, 0); if (!name) - goto error_destroy_disk; + goto error; if (!ped_partition_set_name (part, name)) goto error_free_name; free (name); - if (!ped_disk_commit (disk)) - goto error_destroy_disk; - ped_disk_destroy (disk); + if (!ped_disk_commit (*diskp)) + goto error; return 1; error_free_name: free (name); -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } @@ -943,7 +945,7 @@ _print_disk_geometry (const PedDevice *dev) } static void -_print_disk_info (const PedDevice *dev, const PedDisk *disk) +_print_disk_info (const PedDevice *dev, const PedDisk *diskp) { char const *const transport[] = {"unknown", "scsi", "ide", "dac960", "cpqarray", "file", "ataraid", "i2o", @@ -957,8 +959,8 @@ _print_disk_info (const PedDevice *dev, const PedDisk *disk) - (default_unit == PED_UNIT_CHS || default_unit == PED_UNIT_CYLINDER)); - const char* pt_name = disk ? disk->type->name : "unknown"; - char *disk_flags = disk_print_flags (disk); + const char* pt_name = diskp ? diskp->type->name : "unknown"; + char *disk_flags = disk_print_flags (diskp); if (opt_machine_mode) { switch (default_unit) { @@ -997,9 +999,8 @@ _print_disk_info (const PedDevice *dev, const PedDisk *disk) } static int -do_print (PedDevice** dev) +do_print (PedDevice** dev, PedDisk** diskp) { - PedDisk* disk = NULL; Table* table; int has_extended; int has_name; @@ -1041,22 +1042,23 @@ do_print (PedDevice** dev) } if (!has_devices_arg && !has_list_arg) { - disk = ped_disk_new (*dev); + if (!*diskp) + *diskp = ped_disk_new (*dev); /* Returning NULL here is an indication of failure, when in script mode. Otherwise (interactive mode) it may indicate a real error, but it may also indicate that the user declined when asked to perform some operation. FIXME: what this really needs is an API change, but a reliable exit code is less important in interactive mode. */ - if (disk == NULL && opt_script_mode) + if (*diskp == NULL && opt_script_mode) ok = 0; } - if (disk && - ped_disk_is_flag_available(disk, PED_DISK_CYLINDER_ALIGNMENT)) - if (!ped_disk_set_flag(disk, PED_DISK_CYLINDER_ALIGNMENT, + if (*diskp && + ped_disk_is_flag_available(*diskp, PED_DISK_CYLINDER_ALIGNMENT)) + if (!ped_disk_set_flag(*diskp, PED_DISK_CYLINDER_ALIGNMENT, alignment == ALIGNMENT_CYLINDER)) - goto error_destroy_disk; + return 0; if (has_devices_arg) { char* dev_name; @@ -1089,24 +1091,23 @@ do_print (PedDevice** dev) else if (has_list_arg) return _print_list (); - else if (disk && has_num_arg) { + else if (*diskp && has_num_arg) { PedPartition* part = NULL; int status = 0; - if (command_line_get_partition ("", disk, &part)) + if (command_line_get_partition ("", *diskp, &part)) status = partition_print (part); - ped_disk_destroy (disk); return status; } - _print_disk_info (*dev, disk); - if (!disk) + _print_disk_info (*dev, *diskp); + if (!*diskp) goto nopt; if (!opt_machine_mode) putchar ('\n'); - has_extended = ped_disk_type_check_feature (disk->type, + has_extended = ped_disk_type_check_feature ((*diskp)->type, PED_DISK_TYPE_EXTENDED); - has_name = ped_disk_type_check_feature (disk->type, + has_name = ped_disk_type_check_feature ((*diskp)->type, PED_DISK_TYPE_PARTITION_NAME); PedPartition* part; @@ -1136,8 +1137,8 @@ do_print (PedDevice** dev) table_add_row_from_strlist (table, row1); - for (part = ped_disk_next_partition (disk, NULL); part; - part = ped_disk_next_partition (disk, part)) { + for (part = ped_disk_next_partition (*diskp, NULL); part; + part = ped_disk_next_partition (*diskp, part)) { if ((!has_free_arg && !ped_partition_is_active(part)) || part->type & PED_PARTITION_METADATA) @@ -1212,8 +1213,8 @@ do_print (PedDevice** dev) } else { - for (part = ped_disk_next_partition (disk, NULL); part; - part = ped_disk_next_partition (disk, part)) { + for (part = ped_disk_next_partition (*diskp, NULL); part; + part = ped_disk_next_partition (*diskp, part)) { if ((!has_free_arg && !ped_partition_is_active(part)) || part->type & PED_PARTITION_METADATA) @@ -1261,13 +1262,8 @@ do_print (PedDevice** dev) } } - ped_disk_destroy (disk); - return ok; -error_destroy_disk: - ped_disk_destroy (disk); - return 0; nopt: return ok; } @@ -1276,11 +1272,15 @@ static int _print_list () { PedDevice *current_dev = NULL; + PedDisk *diskp = NULL; ped_device_probe_all(); while ((current_dev = ped_device_get_next(current_dev))) { - do_print (¤t_dev); + do_print (¤t_dev, &diskp); + if (diskp) + ped_disk_destroy (diskp); + diskp = 0; putchar ('\n'); } @@ -1288,9 +1288,9 @@ _print_list () } static int -do_quit (PedDevice** dev) +do_quit (PedDevice** dev, PedDisk **diskp) { - _done (*dev); + _done (*dev, *diskp); exit (EXIT_SUCCESS); } @@ -1436,7 +1436,7 @@ error_remove_partition: } static int -do_rescue (PedDevice** dev) +do_rescue (PedDevice** dev, PedDisk** diskp) { PedDisk* disk; PedSector start = 0, end = 0; @@ -1444,6 +1444,10 @@ do_rescue (PedDevice** dev) PedGeometry probe_start_region; PedGeometry probe_end_region; + if (*diskp) { + ped_disk_destroy (*diskp); + *diskp = 0; + } disk = ped_disk_new (*dev); if (!disk) goto error; @@ -1480,37 +1484,34 @@ error: } static int -do_rm (PedDevice** dev) +do_rm (PedDevice** dev, PedDisk** diskp) { - PedDisk* disk; PedPartition* part = NULL; - disk = ped_disk_new (*dev); - if (!disk) + if (!*diskp) + *diskp = ped_disk_new (*dev); + if (!*diskp) goto error; - if (!command_line_get_partition (_("Partition number?"), disk, &part)) - goto error_destroy_disk; + if (!command_line_get_partition (_("Partition number?"), *diskp, &part)) + goto error; if (!_partition_warn_busy (part)) - goto error_destroy_disk; + goto error; - ped_disk_delete_partition (disk, part); - ped_disk_commit (disk); - ped_disk_destroy (disk); + ped_disk_delete_partition (*diskp, part); + ped_disk_commit (*diskp); if ((*dev)->type != PED_DEVICE_FILE) disk_is_modified = 1; return 1; -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } static int -do_select (PedDevice** dev) +do_select (PedDevice** dev, PedDisk** diskp) { PedDevice* new_dev = *dev; @@ -1520,6 +1521,10 @@ do_select (PedDevice** dev) return 0; ped_device_close (*dev); + if (*diskp) { + ped_disk_destroy (*diskp); + *diskp = 0; + } *dev = new_dev; print_using_dev (*dev); return 1; @@ -1550,26 +1555,25 @@ partition_align_check (PedDisk const *disk, PedPartition const *part, } static int -do_align_check (PedDevice **dev) +do_align_check (PedDevice **dev, PedDisk** diskp) { - PedDisk *disk = ped_disk_new (*dev); - if (!disk) + if (!*diskp) + *diskp = ped_disk_new (*dev); + if (!*diskp) goto error; enum AlignmentType align_type = PA_OPTIMUM; PedPartition *part = NULL; if (!command_line_get_align_type (_("alignment type(min/opt)"), &align_type)) - goto error_destroy_disk; - if (!command_line_get_partition (_("Partition number?"), disk, &part)) - goto error_destroy_disk; + goto error; + if (!command_line_get_partition (_("Partition number?"), *diskp, &part)) + goto error; - bool aligned = partition_align_check (disk, part, align_type); + bool aligned = partition_align_check (*diskp, part, align_type); if (!opt_script_mode) printf(aligned ? _("%d aligned\n") : _("%d not aligned\n"), part->num); - ped_disk_destroy (disk); - if (opt_script_mode) return aligned ? 1 : 0; @@ -1577,115 +1581,107 @@ do_align_check (PedDevice **dev) with the other modes. */ return 1; -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } static int -do_disk_set (PedDevice** dev) +do_disk_set (PedDevice** dev, PedDisk** diskp) { - PedDisk* disk; PedDiskFlag flag; int state; - disk = ped_disk_new (*dev); - if (!disk) + if (!*diskp) + *diskp = ped_disk_new (*dev); + if (!diskp) goto error; - if (!command_line_get_disk_flag (_("Flag to Invert?"), disk, &flag)) - goto error_destroy_disk; - state = (ped_disk_get_flag (disk, flag) == 0 ? 1 : 0); + if (!command_line_get_disk_flag (_("Flag to Invert?"), *diskp, &flag)) + goto error; + state = (ped_disk_get_flag (*diskp, flag) == 0 ? 1 : 0); if (!is_toggle_mode) { if (!command_line_get_state (_("New state?"), &state)) - goto error_destroy_disk; + goto error; } - if (!ped_disk_set_flag (disk, flag, state)) - goto error_destroy_disk; - if (!ped_disk_commit (disk)) - goto error_destroy_disk; - ped_disk_destroy (disk); + if (!ped_disk_set_flag (*diskp, flag, state)) + goto error; + if (!ped_disk_commit (*diskp)) + goto error; if ((*dev)->type != PED_DEVICE_FILE) disk_is_modified = 1; return 1; -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } static int -do_set (PedDevice** dev) +do_set (PedDevice** dev, PedDisk **diskp) { - PedDisk* disk; PedPartition* part = NULL; PedPartitionFlag flag; int state; - disk = ped_disk_new (*dev); - if (!disk) + if (*diskp == 0) + *diskp = ped_disk_new (*dev); + if (!*diskp) goto error; - if (!command_line_get_partition (_("Partition number?"), disk, &part)) - goto error_destroy_disk; + if (!command_line_get_partition (_("Partition number?"), *diskp, &part)) + goto error; if (!command_line_get_part_flag (_("Flag to Invert?"), part, &flag)) - goto error_destroy_disk; + goto error; state = (ped_partition_get_flag (part, flag) == 0 ? 1 : 0); if (!is_toggle_mode) { if (!command_line_get_state (_("New state?"), &state)) - goto error_destroy_disk; + goto error; } if (!ped_partition_set_flag (part, flag, state)) - goto error_destroy_disk; - if (!ped_disk_commit (disk)) - goto error_destroy_disk; - ped_disk_destroy (disk); + goto error; + if (!ped_disk_commit (*diskp)) + goto error; if ((*dev)->type != PED_DEVICE_FILE) disk_is_modified = 1; - return 1; + return 1; -error_destroy_disk: - ped_disk_destroy (disk); error: return 0; } static int -do_disk_toggle (PedDevice **dev) +do_disk_toggle (PedDevice **dev, PedDisk** diskp) { int result; is_toggle_mode = 1; - result = do_disk_set (dev); + result = do_disk_set (dev, diskp); is_toggle_mode = 0; return result; } static int -do_toggle (PedDevice **dev) +do_toggle (PedDevice **dev, PedDisk** diskp) { int result; is_toggle_mode = 1; - result = do_set (dev); + result = do_set (dev, diskp); is_toggle_mode = 0; return result; } static int -do_unit (PedDevice** dev) +do_unit (PedDevice** dev, PedDisk** diskp) { PedUnit unit = ped_unit_get_default (); if (!command_line_get_unit (_("Unit?"), &unit)) @@ -2150,20 +2146,22 @@ return NULL; } static void -_done (PedDevice* dev) +_done (PedDevice* dev, PedDisk* diskp) { -if (dev->boot_dirty && dev->type != PED_DEVICE_FILE) { - ped_exception_throw ( - PED_EXCEPTION_WARNING, - PED_EXCEPTION_OK, - _("You should reinstall your boot loader before " - "rebooting. Read section 4 of the Parted User " - "documentation for more information.")); -} -if (!opt_script_mode && !opt_machine_mode && disk_is_modified) { - ped_exception_throw ( - PED_EXCEPTION_INFORMATION, PED_EXCEPTION_OK, - _("You may need to update /etc/fstab.\n")); + if (diskp) + ped_disk_destroy (diskp); + if (dev->boot_dirty && dev->type != PED_DEVICE_FILE) { + ped_exception_throw ( + PED_EXCEPTION_WARNING, + PED_EXCEPTION_OK, + _("You should reinstall your boot loader before " + "rebooting. Read section 4 of the Parted User " + "documentation for more information.")); + } + if (!opt_script_mode && !opt_machine_mode && disk_is_modified) { + ped_exception_throw ( + PED_EXCEPTION_INFORMATION, PED_EXCEPTION_OK, + _("You may need to update /etc/fstab.\n")); } ped_device_close (dev); @@ -2178,6 +2176,7 @@ int main (int argc, char** argv) { PedDevice* dev; + PedDisk* diskp = 0; int status; set_program_name (argv[0]); @@ -2188,11 +2187,11 @@ main (int argc, char** argv) return 1; if (argc || opt_script_mode) - status = non_interactive_mode (&dev, commands, argc, argv); + status = non_interactive_mode (&dev, &diskp, commands, argc, argv); else - status = interactive_mode (&dev, commands); + status = interactive_mode (&dev, &diskp, commands); - _done (dev); + _done (dev, diskp); return !status; } diff --git a/parted/ui.c b/parted/ui.c index b33f6fc..925110d 100644 --- a/parted/ui.c +++ b/parted/ui.c @@ -1557,7 +1557,7 @@ print_using_dev (PedDevice* dev) } int -interactive_mode (PedDevice** dev, Command* cmd_list[]) +interactive_mode (PedDevice** dev, PedDisk** disk, Command* cmd_list[]) { StrList* list; StrList* command_names = command_get_names (cmd_list); @@ -1590,7 +1590,7 @@ interactive_mode (PedDevice** dev, Command* cmd_list[]) cmd = command_get (commands, word); free (word); if (cmd) { - if (!command_run (cmd, dev)) + if (!command_run (cmd, dev, disk)) command_line_flush (); } else print_commands_help (); @@ -1602,7 +1602,7 @@ interactive_mode (PedDevice** dev, Command* cmd_list[]) int -non_interactive_mode (PedDevice** dev, Command* cmd_list[], +non_interactive_mode (PedDevice** dev, PedDisk **disk, Command* cmd_list[], int argc, char* argv[]) { int i; @@ -1633,7 +1633,7 @@ non_interactive_mode (PedDevice** dev, Command* cmd_list[], goto error; } - if (!command_run (cmd, dev)) + if (!command_run (cmd, dev, disk)) goto error; } return 1; diff --git a/parted/ui.h b/parted/ui.h index 3c6ebc0..260ce37 100644 --- a/parted/ui.h +++ b/parted/ui.h @@ -31,9 +31,11 @@ extern const char *prog_name; extern int init_ui (); extern int init_readline (); -extern int non_interactive_mode (PedDevice** dev, Command* cmd_list[], - int argc, char* argv[]); -extern int interactive_mode (PedDevice** dev, Command* cmd_list[]); +extern int non_interactive_mode (PedDevice** dev, PedDisk **disk, + Command* cmd_list[], int argc, + char* argv[]); +extern int interactive_mode (PedDevice** dev, PedDisk **disk, + Command* cmd_list[]); extern void done_ui (); extern int screen_width (); diff --git a/tests/t0283-overlap-partitions.sh b/tests/t0283-overlap-partitions.sh index 221332d..76b2740 100644 --- a/tests/t0283-overlap-partitions.sh +++ b/tests/t0283-overlap-partitions.sh @@ -30,7 +30,6 @@ parted ---pretend-input-tty $dev <<EOF > out 2>&1 || fail=1 print ignore rm -ignore 2 EOF @@ -59,8 +58,6 @@ Number Start End Size Type File system Flags 2 5242kB 8000kB 2758kB primary (parted) rm -Error: Can't have overlapping partitions. -Ignore/Cancel? ignore Partition number? 2 (parted) EOF @@ -73,7 +70,6 @@ parted ---pretend-input-tty $dev <<EOF > out 2>&1 || fail=1 print ignore rm -ignore 1 EOF @@ -101,8 +97,6 @@ Number Start End Size Type File system Flags 1 1049kB 5243kB 4194kB primary (parted) rm -Error: Can't have a partition outside the disk! -Ignore/Cancel? ignore Partition number? 1 (parted) EOF diff --git a/tests/t3300-palo-prep.sh b/tests/t3300-palo-prep.sh index 4050414..88b2c55 100755 --- a/tests/t3300-palo-prep.sh +++ b/tests/t3300-palo-prep.sh @@ -20,9 +20,9 @@ ss=$sector_size_ cat > exp <<EOF || framework_failure -1:2048s:4095s:2048s:::palo; -1:2048s:4095s:2048s:::prep; -1:2048s:4095s:2048s:::palo; +1:2048s:4095s:2048s:ext2::lba, palo; +1:2048s:4095s:2048s:ext2::lba, prep; +1:2048s:4095s:2048s:ext2::lba, palo; EOF dev=dev-file @@ -37,7 +37,7 @@ parted -m -s $dev mklabel msdos \ set 1 palo on u s print \ > out 2> err || fail=1 -grep -E '^1:2048s:4095s:2048s:::p...;$' out > k; mv k out +grep -E '^1:2048s:4095s:2048s:ext2::lba, p...;$' out > k; mv k out compare exp out || fail=1 -- 1.9.1
bug-parted <at> gnu.org
:bug#16370
; Package parted
.
(Sat, 03 May 2014 01:48:02 GMT) Full text and rfc822 format available.Message #23 received at 16370 <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16370 <at> debbugs.gnu.org Cc: "Brian C. Lane" <bcl <at> redhat.com> Subject: Re: bug#16370: [PATCH] parted: don't reload partition table on every command Date: Fri, 02 May 2014 21:47:12 -0400
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On 04/18/2014 06:55 PM, Phillip Susi wrote: > I could have sworn I fixed that test before, but here it is > > 8<---------------------------->8 > > gpt was using a static local variable to suppress repeatedly > reporting an error if you chose to ignore it. This is incorrect as > the variable is global to all disks, and ignoring the error on one > should not suppress its reporting on another. Moving the flag to > the PedDisk object made it effectively useless because parted was > destroying the PedDisk and reloading the partition table on every > command. Just noticed I didn't Cc you on this, was that an ack on this one with the fixed test? -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJTZEqgAAoJEI5FoCIzSKrwmHwIAIF/g5wvbMLV/rrgYlQ5lmSM jxJYK6IFl0eOn96FtpPMUXlhNoDMlyYIEH6iX/FmhaKm59MaZ+Ueq80CuKTj878B g/ELD/YHgv18BhUqBSwFNRpP2LIvrqWnKnb1xifLwiSISo6DRJI1w78y/8waOdmX /L/wJEi4cs7R0jt5WHcNeutv/GuEBxEgsF51TlPBVLjmEMXf5aCyQ0dOefSrUIxz +qiPu64rSxglF4oZhpnf0LgrEcUgVm0MpSbqKcHBlRm6/mTDeyskr7acJB2hY3+s 1KJI47MSM8fCGZRqdFbjSTKgiSUMOykJy0/5Gtn3v6+Dmiz3NDlBtWShSRxXjq8= =NEr9 -----END PGP SIGNATURE-----
Phillip Susi <psusi <at> ubuntu.com>
:Phillip Susi <psusi <at> ubuntu.com>
:Message #28 received at 16370-done <at> debbugs.gnu.org (full text, mbox):
From: Phillip Susi <psusi <at> ubuntu.com> To: 16370-done <at> debbugs.gnu.org Subject: Re: bug#16370: [PATCH] parted: don't reload partition table on every command Date: Thu, 22 May 2014 20:04:50 -0400
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On 05/05/2014 07:32 PM, Brian C. Lane wrote: >> Just noticed I didn't Cc you on this, was that an ack on this one >> with the fixed test? > > Yes. Also, I'm on the lists so no need for explicit cc's Patch pushed. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCgAGBQJTfpCiAAoJEI5FoCIzSKrwTiEH+wd3HVtjF14jTTBEZzqDnSRo 278Z47xMA5/vi4IQ/gAJZBZVxYha6YcV6AOZdJliPZDoXbZZ7BkAPdMbkB4dfVLg z6QiF/N2XlgsnC/HDoTOUTQjoKTvh4ly7HcnO5Y2PbGitiTm7gWSt27y3fZio5l7 7VTQjpjQg3ro3RvUK4AfLafT1wZ+xlzNa7y6ziyW3SCefvCyjyD4mKr2W9IDtFoU E9UHb3x60Ad74BHhOu/clcKhmfK6OeBSlpBlvVryFvhmhQLurvAetQRy+3duxh0v Rh+8LBdPPVk/wibR1RmZKpcB9Z1dLX03Db/t1vM70zz4b2In4L00OvLKRq1BMww= =e8GH -----END PGP SIGNATURE-----
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org
.
(Fri, 20 Jun 2014 11:24:05 GMT) Full text and rfc822 format available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.