[pve-devel] applied: [PATCH storage] ZFS: refactor waiting for zvol symlinks

[Wolfgang Bumiller]

applied

On Wed, Jul 31, 2019 at 10:58:04AM +0200, Fabian Grünbichler wrote:
> and actually do that not just for creating zvols, but also when
> activating them. this should fix a range of issues/races that sometimes
> occured on bootup, snapshot rollback or similar operations.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
> ---
> the waiting could be smarter I guess, but it is what we've used so far
> 
>  PVE/Storage/ZFSPoolPlugin.pm   | 36 +++++++++++++++++++++++++---------
>  test/run_test_zfspoolplugin.pl | 24 ++++++++++++++++++-----
>  2 files changed, 46 insertions(+), 14 deletions(-)
> 
> diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
> index 2e9c4c7..f66b277 100644
> --- a/PVE/Storage/ZFSPoolPlugin.pm
> +++ b/PVE/Storage/ZFSPoolPlugin.pm
> @@ -189,6 +189,23 @@ sub zfs_request {
>      return $msg;
>  }
>  
> +sub zfs_wait_for_zvol_link {
> +    my ($class, $scfg, $volname, $timeout) = @_;
> +
> +    my $default_timeout = PVE::RPCEnvironment->is_worker() ? 60*5 : 10;
> +    $timeout = $default_timeout if !defined($timeout);
> +
> +    my ($devname, undef, undef) = $class->path($scfg, $volname);
> +
> +    for (my $i = 1; $i <= $timeout; $i++) {
> +	last if -b $devname;
> +	die "timeout: no zvol device link for '$volname' found after $timeout sec found.\n"
> +	    if $i == $timeout;
> +
> +	sleep(1);
> +    }
> +}
> +
>  sub alloc_image {
>      my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
>  
> @@ -202,16 +219,8 @@ sub alloc_image {
>  	    if !$volname;
>  
>  	$class->zfs_create_zvol($scfg, $volname, $size);
> -	my $devname = "/dev/zvol/$scfg->{pool}/$volname";
> +	$class->zfs_wait_for_zvol_link($scfg, $volname);
>  
> -	my $timeout = PVE::RPCEnvironment->is_worker() ? 60*5 : 10;
> -	for (my $i = 1; $i <= $timeout; $i++) {
> -	    last if -b $devname;
> -	    die "Timeout: no zvol after $timeout sec found.\n"
> -		if $i == $timeout;
> -
> -	    sleep(1);
> -	}
>      } elsif ( $fmt eq 'subvol') {
>  
>  	die "illegal name '$volname' - should be 'subvol-$vmid-*'\n"
> @@ -545,6 +554,15 @@ sub deactivate_storage {
>  
>  sub activate_volume {
>      my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
> +
> +    return 1 if defined($snapname);
> +
> +    my (undef, undef, undef, undef, undef, undef, $format) = $class->parse_volname($volname);
> +
> +    return 1 if $format ne 'raw';
> +
> +    $class->zfs_wait_for_zvol_link($scfg, $volname);
> +
>      return 1;
>  }
>  
> diff --git a/test/run_test_zfspoolplugin.pl b/test/run_test_zfspoolplugin.pl
> index 63b1456..785d3b7 100755
> --- a/test/run_test_zfspoolplugin.pl
> +++ b/test/run_test_zfspoolplugin.pl
> @@ -1096,6 +1096,7 @@ my $test7 = sub {
>      };
>  
>      eval {
> +	PVE::Storage::activate_volumes($cfg, ["$storagename:$vmdisk"]);
>  	run_command("sgdisk --randomize-guids \/dev\/zvol\/$zpath\/$vmdisk", outfunc => $parse_guid);
>  	run_command("sgdisk -p \/dev\/zvol\/$zpath\/$vmdisk", outfunc => $parse_guid);
>  
> @@ -1105,6 +1106,7 @@ my $test7 = sub {
>  	run_command("sgdisk --randomize-guids \/dev\/zvol\/$zpath\/$vmdisk", outfunc => $parse_guid);
>  	eval {
>  	    PVE::Storage::volume_snapshot_rollback($cfg, "$storagename:$vmdisk", 'snap1');
> +	    PVE::Storage::activate_volumes($cfg, ["$storagename:$vmdisk"]);
>  	    $tmp_guid = undef;
>  	    run_command("sgdisk -p \/dev\/zvol\/$zpath\/$vmdisk", outfunc => $parse_guid);
>  	    if ($old_guid ne $tmp_guid) {
> @@ -1124,6 +1126,7 @@ my $test7 = sub {
>      $tmp_guid = undef;
>  
>      eval {
> +	PVE::Storage::activate_volumes($cfg, ["$storagename:$vmbase"]);
>  	run_command("sgdisk --randomize-guids \/dev\/zvol\/$zpath\/$vmbase", outfunc => $parse_guid);
>  	run_command("sgdisk -p \/dev\/zvol\/$zpath\/$vmbase", outfunc => $parse_guid);
>  
> @@ -1133,6 +1136,7 @@ my $test7 = sub {
>  	run_command("sgdisk --randomize-guids \/dev\/zvol\/$zpath\/$vmbase", outfunc => $parse_guid);
>  	eval {
>  	    PVE::Storage::volume_snapshot_rollback($cfg, "$storagename:$vmbase", 'snap1');
> +	    PVE::Storage::activate_volumes($cfg, ["$storagename:$vmbase"]);
>  	    $tmp_guid = undef;
>  	    run_command("sgdisk -p \/dev\/zvol\/$zpath\/$vmbase", outfunc => $parse_guid);
>  	    if ($old_guid ne $tmp_guid) {
> @@ -1152,6 +1156,7 @@ my $test7 = sub {
>      $tmp_guid = undef;
>  
>      eval {
> +	PVE::Storage::activate_volumes($cfg, ["$storagename:$vmbase/$vmlinked"]);
>  	run_command("sgdisk --randomize-guids \/dev\/zvol\/$zpath\/$vmlinked", outfunc => $parse_guid);
>  	run_command("sgdisk -p \/dev\/zvol\/$zpath\/$vmlinked", outfunc => $parse_guid);
>  
> @@ -1161,6 +1166,7 @@ my $test7 = sub {
>  	run_command("sgdisk --randomize-guids \/dev\/zvol\/$zpath\/$vmlinked", outfunc => $parse_guid);
>  	eval {
>  	    PVE::Storage::volume_snapshot_rollback($cfg, "$storagename:$vmbase\/$vmlinked", 'snap1');
> +	    PVE::Storage::activate_volumes($cfg, ["$storagename:$vmbase/$vmlinked"]);
>  	    $tmp_guid = undef;
>  	    run_command("sgdisk -p \/dev\/zvol\/$zpath\/$vmlinked", outfunc => $parse_guid);
>  	    if ($old_guid ne $tmp_guid) {
> @@ -2585,7 +2591,6 @@ my $test1 = sub {
>  $tests->{1} = $test1;
>  
>  sub setup_zfs {
> -
>      #create VM zvol
>      print "create zvol $vmdisk\n" if $verbose;
>      run_command("zfs create -V${volsize}G $zpath\/$vmdisk");
> @@ -2601,14 +2606,23 @@ sub setup_zfs {
>      print "create subvol $ctdisk\n" if $verbose;
>      run_command("zfs create -o refquota=${volsize}G $zpath\/$ctdisk");
>  
> -    print "create subvol $vmbase\n" if $verbose;
> +    print "create subvol $ctbase\n" if $verbose;
>      run_command("zfs create -o refquota=${volsize}G $zpath\/$ctbase");
>      run_command("zfs snapshot $zpath\/$ctbase$basesnap");
>  
> -    print "create linked clone $vmlinked\n" if $verbose;
> +    print "create linked clone $ctlinked\n" if $verbose;
>      run_command("zfs clone $zpath\/$ctbase$basesnap $zpath\/$ctlinked -o refquota=${volsize}G");
> -    run_command("udevadm trigger --subsystem-match block");
> -    run_command("udevadm settle --timeout 10 --exit-if-exists=/dev/zvol/$zpath\/$ctlinked");
> +
> +    my $vollist = [
> +	"$storagename:$vmdisk",
> +	"$storagename:$vmbase",
> +	"$storagename:$vmbase/$vmlinked",
> +	"$storagename:$ctdisk",
> +	"$storagename:$ctbase",
> +	"$storagename:$ctbase/$ctlinked",
> +    ];
> +
> +    PVE::Storage::activate_volumes($cfg, $vollist);
>  }
>  
>  sub cleanup_zfs {
> -- 
> 2.20.1