Re: [pve-devel] [PATCH/RFC storage 1/3] postinst: move cifs credential files into subdirectory upon update

[Fabian Ebner <f.ebner@proxmox.com>]

Am 08.06.21 um 15:18 schrieb Fabian Grünbichler:
> On June 4, 2021 3:49 pm, Fabian Ebner wrote:
>> and drop the compat code.
>>
>> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
>> ---
>>   PVE/Storage/CIFSPlugin.pm |  3 ---
>>   debian/postinst           | 30 ++++++++++++++++++++++++++++++
>>   2 files changed, 30 insertions(+), 3 deletions(-)
>>   create mode 100644 debian/postinst
>>
>> diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
>> index be06cc7..9d69b01 100644
>> --- a/PVE/Storage/CIFSPlugin.pm
>> +++ b/PVE/Storage/CIFSPlugin.pm
>> @@ -59,9 +59,6 @@ sub get_cred_file {
>>   
>>       if (-e $cred_file) {
>>   	return $cred_file;
>> -    } elsif (-e "/etc/pve/priv/${storeid}.cred") {
>> -	# FIXME: remove fallback with 7.0 by doing a rename on upgrade from 6.x
>> -	return "/etc/pve/priv/${storeid}.cred";
>>       }
>>       return undef;
>>   }
>> diff --git a/debian/postinst b/debian/postinst
>> new file mode 100644
>> index 0000000..e2453d2
>> --- /dev/null
>> +++ b/debian/postinst
>> @@ -0,0 +1,30 @@
>> +#!/bin/sh
>> +
>> +set -e
>> +
>> +#DEBHELPER#
>> +
>> +case "$1" in
>> +  configure)
>> +    if test -n "$2"; then
>> +
>> +        # TODO: remove once PVE 7.0 is released
> 
> s/7/8/ ?
> 
> because we need to do this renaming on all upgrades of 6.x to 7.x, no
> matter how far along in the 7.x release we are.. for the upgrade from
> 7.x to 8.x we can then be sure that the only such files are leftover
> garbage ;)
> 

Right, it's not like everybody has to go through a specific version.

>> +        if dpkg --compare-versions "$2" 'lt' '7.0-3'; then
>> +            for file in /etc/pve/priv/*.cred; do
>> +                if [ -f "$file" ]; then
>> +                    base=$(basename --suffix=".cred" "$file")
>> +                    if [ -f "/etc/pve/priv/storage/$base.pw" ]; then
>> +                        echo "Warning: found obsolete CIFS credentials file: $file" >&2
>> +                        echo "File: /etc/pve/priv/storage/$base.pw also exists" >&2
>> +                    else
>> +                        cp "$file" "/etc/pve/priv/storage/$base.pw" && rm "$file"
> 
> why not 'mv'?
> 

Bad muscle memory. I got used to this for other scenarios, but since 
it's only a single file and no file-system boundaries are involved here, 
mv is of course better.

> should we maybe print an info in both cases? e.g.
> 
> Warning: found possibly obsolete CIFS credentials file '$file', but '...' already exists.
> 
> and
> 
> Info: found CIFS credentials file '$file', moving to '...'.
> 

Ok, sure.

>> +                    fi
>> +                fi
>> +            done
>> +        fi
>> +    fi
>> +    ;;
>> +
>> +esac
>> +
>> +exit 0
>> -- 
>> 2.30.2
>>
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>
>>
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>