All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs_restore: remove DMAPI support
@ 2022-02-03 17:45 Darrick J. Wong
  2022-02-10 22:46 ` Eric Sandeen
  2022-08-04 11:40 ` Carlos Maiolino
  0 siblings, 2 replies; 4+ messages in thread
From: Darrick J. Wong @ 2022-02-03 17:45 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: xfs

From: Darrick J. Wong <djwong@kernel.org>

The last of the DMAPI stubs were removed from Linux 5.17, so drop this
functionality altogether.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 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/doc/xfsdump.html b/doc/xfsdump.html
index d4d157f..2c9324b 100644
--- a/doc/xfsdump.html
+++ b/doc/xfsdump.html
@@ -1092,7 +1092,6 @@ the size of the hash table.
         bool_t p_ownerpr - whether to restore directory owner/group attributes
         bool_t p_fullpr - whether restoring a full level 0 non-resumed dump
         bool_t p_ignoreorphpr - set if positive subtree or interactive
-        bool_t p_restoredmpr - restore DMI event settings
 </pre>
 <p>
 The hash table maps the inode number to the tree node. It is a
diff --git a/po/de.po b/po/de.po
index 62face8..bdf47d1 100644
--- a/po/de.po
+++ b/po/de.po
@@ -3972,11 +3972,6 @@ msgstr ""
 msgid "no additional media objects needed\n"
 msgstr "keine zusätzlichen Mediendateien benötigt\n"
 
-#: .././restore/content.c:9547
-#, c-format
-msgid "fssetdm_by_handle of %s failed %s\n"
-msgstr "fssetdm_by_handle von %s fehlgeschlagen %s\n"
-
 #: .././restore/content.c:9566
 #, c-format
 msgid "%s quota information written to '%s'\n"
diff --git a/po/pl.po b/po/pl.po
index 3cba8d6..ba25420 100644
--- a/po/pl.po
+++ b/po/pl.po
@@ -3455,11 +3455,6 @@ msgstr "nie są potrzebne dodatkowe obiekty nośnika\n"
 msgid "path_to_handle of %s failed:%s\n"
 msgstr "path_to_handle na %s nie powiodło się: %s\n"
 
-#: .././restore/content.c:9723
-#, c-format
-msgid "fssetdm_by_handle of %s failed %s\n"
-msgstr "fssetdm_by_handle na %s nie powiodło się: %s\n"
-
 #: .././restore/content.c:9742
 #, c-format
 msgid "%s quota information written to '%s'\n"
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);
 static int quotafilecheck(char *type, char *dstdir, char *quotafile);
 
 /* definition of locally defined global variables ****************************/
@@ -894,7 +890,6 @@ content_init(int argc, char *argv[], size64_t vmsz)
 	bool_t changepr;/* cmd line overwrite inhibit specification */
 	bool_t interpr;	/* cmd line interactive mode requested */
 	bool_t ownerpr;	/* cmd line chown/chmod requested */
-	bool_t restoredmpr; /* cmd line restore dm api attrs specification */
 	bool_t restoreextattrpr; /* cmd line restore extended attr spec */
 	bool_t sesscpltpr; /* force completion of prev interrupted session */
 	ix_t stcnt;	/* cmd line number of subtrees requested */
@@ -956,7 +951,6 @@ content_init(int argc, char *argv[], size64_t vmsz)
 	newerpr = BOOL_FALSE;
 	changepr = BOOL_FALSE;
 	ownerpr = BOOL_FALSE;
-	restoredmpr = BOOL_FALSE;
 	restoreextattrpr = BOOL_TRUE;
 	sesscpltpr = BOOL_FALSE;
 	stcnt = 0;
@@ -1162,8 +1156,11 @@ content_init(int argc, char *argv[], size64_t vmsz)
 			tranp->t_noinvupdatepr = BOOL_TRUE;
 			break;
 		case GETOPT_SETDM:
