[pve-devel] [PATCH storage 1/6] dir plugin: update notes: don't attempt to remove non-existent notes

Fabian Ebner f.ebner at proxmox.com
Fri Sep 24 12:40:31 CEST 2021


Am 24.09.21 um 11:03 schrieb Thomas Lamprecht:
> On 24.09.21 10:54, Dominik Csapak wrote:
>> On 9/17/21 15:02, Fabian Ebner wrote:
>>> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
>>> ---
>>>    PVE/Storage/DirPlugin.pm | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm
>>> index 2267f11..0423e5f 100644
>>> --- a/PVE/Storage/DirPlugin.pm
>>> +++ b/PVE/Storage/DirPlugin.pm
>>> @@ -109,7 +109,7 @@ sub update_volume_notes {
>>>          if (defined($notes) && $notes ne '') {
>>>        PVE::Tools::file_set_contents($path, $notes);
>>> -    } else {
>>> +    } elsif (-e $path) {
>>>        unlink $path or die "could not delete notes - $!\n";
>>>        }
>>>        return;
>>>
>>
>> nit: it is still racy, and imho the correct solution would be to
>> ignore the error but only if the file did not exists
>> iow $! is ENOENT
>>

Good point.

>> for most cases it's enough though and i am not sure if the
>> added complexity is worth it...
> 
> IMO it's worth it and it's really not much complexity..
> 
> use POSIX; # or `use POSIX qw(ENOENT);` if we filter default-exports from the POSIX module already
> 
> unlink $path;
> die "could not delete notes - $!\n" if $! && $! != ENOENT;
> 

Ok, I'll do that in v2.





More information about the pve-devel mailing list