[pve-devel] [PATCH storage] pvesm set: handle deletion of properties

[Thomas Lamprecht]

On 5/15/19 10:14 AM, Dominik Csapak wrote:
> works like expected, there is some issue in the context though (comment inline)
> 
> On 5/14/19 4:24 PM, Thomas Lamprecht wrote:
>> the delete parameter get's injected by the SectionConfigs
>> updateSchem, but we need to handle it ourself in the code
>> This makes the following possible:
>>
>> pvesm set STORAGEID --delete property
>>
>> Also the API equivalent is now possible. Adapted from the HA
>> managers Resource update API call.
>>
>> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
>> ---
>>   PVE/API2/Storage/Config.pm | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/PVE/API2/Storage/Config.pm b/PVE/API2/Storage/Config.pm
>> index c114ddc..f5e1560 100755
>> --- a/PVE/API2/Storage/Config.pm
>> +++ b/PVE/API2/Storage/Config.pm
>> @@ -195,6 +195,7 @@ __PACKAGE__->register_method ({
>>         my $storeid = extract_param($param, 'storage');
>>       my $digest = extract_param($param, 'digest');
>> +    my $delete = extract_param($param, 'delete');
>>             PVE::Storage::lock_storage_config(
>>        sub {
>> @@ -208,6 +209,20 @@ __PACKAGE__->register_method ({
>>           my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
>>           my $opts = $plugin->check_config($storeid, $param, 0, 1);
>>   +        if ($delete) {
>> +        my $options = $plugin->private()->{options}->{$scfg->{type}};
>> +        foreach my $k (PVE::Tools::split_list($delete)) {
>> +            my $d = $options->{$k} || die "no such option '$k'\n";
>> +            die "unable to delete required option '$k'\n" if !$d->{optional};
>> +            die "unable to delete fixed option '$k'\n" if $d->{fixed};
>> +            die "cannot set and delete property '$k' at the same time!\n"
>> +            if defined($opts->{$k});
>> +
>> +            delete $scfg->{$k};
>> +        }
>> +        }
>> +
>> +
>>           foreach my $k (%$opts) {
> 
> here we ought to do (keys %$opts), else we use
> even the values as keys...
> 
> shall i send a patch which is based on yours? can you do it as a fixup?

let's save the overhead, I'll fix it up with Suggested-by tag for you.

Thanks for review, and noticing the issue in existing context!

> 
>>           $scfg->{$k} = $opts->{$k};
>>           }
>>