-			restoredmpr = BOOL_TRUE;
-			break;
+			mlog(MLOG_NORMAL | MLOG_ERROR, _(
+			      "-%c option no longer supported\n"),
+			      GETOPT_SETDM);
+			usage();
+			return BOOL_FALSE;
 		case GETOPT_ALERTPROG:
 			if (!optarg || optarg[0] == '-') {
 				mlog(MLOG_NORMAL | MLOG_ERROR, _(
@@ -1574,12 +1571,6 @@ content_init(int argc, char *argv[], size64_t vmsz)
 	}
 
 	if (persp->a.valpr) {
-		if (restoredmpr && persp->a.restoredmpr != restoredmpr) {
-			mlog(MLOG_NORMAL | MLOG_ERROR, _(
-			     "-%c cannot reset flag from previous restore\n"),
-			      GETOPT_SETDM);
-			return BOOL_FALSE;
-		}
 		if (!restoreextattrpr &&
 		       persp->a.restoreextattrpr != restoreextattrpr) {
 			mlog(MLOG_NORMAL | MLOG_ERROR, _(
@@ -1734,7 +1725,6 @@ content_init(int argc, char *argv[], size64_t vmsz)
 			persp->a.newerpr = newerpr;
 			persp->a.newertime = newertime;
 		}
-		persp->a.restoredmpr = restoredmpr;
 		if (!persp->a.dstdirisxfspr) {
 			restoreextattrpr = BOOL_FALSE;
 		}
@@ -2365,7 +2355,6 @@ content_stream_restore(ix_t thrdix)
 					scrhdrp->cih_inomap_nondircnt,
 					tranp->t_vmsz,
 					fullpr,
-					persp->a.restoredmpr,
 					persp->a.dstdirisxfspr,
 					grhdrp->gh_version,
 					tranp->t_truncategenpr);
@@ -7549,12 +7538,6 @@ restore_reg(drive_t *drivep,
 		}
 	}
 
-	if (persp->a.dstdirisxfspr && persp->a.restoredmpr) {
-		HsmBeginRestoreFile(bstatp,
-				     *fdp,
-				     &strctxp->sc_hsmflags);
-	}
-
 	return BOOL_TRUE;
 }
 
@@ -7726,26 +7709,6 @@ restore_complete_reg(stream_context_t *strcxtp)
 		      strerror(errno));
 	}
 
-	if (persp->a.dstdirisxfspr && persp->a.restoredmpr) {
-		fsdmidata_t fssetdm;
-
-		/* Set the DMAPI Fields. */
-		fssetdm.fsd_dmevmask = bstatp->bs_dmevmask;
-		fssetdm.fsd_padding = 0;
-		fssetdm.fsd_dmstate = bstatp->bs_dmstate;
-
-		rval = ioctl(fd, XFS_IOC_FSSETDM, (void *)&fssetdm);
-		if (rval) {
-			mlog(MLOG_NORMAL | MLOG_WARNING,
-			      _("attempt to set DMI attributes of %s "
-			      "failed: %s\n"),
-			      path,
-			      strerror(errno));
-		}
-
-		HsmEndRestoreFile(path, fd, &strcxtp->sc_hsmflags);
-	}
-
 	/* set any extended inode flags that couldn't be set
 	 * prior to restoring the data.
 	 */
@@ -8064,17 +8027,6 @@ restore_symlink(drive_t *drivep,
 				      strerror(errno));
 			}
 		}
-
-		if (persp->a.restoredmpr) {
-		fsdmidata_t fssetdm;
-
-		/*	Restore DMAPI fields. */
-
-		fssetdm.fsd_dmevmask = bstatp->bs_dmevmask;
-		fssetdm.fsd_padding = 0;
-		fssetdm.fsd_dmstate = bstatp->bs_dmstate;
-		rval = do_fssetdm_by_handle(path, &fssetdm);
-		}
 	}
 
 	return BOOL_TRUE;
