Re: [pbs-devel] [PATCH v8 proxmox-backup 4/9] fix #5982: garbage collection: check atime updates are honored

["Max Carrara" <m.carrara@proxmox.com>]

On Sat Apr 5, 2025 at 11:16 AM CEST, Christian Ebner wrote:
> On 4/4/25 17:41, Max Carrara wrote:
> > Regarding my comment in the cover letter: The call to
> > `self.cond_touch_path` above will fail on a directory-based datastore
> > backed by ext4 mounted with -o rw,relatime:
> > 
> >      2025-04-04T16:25:44+02:00: TASK ERROR: atime safety check failed: update atime failed for chunk/file "/mnt/datastore/test-noatime/.chunks/bb9f/bb9f8df61474d25e71fa00722318cd387396ca1736605e1248821cc0de3d3af8" - EPERM: Operation not permitted
> > 
> >  From `man 2 utimensat` (formatted for your reading pleasure):
> > 
> > EPERM
> >      The caller attempted to change one or both timestamps to a value
> >      other than the current time, or to change one of the timestamps to
> >      the current time while leaving the other timestamp unchanged, (i.e.,
> >      times  is  not NULL, neither tv_nsec field is UTIME_NOW, and neither
> >      tv_nsec field is UTIME_OMIT) and either:
> > 
> >      •  the caller's effective user ID does not match the owner of file,
> >         and the caller is not privileged (Linux: does not have the
> >         CAP_FOWNER capability); or,
> > 
> >      •  the file is marked append-only or immutable (see chattr(1)).
> > 
> > Sooo, I investigated a little more, since the EPERM here initially
> > didn't make any sense to me, because the same atime check actually
> > passes when creating the datastore.
> > 
> > Furthermore, the file (chunk) used for this check isn't append-only or
> > immutable (otherwise you'd see an `a` or `i` down there):
> > 
> >      $ lsattr .chunks/bb9f/bb9f8df61474d25e71fa00722318cd387396ca1736605e1248821cc0de3d3af8
> >      --------------e------- .chunks/bb9f/bb9f8df61474d25e71fa00722318cd387396ca1736605e1248821cc0de3d3af8
> > 
> > 
> > And finally, just for completeness' sake, we're using both UTIME_NOW and
> > UTIME_OMIT in `cond_touch_path`:
> > 
> >      let times: [libc::timespec; 2] = [
> >          // access time -> update to now
> >          libc::timespec {
> >              tv_sec: 0,
> >              tv_nsec: libc::UTIME_NOW,
> >          },
> >          // modification time -> keep as is
> >          libc::timespec {
> >              tv_sec: 0,
> >              tv_nsec: libc::UTIME_OMIT,
> >          },
> >      ];
> > 
> > 
> > According to the docs of `man 2 utimensat`, this would suggest that
> > we're not running under the expected effective UID and don't have
> > the CAP_FOWNER capability.
> > 
> > This is the actual crux of the issue, since the chunk is owned by
> > `root`:
> > 
> >      $ ls -alh .chunks/bb9f
> >      total 1.1M
> >      drwxr-x--- 2 backup backup 4.0K Apr  4 17:11 .
> >      drwxr-x--- 1 backup backup 1.1M Apr  4 17:11 ..
> >      -rw-r--r-- 1 root   root    159 Apr  4 17:11 bb9f8df61474d25e71fa00722318cd387396ca1736605e1248821cc0de3d3af8
> > 
> > 
> > Just to add some additional findings -- when the *check is turned off*,
> > GC on that single chunk (no backups made yet!) runs just fine:
> > 
> >      2025-04-04T17:00:52+02:00: starting garbage collection on store test-noatime
> >      2025-04-04T17:00:52+02:00: Access time update check disabled by datastore tuning options.
> >      2025-04-04T17:00:52+02:00: Using access time cutoff 1d 5m, minimum access time is 2025-04-03T14:55:52Z
> >      2025-04-04T17:00:52+02:00: Start GC phase1 (mark used chunks)
> >      2025-04-04T17:00:52+02:00: Start GC phase2 (sweep unused chunks)
> >      2025-04-04T17:00:52+02:00: processed 73% (0 chunks)
> >      2025-04-04T17:00:52+02:00: Removed garbage: 0 B
> >      2025-04-04T17:00:52+02:00: Removed chunks: 0
> >      2025-04-04T17:00:52+02:00: Pending removals: 159 B (in 1 chunks)
> >      2025-04-04T17:00:52+02:00: Original data usage: 0 B
> >      2025-04-04T17:00:52+02:00: On-Disk chunks: 0
> >      2025-04-04T17:00:52+02:00: Deduplication factor: 1.00
> >      2025-04-04T17:00:52+02:00: queued notification (id=4bab0755-3f24-4b1a-99b0-de8e9613db9b)
> >      2025-04-04T17:00:52+02:00: TASK OK
> > 
> > *BUT*, when creating a backup via PVE, the check actually runs, despite
> > being turned off -- this appears to be something separate that you might
> > want to check as well:
>
> That is expected if the chunk has the wrong ownership, but not because 
> the fs_atime_check is executed here. Rather this happens because your VM 
> backup includes an all zero chunk, does not have it as known chunk 
> however (since the is no previous index file). This means the chunk will 
> be uploaded to the PBS, but the chunk store detects it as already 
> present. In that case the chunks atime will be updated too, but that 
> then fails because of the permission issue. See the corresponding code 
> paths here:
>   https://git.proxmox.com/?p=proxmox-backup.git;a=blob;f=pbs-datastore/src/chunk_store.rs;h=dc267d752980a988d22e6d410855d2ceb3c8db46;hb=HEAD#l460

Ah yes, of course! I don't know why I didn't think of that. Thanks for
the clarification!

>
> > 
> >      INFO: starting new backup job: vzdump 100 --mode snapshot --notes-template '{{guestname}}' --remove 0 --node roxy --notification-mode auto --storage pbs-dev-test-atime
> >      INFO: Starting Backup of VM 100 (qemu)
> >      INFO: Backup started at 2025-04-04 17:02:49
> >      INFO: status = running
> >      INFO: VM Name: dns
> >      INFO: include disk 'scsi0' 'local-zfs-main:vm-100-disk-0' 16G
> >      INFO: backup mode: snapshot
> >      INFO: ionice priority: 7
> >      INFO: snapshots found (not included into backup)
> >      INFO: creating Proxmox Backup Server archive 'vm/100/2025-04-04T15:02:49Z'
> >      INFO: issuing guest-agent 'fs-freeze' command
> >      INFO: issuing guest-agent 'fs-thaw' command
> >      ERROR: VM 100 qmp command 'backup' failed - backup register image failed: command error: update atime failed for chunk/file "/mnt/datastore/test-noatime/.chunks/bb9f/bb9f8df61474d25e71fa00722318cd387396ca1736605e1248821cc0de3d3af8" - EPERM: Operation not permitted
> >      INFO: aborting backup job
> >      INFO: resuming VM again
> >      ERROR: Backup of VM 100 failed - VM 100 qmp command 'backup' failed - backup register image failed: command error: update atime failed for chunk/file "/mnt/datastore/test-noatime/.chunks/bb9f/bb9f8df61474d25e71fa00722318cd387396ca1736605e1248821cc0de3d3af8" - EPERM: Operation not permitted
> >      INFO: Failed at 2025-04-04 17:02:49
> >      INFO: Backup job finished with errors
> >      TASK ERROR: job errors
> > 
> > Just to be sure:
> > 
> >      $ cat /etc/proxmox-backup/datastore.cfg
> >      # [...]
> >      datastore: test-noatime
> >              comment
> >              gc-schedule daily
> >              notification-mode notification-system
> >              path /mnt/datastore/test-noatime
> >              tuning gc-atime-safety-check=0
> > 
> > 
> > 
> > So, fixing the permissions with:
> > 
> >      # chown backup: .chunks/bb9f/bb9f8df61474d25e71fa00722318cd387396ca1736605e1248821cc0de3d3af8
> > 
> > .. allows the check to pass:
> > 
> >      2025-04-04T17:14:34+02:00: starting garbage collection on store test-noatime
> >      2025-04-04T17:14:35+02:00: Access time update check successful, proceeding with GC.
> >      2025-04-04T17:14:35+02:00: Using access time cutoff 1d 5m, minimum access time is 2025-04-03T15:09:34Z
> >      2025-04-04T17:14:35+02:00: Start GC phase1 (mark used chunks)
> >      2025-04-04T17:14:35+02:00: Start GC phase2 (sweep unused chunks)
> >      2025-04-04T17:14:35+02:00: processed 73% (0 chunks)
> >      2025-04-04T17:14:35+02:00: Removed garbage: 0 B
> >      2025-04-04T17:14:35+02:00: Removed chunks: 0
> >      2025-04-04T17:14:35+02:00: Original data usage: 0 B
> >      2025-04-04T17:14:35+02:00: On-Disk chunks: 1
> >      2025-04-04T17:14:35+02:00: Deduplication factor: 0.00
> >      2025-04-04T17:14:35+02:00: Average chunk size: 159 B
> >      2025-04-04T17:14:35+02:00: queued notification (id=06d76349-26af-43da-9ddb-5b0ee2fabfa8)
> >      2025-04-04T17:14:35+02:00: TASK OK
> > 
> > So, guess you just have to change the UID and GID when creating the
> > test chunk ;P
>
> Yes, in v9 of the series, the chunk files ownership will be set if an 
> `root`s UID is detected as effective UID. This fixes the issue here and 
> makes the chunk insertion more robust in general IMHO.
>
> Thanks again for catching this early! If you manage to give it another 
> spin would be great.

Yeah I saw that--and it's already applied, too. So, guess another spin's
not needed ;)



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel