Project

General

Profile

Actions
Copy link

Bug #22801

closed

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

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

Status:
Resolved
Priority:
Normal
Assignee:
Jeff Layton
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 1 (0 open — 1 closed)

Blocks CephFS - Bug #22802: libcephfs: allow setting default permsResolvedJeff Layton01/25/2018

Actions
Actions
Copy link
 #1

Updated by Patrick Donnelly over 6 years ago

  • Blocks Bug #22802: libcephfs: allow setting default perms added
Actions
Copy link
 #2

Updated by Jeff Layton over 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
Actions
Copy link
 #3

Updated by Jeff Layton over 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
Actions
Copy link
 #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?

Actions
Copy link
 #5

Updated by Zheng Yan about 6 years ago

I think we should cap_dirtier_uid/gid into CapSnap

Actions
Copy link
 #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?

Actions
Copy link
 #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

Actions
Copy link
 #8

Updated by Patrick Donnelly about 6 years ago

  • Status changed from New to Resolved
Actions
Copy link

Also available in: Atom PDF