| Message ID | 20191212041513.13855-13-allison.henderson@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: Delay Ready Attributes | expand |
On Wed, Dec 11, 2019 at 09:15:11PM -0700, Allison Collins wrote: > Delayed operations cannot return error codes. So we must check for > these conditions first before starting set or remove operations > > Signed-off-by: Allison Collins <allison.henderson@oracle.com> > --- > fs/xfs/libxfs/xfs_attr.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index a3dd620..b5a5c84 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -446,6 +446,18 @@ xfs_attr_set( > goto out_trans_cancel; > > xfs_trans_ijoin(args.trans, dp, 0); > + > + error = xfs_has_attr(&args); > + if (error == -EEXIST) { > + if (name->type & ATTR_CREATE) > + goto out_trans_cancel; > + else > + name->type |= ATTR_REPLACE; Hmm.. this bit looks funny to me. Shouldn't this already be set by userspace? What's the reason for setting it here? Brian > + } > + > + if (error == -ENOATTR && (name->type & ATTR_REPLACE)) > + goto out_trans_cancel; > + > error = xfs_attr_set_args(&args); > if (error) > goto out_trans_cancel; > @@ -534,6 +546,10 @@ xfs_attr_remove( > */ > xfs_trans_ijoin(args.trans, dp, 0); > > + error = xfs_has_attr(&args); > + if (error != -EEXIST) > + goto out; > + > error = xfs_attr_remove_args(&args); > if (error) > goto out; > -- > 2.7.4 >
On 12/13/19 7:16 AM, Brian Foster wrote: > On Wed, Dec 11, 2019 at 09:15:11PM -0700, Allison Collins wrote: >> Delayed operations cannot return error codes. So we must check for >> these conditions first before starting set or remove operations >> >> Signed-off-by: Allison Collins <allison.henderson@oracle.com> >> --- >> fs/xfs/libxfs/xfs_attr.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c >> index a3dd620..b5a5c84 100644 >> --- a/fs/xfs/libxfs/xfs_attr.c >> +++ b/fs/xfs/libxfs/xfs_attr.c >> @@ -446,6 +446,18 @@ xfs_attr_set( >> goto out_trans_cancel; >> >> xfs_trans_ijoin(args.trans, dp, 0); >> + >> + error = xfs_has_attr(&args); >> + if (error == -EEXIST) { >> + if (name->type & ATTR_CREATE) >> + goto out_trans_cancel; >> + else >> + name->type |= ATTR_REPLACE; > > Hmm.. this bit looks funny to me. Shouldn't this already be set by > userspace? What's the reason for setting it here? > > Brian There was a reason I did this, and now i'm trying to remember what it was. I'm pretty sure it's reminiscent of an earlier series that handled a replace with explicit remove and set to avoid in flight error codes with delayed operations. I think I can take it out at this point though, the set has gone through a lot of changes since. Allison > >> + } >> + >> + if (error == -ENOATTR && (name->type & ATTR_REPLACE)) >> + goto out_trans_cancel; >> + >> error = xfs_attr_set_args(&args); >> if (error) >> goto out_trans_cancel; >> @@ -534,6 +546,10 @@ xfs_attr_remove( >> */ >> xfs_trans_ijoin(args.trans, dp, 0); >> >> + error = xfs_has_attr(&args); >> + if (error != -EEXIST) >> + goto out; >> + >> error = xfs_attr_remove_args(&args); >> if (error) >> goto out; >> -- >> 2.7.4 >> >
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index a3dd620..b5a5c84 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -446,6 +446,18 @@ xfs_attr_set( goto out_trans_cancel; xfs_trans_ijoin(args.trans, dp, 0); + + error = xfs_has_attr(&args); + if (error == -EEXIST) { + if (name->type & ATTR_CREATE) + goto out_trans_cancel; + else + name->type |= ATTR_REPLACE; + } + + if (error == -ENOATTR && (name->type & ATTR_REPLACE)) + goto out_trans_cancel; + error = xfs_attr_set_args(&args); if (error) goto out_trans_cancel; @@ -534,6 +546,10 @@ xfs_attr_remove( */ xfs_trans_ijoin(args.trans, dp, 0); + error = xfs_has_attr(&args); + if (error != -EEXIST) + goto out; + error = xfs_attr_remove_args(&args); if (error) goto out;
Delayed operations cannot return error codes. So we must check for these conditions first before starting set or remove operations Signed-off-by: Allison Collins <allison.henderson@oracle.com> --- fs/xfs/libxfs/xfs_attr.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)