Mageia Bugzilla – Attachment 9054 Details for
Bug 20264
Using "clear all" in diskdrake or in installer partitioning on GPT disk results in "failed to del partition #1 on /dev/..." message
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
Log In
[x]
|
New Account
|
Forgot Password
[patch]
Prevent unwanted udev events when writing a GPT partition table
0001-When-writing-a-GPT-partition-table-merge-all-actions.patch (text/plain), 10.24 KB, created by
Thierry Vignaud
on 2017-03-09 12:09:47 CET
(
hide
)
Description:
Prevent unwanted udev events when writing a GPT partition table
Filename:
MIME Type:
Creator:
Thierry Vignaud
Created:
2017-03-09 12:09:47 CET
Size:
10.24 KB
patch
obsolete
>From 385fd60702e1800d2b8fac3ff3790917f524f547 Mon Sep 17 00:00:00 2001 >From: Martin Whitaker <mageia@martin-whitaker.me.uk> >Date: Sun, 5 Mar 2017 19:08:06 +0000 >Subject: [PATCH 1/2] When writing a GPT partition table, merge all actions > into a single commit. > >This avoids unnecessary udev events, which in some cases were causing >udevd to trigger a kernel partition table reread, causing the kernel >to get out of sync with drakx. > >This includes changing c::get_disk_type() to use ped_disk_probe(), >again to prevent unwanted udev events. > >In order to do so, the libparted bindings are enhanced so that we >manipulate a 'ped_disk' object: >- disk_open() returns such an object >- set_disk_type() is removed >- disk_commit() writes back the partition table and tell the kernel > about it >- need_to_tell_kernel() is overidded > >The last change is needed because we now tell the kernel about the new >partition layout in partition_table::gpt::write() when calling >disk_commit() while previously we were deferring that to >partition_table::write() which after having called the gpt's write() >calls tell_kernel() -> c::tell_kernel_to_reread_partition_table() > >So we must tell partition_table::write() that telling the kernel to >reread the partition_table is not needed if we already succeeded in >that. >--- > perl-install/c/stuff.xs.pl | 128 ++++++++++++++++-------------------- > perl-install/partition_table.pm | 2 +- > perl-install/partition_table/gpt.pm | 32 ++++++--- > 3 files changed, 78 insertions(+), 84 deletions(-) > >diff --git a/perl-install/c/stuff.xs.pl b/perl-install/c/stuff.xs.pl >index e255b57..0e52e6f 100755 >--- a/perl-install/c/stuff.xs.pl >+++ b/perl-install/c/stuff.xs.pl >@@ -610,25 +610,23 @@ get_iso_volume_ids(int fd) > > print ' > >+TYPEMAP: <<HERE >+PedDisk* T_PTROBJ >+HERE >+ >+ >+ > int >-set_partition_flag(char * device_path, int part_number, char *type) >+set_partition_flag(PedDisk *disk, int part_number, char * type) > CODE: >- PedDevice *dev = ped_device_get(device_path); > RETVAL = 0; >- if(dev) { >- PedDisk* disk = ped_disk_new(dev); >- if(disk) { >- PedPartition* part = ped_disk_get_partition(disk, part_number); >- if (!part) { >- printf("set_partition_flag: failed to find partition\n"); >- } else { >- PedPartitionFlag flag = string_to_pedpartflag(type); >- if (flag) >- RETVAL=ped_partition_set_flag(part, flag, 1); >- if (RETVAL) >- RETVAL = ped_disk_commit_to_dev(disk); >- } >- ped_disk_destroy(disk); >+ PedPartition* part = ped_disk_get_partition(disk, part_number); >+ if (!part) { >+ printf("set_partition_flag: failed to find partition\n"); >+ } else { >+ PedPartitionFlag flag = string_to_pedpartflag(type); >+ if (flag) { >+ RETVAL = ped_partition_set_flag(part, flag, 1); > } > } > OUTPUT: >@@ -700,91 +698,75 @@ get_disk_partitions(char * device_path) > ped_disk_destroy(disk); > } > >-int >-set_disk_type(char * device_path, const char * type_name) >+PedDisk* >+disk_open(char * device_path, const char * type_name = NULL) > CODE: > PedDevice *dev = ped_device_get(device_path); >- RETVAL = 0; >+ RETVAL = NULL; > if(dev) { >- PedDiskType* type = ped_disk_type_get(type_name); >- if(type) { >- PedDisk* disk = ped_disk_new_fresh(dev, type); >- if(disk) { >- RETVAL = ped_disk_commit(disk); >- ped_disk_destroy(disk); >+ if(type_name) { >+ PedDiskType* type = ped_disk_type_get(type_name); >+ if(type) { >+ RETVAL = ped_disk_new_fresh(dev, type); > } >+ } else { >+ RETVAL = ped_disk_new(dev); > } > } > OUTPUT: > RETVAL > > int >-disk_delete_all(char * device_path) >+disk_delete_all(PedDisk* disk) > CODE: >- PedDevice *dev = ped_device_get(device_path); > RETVAL = 0; >- if(dev) { >- PedDisk* disk = ped_disk_new(dev); >- if(disk) { >- RETVAL = ped_disk_delete_all(disk); >- if(RETVAL) >- RETVAL = ped_disk_commit(disk); >- ped_disk_destroy(disk); >- } >+ if (ped_disk_delete_all(disk)) { >+ RETVAL = 1; > } > OUTPUT: > RETVAL > > int >-disk_del_partition(char * device_path, int part_number) >+disk_del_partition(PedDisk* disk, int part_number) > CODE: >- PedDevice *dev = ped_device_get(device_path); > RETVAL = 0; >- if(dev) { >- PedDisk* disk = ped_disk_new(dev); >- if(disk) { >- PedPartition* part = ped_disk_get_partition(disk, part_number); >- if(!part) { >- printf("disk_del_partition: failed to find partition\n"); >- } else { >- RETVAL=ped_disk_delete_partition(disk, part); >- if(RETVAL) { >- RETVAL = ped_disk_commit_to_dev(disk); >- } else { >- printf("del_partition failed\n"); >- } >- } >- ped_disk_destroy(disk); >- } >+ PedPartition* part = ped_disk_get_partition(disk, part_number); >+ if(!part) { >+ printf("disk_del_partition: failed to find partition\n"); >+ } else { >+ RETVAL = ped_disk_delete_partition(disk, part); > } > OUTPUT: > RETVAL > > int >-disk_add_partition(char * device_path, double start, double length, const char * fs_type) >+disk_add_partition(PedDisk* disk, double start, double length, const char * fs_type) > CODE: >- PedDevice *dev = ped_device_get(device_path); > RETVAL=0; >- if(dev) { >- PedDisk* disk = ped_disk_new(dev); >- if(disk) { >- PedGeometry* geom = ped_geometry_new(dev, (long long)start, (long long)length); >- PedPartition* part = ped_partition_new (disk, PED_PARTITION_NORMAL, ped_file_system_type_get(fs_type), (long long)start, (long long)start+length-1); >- PedConstraint* constraint = ped_constraint_new_from_max(geom); >- if(!part) { >- printf("ped_partition_new failed\n"); >- } else >- RETVAL = ped_disk_add_partition (disk, part, constraint); >- if(RETVAL) { >- RETVAL = ped_disk_commit_to_dev(disk); >- } else { >- printf("add_partition failed\n"); >- } >- ped_geometry_destroy(geom); >- ped_constraint_destroy(constraint); >- ped_disk_destroy(disk); >+ PedGeometry* geom = ped_geometry_new(disk->dev, (long long)start, (long long)length); >+ PedPartition* part = ped_partition_new (disk, PED_PARTITION_NORMAL, ped_file_system_type_get(fs_type), (long long)start, (long long)start+length-1); >+ PedConstraint* constraint = ped_constraint_new_from_max(geom); >+ if(!part) { >+ printf("ped_partition_new failed\n"); >+ } else { >+ RETVAL = ped_disk_add_partition (disk, part, constraint); >+ } >+ ped_geometry_destroy(geom); >+ ped_constraint_destroy(constraint); >+ OUTPUT: >+ RETVAL >+ >+int >+disk_commit(PedDisk *disk) >+ CODE: >+ RETVAL = 0; >+ if (ped_disk_commit_to_dev(disk)) { >+ RETVAL = 1; >+ if (ped_disk_commit_to_os(disk)) { >+ RETVAL = 2; > } > } >+ ped_disk_destroy(disk); > OUTPUT: > RETVAL > >diff --git a/perl-install/partition_table.pm b/perl-install/partition_table.pm >index 20d6a2b..791e825 100644 >--- a/perl-install/partition_table.pm >+++ b/perl-install/partition_table.pm >@@ -435,7 +435,7 @@ sub tell_kernel { > > my $F = partition_table::raw::openit($hd); > >- my $force_reboot = any { $_->[0] eq 'init' } @$tell_kernel; >+ my $force_reboot = $hd->{rebootNeeded} || any { $_->[0] eq 'init' } @$tell_kernel; > if (!$force_reboot) { > foreach (@$tell_kernel) { > my ($action, $part_number, $o_start, $o_size) = @$_; >diff --git a/perl-install/partition_table/gpt.pm b/perl-install/partition_table/gpt.pm >index f2f39f0..010bac6 100644 >--- a/perl-install/partition_table/gpt.pm >+++ b/perl-install/partition_table/gpt.pm >@@ -73,12 +73,15 @@ sub read_one { > sub write { > my ($hd, $_handle, $_sector, $pt, $_info) = @_; > >+ my $ped_disk; > my $partitions_killed; > > # Initialize the disk if current partition table is not gpt > if (c::get_disk_type($hd->{file}) ne "gpt") { >- c::set_disk_type($hd->{file}, "gpt"); >+ $ped_disk = c::disk_open($hd->{file}, "gpt") or die "failed to create new partition table on $hd->{file}"; > $partitions_killed = 1; >+ } else { >+ $ped_disk = c::disk_open($hd->{file}) or die "failed to open partition table on $hd->{file}"; > } > > foreach (@{$hd->{will_tell_kernel}}) { >@@ -87,7 +90,7 @@ sub write { > log::l("GPT partitioning: ($action, $part_number, $o_start, $o_size)"); > if ($action eq 'add') { > local $part->{fs_type} = $rev_parted_mapping{$part->{fs_type}} if $rev_parted_mapping{$part->{fs_type}}; >- c::disk_add_partition($hd->{file}, $o_start, $o_size, $part->{fs_type}) or die "failed to add partition #$part_number on $hd->{file}"; >+ c::disk_add_partition($ped_disk, $o_start, $o_size, $part->{fs_type}) or die "failed to add partition #$part_number on $hd->{file}"; > my $flag; > if (isESP($part)) { > $flag = 'ESP'; >@@ -99,20 +102,29 @@ sub write { > $flag = 'RAID'; > } > if ($flag) { >- c::set_partition_flag($hd->{file}, $part_number, $flag) >+ c::set_partition_flag($ped_disk, $part_number, $flag) > or die "failed to set type '$flag' for $part->{file} on $part->{mntpoint}"; > } > } elsif ($action eq 'del' && !$partitions_killed) { >- c::disk_del_partition($hd->{file}, $part_number) or die "failed to del partition #$part_number on $hd->{file}"; >+ c::disk_del_partition($ped_disk, $part_number) or die "failed to del partition #$part_number on $hd->{file}"; > } elsif ($action eq 'init' && !$partitions_killed) { >- c::disk_delete_all($hd->{file}) or die "failed to delete all partitions on $hd->{file}"; >+ c::disk_delete_all($ped_disk) or die "failed to delete all partitions on $hd->{file}"; > } > } >- # prevent errors when telling kernel to reread partition table: >- # (above add/del_partition result in udev events) >- system(qw(udevadm settle)); >- common::sync(); >- 1; >+ >+ # Commit changes to the disk and inform the kernel >+ my $commit_level = c::disk_commit($ped_disk); >+ # Updating the kernel partition table will fail if any of the deleted partitions were mounted >+ $hd->{rebootNeeded} = 1 if $commit_level < 2; >+ $commit_level; >+} >+ >+# Hint partition_table::write() that telling the kernel to reread the >+# partition_table is not needed if we already succeeded in that: >+sub need_to_tell_kernel { >+ my ($hd) = @_; >+ # If we failed, try again (partion_table::tell_kernel() will unmount some partitions first) >+ $hd->{rebootNeeded}; > } > > sub initialize { >-- >2.10.2 >
From 385fd60702e1800d2b8fac3ff3790917f524f547 Mon Sep 17 00:00:00 2001 From: Martin Whitaker <mageia@martin-whitaker.me.uk> Date: Sun, 5 Mar 2017 19:08:06 +0000 Subject: [PATCH 1/2] When writing a GPT partition table, merge all actions into a single commit. This avoids unnecessary udev events, which in some cases were causing udevd to trigger a kernel partition table reread, causing the kernel to get out of sync with drakx. This includes changing c::get_disk_type() to use ped_disk_probe(), again to prevent unwanted udev events. In order to do so, the libparted bindings are enhanced so that we manipulate a 'ped_disk' object: - disk_open() returns such an object - set_disk_type() is removed - disk_commit() writes back the partition table and tell the kernel about it - need_to_tell_kernel() is overidded The last change is needed because we now tell the kernel about the new partition layout in partition_table::gpt::write() when calling disk_commit() while previously we were deferring that to partition_table::write() which after having called the gpt's write() calls tell_kernel() -> c::tell_kernel_to_reread_partition_table() So we must tell partition_table::write() that telling the kernel to reread the partition_table is not needed if we already succeeded in that. --- perl-install/c/stuff.xs.pl | 128 ++++++++++++++++-------------------- perl-install/partition_table.pm | 2 +- perl-install/partition_table/gpt.pm | 32 ++++++--- 3 files changed, 78 insertions(+), 84 deletions(-) diff --git a/perl-install/c/stuff.xs.pl b/perl-install/c/stuff.xs.pl index e255b57..0e52e6f 100755 --- a/perl-install/c/stuff.xs.pl +++ b/perl-install/c/stuff.xs.pl @@ -610,25 +610,23 @@ get_iso_volume_ids(int fd) print ' +TYPEMAP: <<HERE +PedDisk* T_PTROBJ +HERE + + + int -set_partition_flag(char * device_path, int part_number, char *type) +set_partition_flag(PedDisk *disk, int part_number, char * type) CODE: - PedDevice *dev = ped_device_get(device_path); RETVAL = 0; - if(dev) { - PedDisk* disk = ped_disk_new(dev); - if(disk) { - PedPartition* part = ped_disk_get_partition(disk, part_number); - if (!part) { - printf("set_partition_flag: failed to find partition\n"); - } else { - PedPartitionFlag flag = string_to_pedpartflag(type); - if (flag) - RETVAL=ped_partition_set_flag(part, flag, 1); - if (RETVAL) - RETVAL = ped_disk_commit_to_dev(disk); - } - ped_disk_destroy(disk); + PedPartition* part = ped_disk_get_partition(disk, part_number); + if (!part) { + printf("set_partition_flag: failed to find partition\n"); + } else { + PedPartitionFlag flag = string_to_pedpartflag(type); + if (flag) { + RETVAL = ped_partition_set_flag(part, flag, 1); } } OUTPUT: @@ -700,91 +698,75 @@ get_disk_partitions(char * device_path) ped_disk_destroy(disk); } -int -set_disk_type(char * device_path, const char * type_name) +PedDisk* +disk_open(char * device_path, const char * type_name = NULL) CODE: PedDevice *dev = ped_device_get(device_path); - RETVAL = 0; + RETVAL = NULL; if(dev) { - PedDiskType* type = ped_disk_type_get(type_name); - if(type) { - PedDisk* disk = ped_disk_new_fresh(dev, type); - if(disk) { - RETVAL = ped_disk_commit(disk); - ped_disk_destroy(disk); + if(type_name) { + PedDiskType* type = ped_disk_type_get(type_name); + if(type) { + RETVAL = ped_disk_new_fresh(dev, type); } + } else { + RETVAL = ped_disk_new(dev); } } OUTPUT: RETVAL int -disk_delete_all(char * device_path) +disk_delete_all(PedDisk* disk) CODE: - PedDevice *dev = ped_device_get(device_path); RETVAL = 0; - if(dev) { - PedDisk* disk = ped_disk_new(dev); - if(disk) { - RETVAL = ped_disk_delete_all(disk); - if(RETVAL) - RETVAL = ped_disk_commit(disk); - ped_disk_destroy(disk); - } + if (ped_disk_delete_all(disk)) { + RETVAL = 1; } OUTPUT: RETVAL int -disk_del_partition(char * device_path, int part_number) +disk_del_partition(PedDisk* disk, int part_number) CODE: - PedDevice *dev = ped_device_get(device_path); RETVAL = 0; - if(dev) { - PedDisk* disk = ped_disk_new(dev); - if(disk) { - PedPartition* part = ped_disk_get_partition(disk, part_number); - if(!part) { - printf("disk_del_partition: failed to find partition\n"); - } else { - RETVAL=ped_disk_delete_partition(disk, part); - if(RETVAL) { - RETVAL = ped_disk_commit_to_dev(disk); - } else { - printf("del_partition failed\n"); - } - } - ped_disk_destroy(disk); - } + PedPartition* part = ped_disk_get_partition(disk, part_number); + if(!part) { + printf("disk_del_partition: failed to find partition\n"); + } else { + RETVAL = ped_disk_delete_partition(disk, part); } OUTPUT: RETVAL int -disk_add_partition(char * device_path, double start, double length, const char * fs_type) +disk_add_partition(PedDisk* disk, double start, double length, const char * fs_type) CODE: - PedDevice *dev = ped_device_get(device_path); RETVAL=0; - if(dev) { - PedDisk* disk = ped_disk_new(dev); - if(disk) { - PedGeometry* geom = ped_geometry_new(dev, (long long)start, (long long)length); - PedPartition* part = ped_partition_new (disk, PED_PARTITION_NORMAL, ped_file_system_type_get(fs_type), (long long)start, (long long)start+length-1); - PedConstraint* constraint = ped_constraint_new_from_max(geom); - if(!part) { - printf("ped_partition_new failed\n"); - } else - RETVAL = ped_disk_add_partition (disk, part, constraint); - if(RETVAL) { - RETVAL = ped_disk_commit_to_dev(disk); - } else { - printf("add_partition failed\n"); - } - ped_geometry_destroy(geom); - ped_constraint_destroy(constraint); - ped_disk_destroy(disk); + PedGeometry* geom = ped_geometry_new(disk->dev, (long long)start, (long long)length); + PedPartition* part = ped_partition_new (disk, PED_PARTITION_NORMAL, ped_file_system_type_get(fs_type), (long long)start, (long long)start+length-1); + PedConstraint* constraint = ped_constraint_new_from_max(geom); + if(!part) { + printf("ped_partition_new failed\n"); + } else { + RETVAL = ped_disk_add_partition (disk, part, constraint); + } + ped_geometry_destroy(geom); + ped_constraint_destroy(constraint); + OUTPUT: + RETVAL + +int +disk_commit(PedDisk *disk) + CODE: + RETVAL = 0; + if (ped_disk_commit_to_dev(disk)) { + RETVAL = 1; + if (ped_disk_commit_to_os(disk)) { + RETVAL = 2; } } + ped_disk_destroy(disk); OUTPUT: RETVAL diff --git a/perl-install/partition_table.pm b/perl-install/partition_table.pm index 20d6a2b..791e825 100644 --- a/perl-install/partition_table.pm +++ b/perl-install/partition_table.pm @@ -435,7 +435,7 @@ sub tell_kernel { my $F = partition_table::raw::openit($hd); - my $force_reboot = any { $_->[0] eq 'init' } @$tell_kernel; + my $force_reboot = $hd->{rebootNeeded} || any { $_->[0] eq 'init' } @$tell_kernel; if (!$force_reboot) { foreach (@$tell_kernel) { my ($action, $part_number, $o_start, $o_size) = @$_; diff --git a/perl-install/partition_table/gpt.pm b/perl-install/partition_table/gpt.pm index f2f39f0..010bac6 100644 --- a/perl-install/partition_table/gpt.pm +++ b/perl-install/partition_table/gpt.pm @@ -73,12 +73,15 @@ sub read_one { sub write { my ($hd, $_handle, $_sector, $pt, $_info) = @_; + my $ped_disk; my $partitions_killed; # Initialize the disk if current partition table is not gpt if (c::get_disk_type($hd->{file}) ne "gpt") { - c::set_disk_type($hd->{file}, "gpt"); + $ped_disk = c::disk_open($hd->{file}, "gpt") or die "failed to create new partition table on $hd->{file}"; $partitions_killed = 1; + } else { + $ped_disk = c::disk_open($hd->{file}) or die "failed to open partition table on $hd->{file}"; } foreach (@{$hd->{will_tell_kernel}}) { @@ -87,7 +90,7 @@ sub write { log::l("GPT partitioning: ($action, $part_number, $o_start, $o_size)"); if ($action eq 'add') { local $part->{fs_type} = $rev_parted_mapping{$part->{fs_type}} if $rev_parted_mapping{$part->{fs_type}}; - c::disk_add_partition($hd->{file}, $o_start, $o_size, $part->{fs_type}) or die "failed to add partition #$part_number on $hd->{file}"; + c::disk_add_partition($ped_disk, $o_start, $o_size, $part->{fs_type}) or die "failed to add partition #$part_number on $hd->{file}"; my $flag; if (isESP($part)) { $flag = 'ESP'; @@ -99,20 +102,29 @@ sub write { $flag = 'RAID'; } if ($flag) { - c::set_partition_flag($hd->{file}, $part_number, $flag) + c::set_partition_flag($ped_disk, $part_number, $flag) or die "failed to set type '$flag' for $part->{file} on $part->{mntpoint}"; } } elsif ($action eq 'del' && !$partitions_killed) { - c::disk_del_partition($hd->{file}, $part_number) or die "failed to del partition #$part_number on $hd->{file}"; + c::disk_del_partition($ped_disk, $part_number) or die "failed to del partition #$part_number on $hd->{file}"; } elsif ($action eq 'init' && !$partitions_killed) { - c::disk_delete_all($hd->{file}) or die "failed to delete all partitions on $hd->{file}"; + c::disk_delete_all($ped_disk) or die "failed to delete all partitions on $hd->{file}"; } } - # prevent errors when telling kernel to reread partition table: - # (above add/del_partition result in udev events) - system(qw(udevadm settle)); - common::sync(); - 1; + + # Commit changes to the disk and inform the kernel + my $commit_level = c::disk_commit($ped_disk); + # Updating the kernel partition table will fail if any of the deleted partitions were mounted + $hd->{rebootNeeded} = 1 if $commit_level < 2; + $commit_level; +} + +# Hint partition_table::write() that telling the kernel to reread the +# partition_table is not needed if we already succeeded in that: +sub need_to_tell_kernel { + my ($hd) = @_; + # If we failed, try again (partion_table::tell_kernel() will unmount some partitions first) + $hd->{rebootNeeded}; } sub initialize { -- 2.10.2
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 20264
:
9028
|
9029
|
9030
|
9031
|
9037
|
9038
|
9039
|
9040
|
9041
|
9042
|
9043
|
9047
|
9054
|
9055
|
9060
|
9082