@@ -8777,7 +8729,7 @@ restore_extattr(drive_t *drivep,
 		}
 		assert(nread == (int)(recsz - EXTATTRHDR_SZ));
 
-		if (!persp->a.restoreextattrpr && !persp->a.restoredmpr) {
+		if (!persp->a.restoreextattrpr) {
 			continue;
 		}
 
@@ -8796,19 +8748,6 @@ restore_extattr(drive_t *drivep,
 			}
 		} else if (isfilerestored && path[0] != '\0') {
 			setextattr(path, ahdrp);
-
-			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);
-			}
 		}
 	}
 	/* NOTREACHED */
@@ -9709,32 +9648,6 @@ display_needed_objects(purp_t purp,
 	}
 }
 
-static int
-do_fssetdm_by_handle(
-	char		*path,
-	fsdmidata_t	*fdmp)
-{
-	void		*hanp;
-	size_t		hlen=0;
-	int		rc;
-
-	if (path_to_handle(path, &hanp, &hlen)) {
-		mlog(MLOG_NORMAL | MLOG_WARNING, _(
-			"path_to_handle of %s failed:%s\n"),
-			path, strerror(errno));
-		return -1;
-	}
-
-	rc = fssetdm_by_handle(hanp, hlen, fdmp);
-	free_handle(hanp, hlen);
-	if (rc) {
-		mlog(MLOG_NORMAL | MLOG_WARNING, _(
-			"fssetdm_by_handle of %s failed %s\n"),
-			path, strerror(errno));
-	}
-	return rc;
-}
-
 static int
 quotafilecheck(char *type, char *dstdir, char *quotafile)
 {
diff --git a/restore/tree.c b/restore/tree.c
index 0670318..5429b74 100644
--- a/restore/tree.c
+++ b/restore/tree.c
@@ -108,9 +108,6 @@ struct treePersStorage {
 	bool_t p_ignoreorphpr;
 		/* set if positive subtree or interactive
 		 */
-	bool_t p_restoredmpr;
-		/* restore DMI event settings
-		 */
 	bool_t p_truncategenpr;
 		/* truncate inode generation number (for compatibility
 		 * with xfsdump format 2 and earlier)
@@ -348,7 +345,6 @@ tree_init(char *hkdir,
 	   size64_t nondircnt,
 	   size64_t vmsz,
 	   bool_t fullpr,
-	   bool_t restoredmpr,
 	   bool_t dstdirisxfspr,
 	   uint32_t dumpformat,
 	   bool_t truncategenpr)
@@ -508,10 +504,6 @@ tree_init(char *hkdir,
 	 */
 	persp->p_fullpr = fullpr;
 
-	/* record if DMI event settings should be restored
-	 */
-	persp->p_restoredmpr = restoredmpr;
-
 	/* record if truncated generation numbers are required
 	 */
 	if (dumpformat < GLOBAL_HDR_VERSION_3) {
@@ -2550,31 +2542,6 @@ setdirattr(dah_t dah, char *path)
 		}
 	}
 
-	if (tranp->t_dstdirisxfspr && persp->p_restoredmpr) {
-		fsdmidata_t fssetdm;
-
-		fssetdm.fsd_dmevmask = dirattr_get_dmevmask(dah);
-		fssetdm.fsd_padding = 0;	/* not used */
-		fssetdm.fsd_dmstate = (uint16_t)dirattr_get_dmstate(dah);
-
-		/* restore DMAPI event settings etc.
-		 */
-		rval = ioctl(fd,
-			      XFS_IOC_FSSETDM,
-			      (void *)&fssetdm);
-		if (rval) {
-			mlog(errno == EINVAL
-			      ?
-			      (MLOG_NITTY + 1) | MLOG_TREE
-			      :
-			      MLOG_NITTY | MLOG_TREE,
-			      "set DMI attributes"
-			      " of %s failed: %s\n",
-			      path,
-			      strerror(errno));
-		}
-	}
-
 	utimbuf.actime = dirattr_get_atime(dah);
 	utimbuf.modtime = dirattr_get_mtime(dah);
 	rval = utime(path, &utimbuf);
diff --git a/restore/tree.h b/restore/tree.h
index 4f9ffe8..bf66e3d 100644
--- a/restore/tree.h
+++ b/restore/tree.h
@@ -31,7 +31,6 @@ extern bool_t tree_init(char *hkdir,
 			 size64_t nondircnt,
 			 size64_t vmsz,
 			 bool_t fullpr,
-			 bool_t restoredmpr,
 			 bool_t dstdirisxfspr,
 			 uint32_t dumpformat,
 			 bool_t truncategenpr);

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] xfs_restore: remove DMAPI support
  2022-02-03 17:45 [PATCH] xfs_restore: remove DMAPI support Darrick J. Wong
