[pve-devel] [PATCH manager 2/2] pve6to7: storage content: ignore misconfigured unreferenced volumes

[Fabian Grünbichler]

On July 7, 2021 12:22 pm, Fabian Ebner wrote:
> If the same local storage is configured twice with content type
> separation, migration in PVE 6 would lead to the volumes being
> duplicated. As that would happen for every migration, such an issue
> would likely be noticed already, and in PVE 7 such configuration is
> not problematic for migration anymore. Also, misconfigured
> unreferenced volumes are not an issue with respect to the upgrade
> itself, just drop the check.

but those checks also catch storages that are misconfigured for which no 
such inverse storage with opposite content type and otherwise identical 
settings exists? we can't just drop them altogether?

we COULD skip them conditionally for storage pairs (same type, same 
'path'/pool/pool+mons/.., one with images on with rootfs), but such 
setups are still wrong and not properly separated IMHO. and stuff like 
dir-storage on-top of other dir-like storage with volumes stored in the 
same path are not really "detectable" in a reliable and cheap fashion 
and should really be fixed by the user (e.g., by moving the dir storage 
into a non-confusable sub-dir of the backing storage).

> 
> It's not necessary to scan storages with either 'images' or 'rootdir'
> anymore, as only the log_info() remains.
> 
> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
> ---
>  PVE/CLI/pve6to7.pm | 43 ++++++-------------------------------------
>  1 file changed, 6 insertions(+), 37 deletions(-)
> 
> diff --git a/PVE/CLI/pve6to7.pm b/PVE/CLI/pve6to7.pm
> index 17da70e8..7d7b09d2 100644
> --- a/PVE/CLI/pve6to7.pm
> +++ b/PVE/CLI/pve6to7.pm
> @@ -695,15 +695,11 @@ sub check_description_lengths {
>  sub check_storage_content {
>      log_info("Checking storage content type configuration..");
>  
> -    my $found_referenced;
> -    my $found_unreferenced;
> +    my $found;
>      my $pass = 1;
>  
>      my $storage_cfg = PVE::Storage::config();
>  
> -    my $potentially_affected = {};
> -    my $referenced_volids = {};
> -
>      for my $storeid (sort keys $storage_cfg->{ids}->%*) {
>  	my $scfg = $storage_cfg->{ids}->{$storeid};
>  
> @@ -718,7 +714,8 @@ sub check_storage_content {
>  	    delete $scfg->{content}->{none}; # scan for guest images below
>  	}
>  
> -	next if $scfg->{content}->{images} && $scfg->{content}->{rootdir};
> +	next if $scfg->{content}->{images};
> +	next if $scfg->{content}->{rootdir};
>  
>  	# Skip 'iscsi(direct)' (and foreign plugins with potentially similiar behavior) with 'none',
>  	# because that means "use LUNs directly" and vdisk_list() in PVE 6.x still lists those.
> @@ -739,12 +736,8 @@ sub check_storage_content {
>  	}
>  	my @volids = map { $_->{volid} } $res->{$storeid}->@*;
>  
> -	for my $volid (@volids) {
> -	    $potentially_affected->{$volid} = 1;
> -	}
> -
>  	my $number = scalar(@volids);
> -	if ($number > 0 && !$scfg->{content}->{images} && !$scfg->{content}->{rootdir}) {
> +	if ($number > 0) {
>  	    log_info("storage '$storeid' - neither content type 'images' nor 'rootdir' configured"
>  		.", but found $number guest volume(s)");
>  	}
> @@ -753,8 +746,6 @@ sub check_storage_content {
>      my $check_volid = sub {
>  	my ($volid, $vmid, $vmtype, $reference) = @_;
>  
> -	$referenced_volids->{$volid} = 1 if $reference ne 'unreferenced';
> -
>  	my $guesttext = $vmtype eq 'qemu' ? 'VM' : 'CT';
>  	my $prefix = "$guesttext $vmid - volume '$volid' ($reference)";
>  
> @@ -777,19 +768,14 @@ sub check_storage_content {
>  	}
>  
>  	if (!$scfg->{content}->{$vtype}) {
> -	    $found_referenced = 1 if $reference ne 'unreferenced';
> -	    $found_unreferenced = 1 if $reference eq 'unreferenced';
> +	    $found = 1;
>  	    $pass = 0;
>  	    log_warn("$prefix - storage does not have content type '$vtype' configured.");
>  	}
>      };
>  
> -    my $guests = {};
> -
>      my $cts = PVE::LXC::config_list();
>      for my $vmid (sort { $a <=> $b } keys %$cts) {
> -	$guests->{$vmid} = 'lxc';
> -
>  	my $conf = PVE::LXC::Config->load_config($vmid);
>  
>  	my $volhash = {};
> @@ -817,8 +803,6 @@ sub check_storage_content {
>  
>      my $vms = PVE::QemuServer::config_list();
>      for my $vmid (sort { $a <=> $b } keys %$vms) {
> -	$guests->{$vmid} = 'qemu';
> -
>  	my $conf = PVE::QemuConfig->load_config($vmid);
>  
>  	my $volhash = {};
> @@ -849,26 +833,11 @@ sub check_storage_content {
>  	}
>      }
>  
> -    if ($found_referenced) {
> +    if ($found) {
>  	log_warn("Proxmox VE 7.0 enforces stricter content type checks. The guests above " .
>  	    "might not work until the storage configuration is fixed.");
>      }
>  
> -    for my $volid (sort keys $potentially_affected->%*) {
> -	next if $referenced_volids->{$volid}; # already checked
> -
> -	my (undef, undef, $vmid) = PVE::Storage::parse_volname($storage_cfg, $volid);
> -	my $vmtype = $guests->{$vmid};
> -	next if !$vmtype;
> -
> -	$check_volid->($volid, $vmid, $vmtype, 'unreferenced');
> -    }
> -
> -    if ($found_unreferenced) {
> -	log_warn("When migrating, Proxmox VE 7.0 only scans storages with the appropriate " .
> -	    "content types for unreferenced guest volumes.");
> -    }
> -
>      if ($pass) {
>  	log_pass("no problems found");
>      }
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>