On Tue, Apr 09, 2024 at 10:16:58PM -0700, Christoph Hellwig wrote:
> On Tue, Apr 09, 2024 at 05:55:20PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > Add a new XFS_DA_OP_PARENT flag to signal that the caller wants to look
>
> The flag doesn't actually exist, the match is done on the
> XFS_ATTR_PARENT namespaces.
How about:
"xfs: allow xattr matching on name and value for local/sf pptr attrs
"If a file is hardlinked with the same name but from multiple parents,
the parent pointers will all have the same dirent name (== attr name)
but with different parent_ino/parent_gen values. To disambiguate, we
need to be able to match on both the attr name and the attr value. This
is in contrast to regular xattrs, which are matched only on name.
"Therefore, plumb in the ability to match shortform and local attrs on
name and value in the XFS_ATTR_PARENT namespace. Parent pointer attr
values are never large enough to be stored in a remote attr, so we need
can reject these cases as corruption."
> >
> > @@ -2444,14 +2477,17 @@ xfs_attr3_leaf_lookup_int(
> > name_loc = xfs_attr3_leaf_name_local(leaf, probe);
> > if (!xfs_attr_match(args, entry->flags,
> > name_loc->nameval,
> > - name_loc->namelen))
> > + name_loc->namelen,
> > + &name_loc->nameval[name_loc->namelen],
> > + be16_to_cpu(name_loc->valuelen)))
>
> If we'd switch from the odd pre-existing three-tab indent to the normal
> two-tab indent we'd avoid the overly long line here.
>
> > continue;
> > args->index = probe;
> > return -EEXIST;
> > } else {
> > name_rmt = xfs_attr3_leaf_name_remote(leaf, probe);
> > if (!xfs_attr_match(args, entry->flags, name_rmt->name,
> > - name_rmt->namelen))
> > + name_rmt->namelen, NULL,
> > + be32_to_cpu(name_rmt->valuelen)))
>
> ... and here.
Believe it or not that's what vim autoformat does by default.
But yes, I'll reduce it to two indents to reduce the indentation and
overflow.
> The remote side might also benefit from a local variable to store the
> endian swapped version of the valuelen instead of calculating it twice.
Ok.
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
Thanks!
--D
