all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Fabian Ebner <f.ebner@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
	Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Dominik Csapak <d.csapak@proxmox.com>
Subject: Re: [pve-devel] [PATCH storage 1/6] dir plugin: update notes: don't attempt to remove non-existent notes
Date: Fri, 24 Sep 2021 12:40:31 +0200	[thread overview]
Message-ID: <00d4a32f-0b23-83b0-c09b-fe05ab4d69ed@proxmox.com> (raw)
In-Reply-To: <a5b8645d-222c-06b0-c753-031242fdd0b3@proxmox.com>

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@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.




  reply	other threads:[~2021-09-24 10:40 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-17 13:02 [pve-devel] [RFC storage/manager] fix #3307: allow backups to be marked as protected Fabian Ebner
2021-09-17 13:02 ` [pve-devel] [PATCH storage 1/6] dir plugin: update notes: don't attempt to remove non-existent notes Fabian Ebner
2021-09-24  8:54   ` Dominik Csapak
2021-09-24  9:03     ` Thomas Lamprecht
2021-09-24 10:40       ` Fabian Ebner [this message]
2021-09-17 13:02 ` [pve-devel] [RFC storage 2/6] add generalized functions to manage volume attributes Fabian Ebner
2021-09-24  8:54   ` Dominik Csapak
2021-09-24 11:05     ` Fabian Ebner
2021-09-24 11:16       ` Dominik Csapak
2021-09-24 11:31         ` Fabian Ebner
2021-09-17 13:02 ` [pve-devel] [PATCH storage 3/6] prune mark: preserve additional information for the keep-all case Fabian Ebner
2021-09-17 13:02 ` [pve-devel] [RFC storage 4/6] fix #3307: make it possible to set protection for backups Fabian Ebner
2021-09-24  8:54   ` Dominik Csapak
2021-09-24 11:17     ` Fabian Ebner
2021-09-17 13:02 ` [pve-devel] [RFC storage 5/6] prune: mark renamed and protected backups differently Fabian Ebner
2021-09-17 13:02 ` [pve-devel] [RFC storage 6/6] pbs: integrate support for protected Fabian Ebner
2021-09-24  8:55   ` Dominik Csapak
2021-09-24 11:32     ` Fabian Ebner
2021-09-24 11:48       ` Dominik Csapak
2021-09-17 13:02 ` [pve-devel] [RFC manager 1/1] vzdump: skip protected backups for dumpdir pruning Fabian Ebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=00d4a32f-0b23-83b0-c09b-fe05ab4d69ed@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=t.lamprecht@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal