[pve-devel] [PATCH storage 2/2] Fix #2550: Wipe disks before creating a directory

[Fabian Grünbichler]

On February 18, 2020 11:54 am, Dominic Jäger wrote:
> On Mon, Jan 27, 2020 at 03:57:19PM +0100, Fabian Grünbichler wrote:
>> On January 27, 2020 1:38 pm, Dominic Jäger wrote:
>> > Necessary because leftovers on a disk can make partitioning it fail.
>> 
>> shouldn't those leftovers be detected by our "is disk in use" checks? 
>> what's missing there?
> 
> The API calls PVE::Diskmanage::assert_disk_unused which relies on get_disks.
> get_disks scans for ZFS, LVM, mounts, device mapper and partitions.
> 
> In this case the partition scanning is important. From the step "mklabel gpt"
> in my script [0] onwards the partition folder /sys/block/sdb/sdb1 does not
> exist anymore. However, this is what get_disks looks for [1].

but the order should be:
- check if disk is in use
- wipe
- reformat

not
- wipe
- check if disk is in use
- reformat

?

> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=2550#c2
> [1] https://git.proxmox.com/?p=pve-storage.git;a=blob;f=PVE/Diskmanage.pm;h=abb90a79d93038cf470fe45ba1c7168c8d0f40a5;hb=refs/heads/master#l606
> 
>> 
>> > 
>> > Signed-off-by: Dominic Jäger <d.jaeger at proxmox.com>
>> > ---
>> >  PVE/API2/Disks/Directory.pm | 9 ++++++++-
>> >  1 file changed, 8 insertions(+), 1 deletion(-)
>> > 
>> > diff --git a/PVE/API2/Disks/Directory.pm b/PVE/API2/Disks/Directory.pm
>> > index 4c74776..8a317d3 100644
>> > --- a/PVE/API2/Disks/Directory.pm
>> > +++ b/PVE/API2/Disks/Directory.pm
>> > @@ -236,8 +236,15 @@ __PACKAGE__->register_method ({
>> >  	    my $mountunitpath = "/etc/systemd/system/$mountunitname";
>> >  
>> >  	    PVE::Diskmanage::locked_disk_action(sub {
>> > +		# Leftovers might thwart partitioning
>> > +		my $cmd = ['/sbin/wipefs', '-a', $dev];
>> > +		print "# ", join(' ', @$cmd), "\n";
>> > +		run_command($cmd);
>> 
>> would it make sense to (optionally) include wipefs into wipe_disks?
> 
> Will be included in the next version (like all the other feedback). Thank you!
>> > +
>> > +		PVE::Diskmanage::wipe_disks($dev);
>> > +
>> >  		# create partition
>> > -		my $cmd = [$SGDISK, '-n1', '-t1:8300', $dev];
>> > +		$cmd = [$SGDISK, '-n1', '-t1:8300', $dev];
>> >  		print "# ", join(' ', @$cmd), "\n";
>> >  		run_command($cmd);
>