@ 2022-02-10 22:46 ` Eric Sandeen
  2022-06-28 22:58   ` Darrick J. Wong
  2022-08-04 11:40 ` Carlos Maiolino
  1 sibling, 1 reply; 4+ messages in thread
From: Eric Sandeen @ 2022-02-10 22:46 UTC (permalink / raw)
  To: Darrick J. Wong, Eric Sandeen; +Cc: xfs, Anthony Iliopoulos

On 2/3/22 11:45 AM, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> 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?  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.


> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  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?

...

> @@ -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;

> -
> -			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.

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?

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 ...

-Eric


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] xfs_restore: remove DMAPI support
  2022-02-10 22:46 ` Eric Sandeen
@ 2022-06-28 22:58   ` Darrick J. Wong
  0 siblings, 0 replies; 4+ messages in thread
From: Darrick J. Wong @ 2022-06-28 22:58 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Eric Sandeen, xfs, Anthony Iliopoulos

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@kernel.org>
> > 
> > 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@kernel.org>
> > ---
> >  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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] xfs_restore: remove DMAPI support
  2022-02-03 17:45 [PATCH] xfs_restore: remove DMAPI support Darrick J. Wong
  2022-02-10 22:46 ` Eric Sandeen
@ 2022-08-04 11:40 ` Carlos Maiolino
  1 sibling, 0 replies; 4+ messages in thread
From: Carlos Maiolino @ 2022-08-04 11:40 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Eric Sandeen, xfs

On Thu, Feb 03, 2022 at 09:45:40AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> The last of the DMAPI stubs were removed from Linux 5.17, so drop this
> functionality altogether.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>

Patch looks good, and fixes the broken build:

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Tested-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  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/doc/xfsdump.html b/doc/xfsdump.html
> index d4d157f..2c9324b 100644
> --- a/doc/xfsdump.html
> +++ b/doc/xfsdump.html
> @@ -1092,7 +1092,6 @@ the size of the hash table.
>          bool_t p_ownerpr - whether to restore directory owner/group attributes
>          bool_t p_fullpr - whether restoring a full level 0 non-resumed dump
>          bool_t p_ignoreorphpr - set if positive subtree or interactive
> -        bool_t p_restoredmpr - restore DMI event settings
>  </pre>
>  <p>
>  The hash table maps the inode number to the tree node. It is a
> diff --git a/po/de.po b/po/de.po
> index 62face8..bdf47d1 100644
> --- a/po/de.po
> +++ b/po/de.po
> @@ -3972,11 +3972,6 @@ msgstr ""
>  msgid "no additional media objects needed\n"
>  msgstr "keine zusätzlichen Mediendateien benötigt\n"
>  
> -#: .././restore/content.c:9547
> -#, c-format
> -msgid "fssetdm_by_handle of %s failed %s\n"
> -msgstr "fssetdm_by_handle von %s fehlgeschlagen %s\n"
> -
>  #: .././restore/content.c:9566
>  #, c-format
>  msgid "%s quota information written to '%s'\n"
> diff --git a/po/pl.po b/po/pl.po
> index 3cba8d6..ba25420 100644
> --- a/po/pl.po
> +++ b/po/pl.po
> @@ -3455,11 +3455,6 @@ msgstr "nie są potrzebne dodatkowe obiekty nośnika\n"
>  msgid "path_to_handle of %s failed:%s\n"
>  msgstr "path_to_handle na %s nie powiodło się: %s\n"
>  
> -#: .././restore/content.c:9723
> -#, c-format
> -msgid "fssetdm_by_handle of %s failed %s\n"
> -msgstr "fssetdm_by_handle na %s nie powiodło się: %s\n"
> -
>  #: .././restore/content.c:9742
>  #, c-format
>  msgid "%s quota information written to '%s'\n"
> 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);
>  static int quotafilecheck(char *type, char *dstdir, char *quotafile);
>  
>  /* definition of locally defined global variables ****************************/
> @@ -894,7 +890,6 @@ content_init(int argc, char *argv[], size64_t vmsz)
>  	bool_t changepr;/* cmd line overwrite inhibit specification */
>  	bool_t interpr;	/* cmd line interactive mode requested */
>  	bool_t ownerpr;	/* cmd line chown/chmod requested */
> -	bool_t restoredmpr; /* cmd line restore dm api attrs specification */
>  	bool_t restoreextattrpr; /* cmd line restore extended attr spec */
>  	bool_t sesscpltpr; /* force completion of prev interrupted session */
>  	ix_t stcnt;	/* cmd line number of subtrees requested */
> @@ -956,7 +951,6 @@ content_init(int argc, char *argv[], size64_t vmsz)
>  	newerpr = BOOL_FALSE;
>  	changepr = BOOL_FALSE;
>  	ownerpr = BOOL_FALSE;
> -	restoredmpr = BOOL_FALSE;
>  	restoreextattrpr = BOOL_TRUE;
>  	sesscpltpr = BOOL_FALSE;
>  	stcnt = 0;
> @@ -1162,8 +1156,11 @@ content_init(int argc, char *argv[], size64_t vmsz)
>  			tranp->t_noinvupdatepr = BOOL_TRUE;
>  			break;
>  		case GETOPT_SETDM:
> -			restoredmpr = BOOL_TRUE;
> -			break;
> +			mlog(MLOG_NORMAL | MLOG_ERROR, _(
> +			      "-%c option no longer supported\n"),
> +			      GETOPT_SETDM);
> +			usage();
> +			return BOOL_FALSE;
>  		case GETOPT_ALERTPROG:
>  			if (!optarg || optarg[0] == '-') {
>  				mlog(MLOG_NORMAL | MLOG_ERROR, _(
> @@ -1574,12 +1571,6 @@ content_init(int argc, char *argv[], size64_t vmsz)
>  	}
>  
>  	if (persp->a.valpr) {
> -		if (restoredmpr && persp->a.restoredmpr != restoredmpr) {
> -			mlog(MLOG_NORMAL | MLOG_ERROR, _(
> -			     "-%c cannot reset flag from previous restore\n"),
> -			      GETOPT_SETDM);
> -			return BOOL_FALSE;
> -		}
>  		if (!restoreextattrpr &&
>  		       persp->a.restoreextattrpr != restoreextattrpr) {
>  			mlog(MLOG_NORMAL | MLOG_ERROR, _(
> @@ -1734,7 +1725,6 @@ content_init(int argc, char *argv[], size64_t vmsz)
>  			persp->a.newerpr = newerpr;
>  			persp->a.newertime = newertime;
>  		}
> -		persp->a.restoredmpr = restoredmpr;
>  		if (!persp->a.dstdirisxfspr) {
>  			restoreextattrpr = BOOL_FALSE;
>  		}
> @@ -2365,7 +2355,6 @@ content_stream_restore(ix_t thrdix)
>  					scrhdrp->cih_inomap_nondircnt,
>  					tranp->t_vmsz,
>  					fullpr,
> -					persp->a.restoredmpr,
>  					persp->a.dstdirisxfspr,
>  					grhdrp->gh_version,
>  					tranp->t_truncategenpr);
> @@ -7549,12 +7538,6 @@ restore_reg(drive_t *drivep,
>  		}
>  	}
>  
> -	if (persp->a.dstdirisxfspr && persp->a.restoredmpr) {
> -		HsmBeginRestoreFile(bstatp,
> -				     *fdp,
> -				     &strctxp->sc_hsmflags);
> -	}
> -
>  	return BOOL_TRUE;
>  }
>  
> @@ -7726,26 +7709,6 @@ restore_complete_reg(stream_context_t *strcxtp)
>  		      strerror(errno));
>  	}
>  
> -	if (persp->a.dstdirisxfspr && persp->a.restoredmpr) {
> -		fsdmidata_t fssetdm;
> -
> -		/* Set the DMAPI Fields. */
> -		fssetdm.fsd_dmevmask = bstatp->bs_dmevmask;
> -		fssetdm.fsd_padding = 0;
> -		fssetdm.fsd_dmstate = bstatp->bs_dmstate;
> -
> -		rval = ioctl(fd, XFS_IOC_FSSETDM, (void *)&fssetdm);
> -		if (rval) {
> -			mlog(MLOG_NORMAL | MLOG_WARNING,
> -			      _("attempt to set DMI attributes of %s "
> -			      "failed: %s\n"),
> -			      path,
> -			      strerror(errno));
> -		}
> -
> -		HsmEndRestoreFile(path, fd, &strcxtp->sc_hsmflags);
> -	}
> -
>  	/* set any extended inode flags that couldn't be set
>  	 * prior to restoring the data.
>  	 */
> @@ -8064,17 +8027,6 @@ restore_symlink(drive_t *drivep,
>  				      strerror(errno));
>  			}
>  		}
> -
> -		if (persp->a.restoredmpr) {
> -		fsdmidata_t fssetdm;
> -
> -		/*	Restore DMAPI fields. */
> -
> -		fssetdm.fsd_dmevmask = bstatp->bs_dmevmask;
> -		fssetdm.fsd_padding = 0;
> -		fssetdm.fsd_dmstate = bstatp->bs_dmstate;
> -		rval = do_fssetdm_by_handle(path, &fssetdm);
> -		}
>  	}
>  
>  	return BOOL_TRUE;
> @@ -8777,7 +8729,7 @@ restore_extattr(drive_t *drivep,
>  		}
>  		assert(nread == (int)(recsz - EXTATTRHDR_SZ));
>  
> -		if (!persp->a.restoreextattrpr && !persp->a.restoredmpr) {
> +		if (!persp->a.restoreextattrpr) {
>  			continue;
>  		}
>  
> @@ -8796,19 +8748,6 @@ restore_extattr(drive_t *drivep,
>  			}
>  		} else if (isfilerestored && path[0] != '\0') {
>  			setextattr(path, ahdrp);
> -
> -			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);
> -			}
>  		}
>  	}
>  	/* NOTREACHED */
> @@ -9709,32 +9648,6 @@ display_needed_objects(purp_t purp,
>  	}
>  }
>  
> -static int
> -do_fssetdm_by_handle(
> -	char		*path,
> -	fsdmidata_t	*fdmp)
> -{
> -	void		*hanp;
> -	size_t		hlen=0;
> -	int		rc;
> -
> -	if (path_to_handle(path, &hanp, &hlen)) {
> -		mlog(MLOG_NORMAL | MLOG_WARNING, _(
> -			"path_to_handle of %s failed:%s\n"),
> -			path, strerror(errno));
> -		return -1;
> -	}
> -
> -	rc = fssetdm_by_handle(hanp, hlen, fdmp);
> -	free_handle(hanp, hlen);
> -	if (rc) {
> -		mlog(MLOG_NORMAL | MLOG_WARNING, _(
> -			"fssetdm_by_handle of %s failed %s\n"),
> -			path, strerror(errno));
> -	}
> -	return rc;
> -}
> -
>  static int
>  quotafilecheck(char *type, char *dstdir, char *quotafile)
>  {
> diff --git a/restore/tree.c b/restore/tree.c
> index 0670318..5429b74 100644
> --- a/restore/tree.c
> +++ b/restore/tree.c
> @@ -108,9 +108,6 @@ struct treePersStorage {
>  	bool_t p_ignoreorphpr;
>  		/* set if positive subtree or interactive
>  		 */
> -	bool_t p_restoredmpr;
> -		/* restore DMI event settings
> -		 */
>  	bool_t p_truncategenpr;
>  		/* truncate inode generation number (for compatibility
>  		 * with xfsdump format 2 and earlier)
> @@ -348,7 +345,6 @@ tree_init(char *hkdir,
>  	   size64_t nondircnt,
>  	   size64_t vmsz,
>  	   bool_t fullpr,
> -	   bool_t restoredmpr,
>  	   bool_t dstdirisxfspr,
>  	   uint32_t dumpformat,
>  	   bool_t truncategenpr)
> @@ -508,10 +504,6 @@ tree_init(char *hkdir,
>  	 */
>  	persp->p_fullpr = fullpr;
>  
> -	/* record if DMI event settings should be restored
> -	 */
> -	persp->p_restoredmpr = restoredmpr;
> -
>  	/* record if truncated generation numbers are required
>  	 */
>  	if (dumpformat < GLOBAL_HDR_VERSION_3) {
> @@ -2550,31 +2542,6 @@ setdirattr(dah_t dah, char *path)
>  		}
>  	}
>  
> -	if (tranp->t_dstdirisxfspr && persp->p_restoredmpr) {
> -		fsdmidata_t fssetdm;
> -
> -		fssetdm.fsd_dmevmask = dirattr_get_dmevmask(dah);
> -		fssetdm.fsd_padding = 0;	/* not used */
> -		fssetdm.fsd_dmstate = (uint16_t)dirattr_get_dmstate(dah);
> -
> -		/* restore DMAPI event settings etc.
> -		 */
> -		rval = ioctl(fd,
> -			      XFS_IOC_FSSETDM,
> -			      (void *)&fssetdm);
> -		if (rval) {
> -			mlog(errno == EINVAL
> -			      ?
> -			      (MLOG_NITTY + 1) | MLOG_TREE
> -			      :
> -			      MLOG_NITTY | MLOG_TREE,
> -			      "set DMI attributes"
> -			      " of %s failed: %s\n",
> -			      path,
> -			      strerror(errno));
> -		}
> -	}
> -
>  	utimbuf.actime = dirattr_get_atime(dah);
>  	utimbuf.modtime = dirattr_get_mtime(dah);
>  	rval = utime(path, &utimbuf);
> diff --git a/restore/tree.h b/restore/tree.h
> index 4f9ffe8..bf66e3d 100644
> --- a/restore/tree.h
> +++ b/restore/tree.h
> @@ -31,7 +31,6 @@ extern bool_t tree_init(char *hkdir,
>  			 size64_t nondircnt,
>  			 size64_t vmsz,
>  			 bool_t fullpr,
> -			 bool_t restoredmpr,
>  			 bool_t dstdirisxfspr,
>  			 uint32_t dumpformat,
>  			 bool_t truncategenpr);
> 

-- 
Carlos


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-08-04 11:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-03 17:45 [PATCH] xfs_restore: remove DMAPI support Darrick J. Wong
2022-02-10 22:46 ` Eric Sandeen
2022-06-28 22:58   ` Darrick J. Wong
2022-08-04 11:40 ` Carlos Maiolino

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.