On Thu, Feb 10, 2022 at 04:46:13PM -0600, Eric Sandeen wrote:
> On 2/3/22 11:45 AM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > The last of the DMAPI stubs were removed from Linux 5.17, so drop this
> > functionality altogether.
>
> Why is this better than letting it EINVAL/ENOTTY/ENOWHATEVER when the
> ioctl gets called?
5.17 removed the ioctl definitions, so xfsdump won't build anymore.
> Though I don't really care, so I will go ahead and
> review it. :)
>
> At this point I suppose finally pulling in Anthony's
> xfsdump: remove BMV_IF_NO_DMAPI_READ flag
> would make sense as well.
Yes.
>
> > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> > ---
> > doc/xfsdump.html | 1 -
> > po/de.po | 5 ---
> > po/pl.po | 5 ---
> > restore/content.c | 99 +++--------------------------------------------------
> > restore/tree.c | 33 ------------------
> > restore/tree.h | 1 -
> > 6 files changed, 6 insertions(+), 138 deletions(-)
> >
>
> ...
>
> > diff --git a/restore/content.c b/restore/content.c
> > index 6b22965..e9b0a07 100644
> > --- a/restore/content.c
> > +++ b/restore/content.c
> > @@ -477,9 +477,6 @@ struct pers {
> > /* how many pages following the header page are reserved
> > * for the subtree descriptors
> > */
> > - bool_t restoredmpr;
> > - /* restore DMAPI event settings
> > - */
> > bool_t restoreextattrpr;
> > /* restore extended attributes
> > */
> > @@ -858,7 +855,6 @@ static void partial_reg(ix_t d_index, xfs_ino_t ino, off64_t fsize,
> > off64_t offset, off64_t sz);
> > static bool_t partial_check (xfs_ino_t ino, off64_t fsize);
> > static bool_t partial_check2 (partial_rest_t *isptr, off64_t fsize);
> > -static int do_fssetdm_by_handle(char *path, fsdmidata_t *fdmp);
>
> with fsdmidata_t completely gone I think its typedef can go too?
MProbably.
> ...
>
> > @@ -8796,19 +8748,6 @@ restore_extattr(drive_t *drivep,
> > }
> > } else if (isfilerestored && path[0] != '\0') {
> > setextattr(path, ahdrp);
>
> Pretty sure there's a hunk in setextattr that could go too, right?
>
> @@ -8840,20 +8779,16 @@ restore_dir_extattr_cb_cb(extattrhdr_t *ahdrp, void *ctxp)
> static void
> setextattr(char *path, extattrhdr_t *ahdrp)
> {
> - static char dmiattr[] = "SGI_DMI_";
> bool_t isrootpr = ahdrp->ah_flags & EXTATTRHDR_FLAGS_ROOT;
> bool_t issecurepr = ahdrp->ah_flags & EXTATTRHDR_FLAGS_SECURE;
> - bool_t isdmpr;
> int attr_namespace;
> int rval;
>
> - isdmpr = (isrootpr &&
> - !strncmp((char *)(&ahdrp[1]), dmiattr, sizeof(dmiattr)-1));
>
> /* If restoreextattrpr not set, then we are here because -D was
> * specified. So return unless it looks like a root DMAPI attribute.
> */
> - if (!persp->a.restoreextattrpr && !isdmpr)
> + if (!persp->a.restoreextattrpr)
> return;
Er... yes? Looks right, but xfsdump is enough of a mess... :/
>
> > -
> > - if (persp->a.dstdirisxfspr && persp->a.restoredmpr) {
> > - int flag = 0;
> > - char *attrname = (char *)&ahdrp[1];
> > - if (ahdrp->ah_flags & EXTATTRHDR_FLAGS_ROOT)
> > - flag = ATTR_ROOT;
> > - else if (ahdrp->ah_flags & EXTATTRHDR_FLAGS_SECURE)
> > - flag = ATTR_SECURE;
> > -
> > - HsmRestoreAttribute(flag,
> > - attrname,
> > - &strctxp->sc_hsmflags);
>
> And with the only user of strctxp gone it's now an unused local var, I think.
I don't do words that lack ^^^^^^^ vowels.
> Anyway....
>
> I wonder if there's still more that could be ripped out:
>
> uint32_t bs_dmevmask; /* DMI event mask 4 6c */
> uint16_t bs_dmstate; /* DMI state info 2 6e */
>
> Those can't go, I guess, because they are part of the header in the on-disk format.
>
> But why are we still fiddling with them? For that matter, why does hsmapi.c still
> exist at all?
It probably can go too.
> I have the sense that if we really want to remove all dmapi support there's further
> to go, but as with all things xfsdump, it scares me a bit ...
<nod>
--D
> -Eric
>
