[pve-devel] applied: [PATCH v2 manager 2/2] pve6to7: more fine-grained detection of misconfigured guest volumes

[Fabian Grünbichler]

On June 30, 2021 11:16 am, Fabian Ebner wrote:
> If neither 'rootdir' nor 'images' are configured on a storage, but
> there are guest images, just log the number of volumes found. If they
> are relevant for migration, the check for unreferenced volumes will
> catch them later.
> 
> Also detect content type mismatch for all volumes of existing guests,
> which also covers the case of a VM image on a storage with only
> 'rootdir' and vice versa. To catch all such unreferenced volumes too,
> it is necessary to scan all storages that do not have both content
> types configured.
> 
> Change the message from 'will not work' to 'might not work'. If a
> volume only referenced by a snapshot is misconfigured, it doesn't mean
> that the guest doesn't work at all. Or it might be an ISO on a
> misconfigured storage.
> 
> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
> ---
> 
> Changes from v1 (mostly thanks to Fabian G.'s off-list feedback):
>     * Add eval around vdisk_list and parse_volname.
>     * Activate storage first, because vdisk_list does not do that
>       for storages with neither 'rootdir' nor 'images (although it
>       still scanned for disks...).
>     * Also warn about unreferenced volumes on a misconfigured storage
>       if a guest exists. Put a second final warning explaining that
>       they are not picked up for migration anymore.
>     * Don't put single quotes around the number of volumes in ouput
>       (looked weird).
>     * Use "Proxmox VE" instead of "PVE" in message.
> 
>  PVE/CLI/pve6to7.pm | 154 +++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 141 insertions(+), 13 deletions(-)
> 
> diff --git a/PVE/CLI/pve6to7.pm b/PVE/CLI/pve6to7.pm
> index 3d5b780b..6dd7760c 100644
> --- a/PVE/CLI/pve6to7.pm
> +++ b/PVE/CLI/pve6to7.pm
> @@ -706,11 +706,15 @@ sub check_description_lengths {
>  sub check_storage_content {
>      log_info("Checking storage content type configuration..");
>  
> -    my $found;
> +    my $found_referenced;
> +    my $found_unreferenced;
>      my $pass = 1;
>  
>      my $storage_cfg = PVE::Storage::config();
>  
> +    my $potentially_affected = {};
> +    my $referenced_volids = {};
> +
>      for my $storeid (keys $storage_cfg->{ids}->%*) {
>  	my $scfg = $storage_cfg->{ids}->{$storeid};
>  
> @@ -724,8 +728,7 @@ sub check_storage_content {
>  	    delete $scfg->{content}->{none}; # scan for guest images below
>  	}
>  
> -	next if $scfg->{content}->{images};
> -	next if $scfg->{content}->{rootdir};
> +	next if $scfg->{content}->{images} && $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.
> @@ -733,22 +736,147 @@ sub check_storage_content {
>  	# and 'images' or 'rootdir', hence being potentially misconfigured.
>  	next if $scfg->{type} ne 'dir' && $scfg->{content}->{none};
>  
> -	my $res = PVE::Storage::vdisk_list($storage_cfg, $storeid);
> -	my $disk_list = $res->{$storeid};
> +	eval { PVE::Storage::activate_storage($storage_cfg, $storeid) };
> +	if (my $err = $@) {
> +	    log_warn("activating '$storeid' failed - $err");
> +	    next;
> +	}
> +
> +	my $res = eval { PVE::Storage::vdisk_list($storage_cfg, $storeid); };
> +	if (my $err = $@) {
> +	    log_warn("listing images on '$storeid' failed - $err");
> +	    next;
> +	}
> +	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}) {
> +	    log_info("storage '$storeid' - neither content type 'images' nor 'rootdir' configured"
> +		.", but found $number guest volume(s)");
> +	}
> +    }
> +
> +    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)";
>  
> -	my @volumes = map { $_->{volid} } $disk_list->@*;
> +	my ($storeid) = PVE::Storage::parse_volume_id($volid, 1);
> +	return if !defined($storeid);
>  
> -	if (scalar(@volumes) > 0) {
> -	    $found = 1;
> +	my $scfg = $storage_cfg->{ids}->{$storeid};
> +	if (!$scfg) {
>  	    $pass = 0;
> -	    log_warn("storage '$storeid' - neither content type 'images' nor 'rootdir' configured"
> -		.", but found guest volume(s):\n    " . join("\n    ", @volumes));
> +	    log_warn("$prefix - storage does not exist!");
> +	    return;
> +	}
> +
> +	# cannot use parse_volname for containers, as it can return 'images'
> +	# but containers cannot have ISO images attached, so assume 'rootdir'
> +	my $vtype = 'rootdir';
> +	if ($vmtype eq 'qemu') {
> +	    ($vtype) = eval { PVE::Storage::parse_volname($storage_cfg, $volid); };
> +	    return if $@;
>  	}
> +
> +	if (!$scfg->{content}->{$vtype}) {
> +	    $found_referenced = 1 if $reference ne 'unreferenced';
> +	    $found_unreferenced = 1 if $reference eq 'unreferenced';
> +	    $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 = {};
> +
> +	my $check = sub {
> +	    my ($ms, $mountpoint, $reference) = @_;
> +
> +	    my $volid = $mountpoint->{volume};
> +	    return if !$volid || $mountpoint->{type} ne 'volume';
> +
> +	    return if $volhash->{$volid}; # volume might be referenced multiple times
> +
> +	    $volhash->{$volid} = 1;
> +
> +	    $check_volid->($volid, $vmid, 'lxc', $reference);
> +	};
> +
> +	my $opts = { include_unused => 1 };
> +	PVE::LXC::Config->foreach_volume_full($conf, $opts, $check, 'in config');
> +	for my $snapname (keys $conf->{snapshots}->%*) {
> +	    my $snap = $conf->{snapshots}->{$snapname};
> +	    PVE::LXC::Config->foreach_volume_full($snap, $opts, $check, "in snapshot '$snapname'");
> +	}
> +    }
> +
> +    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 = {};
> +
> +	my $check = sub {
> +	    my ($key, $drive, $reference) = @_;
> +
> +	    my $volid = $drive->{file};
> +	    return if $volid =~ m|^/|;
> +
> +	    return if $volhash->{$volid}; # volume might be referenced multiple times
> +
> +	    $volhash->{$volid} = 1;
> +
> +	    $check_volid->($volid, $vmid, 'qemu', $reference);
> +	};
> +
> +	my $opts = {
> +	    extra_keys => ['vmstate'],
> +	    include_unused => 1,
> +	};
> +	# startup from a suspended state works even without 'images' content type on the
> +	# state storage, so do not check 'vmstate' for $conf
> +	PVE::QemuConfig->foreach_volume_full($conf, { include_unused => 1 }, $check, 'in config');
> +	for my $snapname (keys $conf->{snapshots}->%*) {
> +	    my $snap = $conf->{snapshots}->{$snapname};
> +	    PVE::QemuConfig->foreach_volume_full($snap, $opts, $check, "in snapshot '$snapname'");
> +	}
> +    }
> +
> +    if ($found_referenced) {
> +	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) {
> -	log_warn("PVE 7.0 enforces stricter content type checks. Guests referencing the above " .
> -	    "volumes will not work until the storage configuration is fixed.");
> +    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) {
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>