Project

General

Profile

Bug #22801

client: Client::flush_snaps still uses obsolete Client::user_id/group_id

Added by Patrick Donnelly about 6 years ago. Updated about 6 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
-
% Done:

0%

Source:
Development
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Component(FS):
Client
Labels (FS):
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

This appears to be a hold-out from the UserPerm work last year and the last remaining user of Client::user_id|user_gid (with the exception of SyntheticClient).


Related issues

Blocks CephFS - Bug #22802: libcephfs: allow setting default perms Resolved 01/25/2018

History

#1 Updated by Patrick Donnelly about 6 years ago

  • Blocks Bug #22802: libcephfs: allow setting default perms added

#2 Updated by Jeff Layton about 6 years ago

  • Subject changed from client: Client::flush_snaps still uses obsolete user_id/user_gid to client: Client::flush_snaps still uses obsolete Client::user_id/group_gid

#3 Updated by Jeff Layton about 6 years ago

  • Subject changed from client: Client::flush_snaps still uses obsolete Client::user_id/group_gid to client: Client::flush_snaps still uses obsolete Client::user_id/group_id

#4 Updated by Jeff Layton about 6 years ago

I don't have the best grasp of how snapshots work in cephfs, so I'm a little confused as to what should be done here:

flush_snaps is called from all sorts of async contexts, so we can't rely on the thread context to get credentials. send_cap sets those values to in->cap_dirtier_uid/gid. Should we do the same here?

#5 Updated by Zheng Yan about 6 years ago

I think we should cap_dirtier_uid/gid into CapSnap

#6 Updated by Jeff Layton about 6 years ago

Hmm...cap_dirtier_uid is only set to anything non-default in _do_setattr(). That function does this very early:

  if (in->snapid != CEPH_NOSNAP) {
    return -EROFS;
  }

So it looks like it'd only ever be set to -1 on a snapshot? If so, then maybe we should just have this flush_snaps set these values to -1?

#7 Updated by Zheng Yan about 6 years ago

CapSnap is for flushing snapshoted metadata (the metadata that were dirty at the time of mksnap), nothing do with setatrr on snap inode

#8 Updated by Patrick Donnelly about 6 years ago

  • Status changed from New to Resolved

Also available in: Atom PDF