[Top] [All Lists]

Re: [PATCH] xfsdump: prune inventory sessions by session id

To: aelder@xxxxxxx
Subject: Re: [PATCH] xfsdump: prune inventory sessions by session id
From: Bill Kendall <wkendall@xxxxxxx>
Date: Fri, 25 Feb 2011 15:03:32 -0600
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1298662049.1990.7053.camel@doink>
References: <1298564720-11447-1-git-send-email-wkendall@xxxxxxx> <1298662049.1990.7053.camel@doink>
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20101208 Thunderbird/3.1.7
On 02/25/2011 01:27 PM, Alex Elder wrote:
On Thu, 2011-02-24 at 10:25 -0600, Bill Kendall wrote:
Allow xfsinvutil to prune inventory sessions by their ID
instead of only by filesystem and cutoff date.

This would have been a bit nicer with a little more
explanation.  I.e.:
- You specify the session id using the new "-s<sessid>"
   command line option.
- The "-s" option is mutually exclusive with "-u<UUID>"
   and "-M<mount_point>  <mm/dd/yy>".  It also cannot be
   used together with the "-i" (interactive) or "-C"
   (consistency check) options.
- The change is implemented by adding a session id argument
   to CheckAndPruneFstab(), CheckAndPruneInvIndexFile(), and
   CheckAndPruneStObjFile().  That session becomes a third
   way of identifying entries to be pruned (in addition
   to mount point and UUID).

Feel free to amend the commit message when you check this
in. I agree that at least it should have mentioned that
this adds a new xfsinvutil -s option.

Anyway, this looks good to me.  I have one question
below, but regardless of your answer...

Reviewed-by: Alex Elder<aelder@xxxxxxx>

Signed-off-by: Bill Kendall<wkendall@xxxxxxx>

. . .

diff --git a/invutil/invutil.c b/invutil/invutil.c
index af6836b..37489c0 100644
--- a/invutil/invutil.c
+++ b/invutil/invutil.c

. . .

@@ -594,14 +648,8 @@ CheckAndPruneInvIndexFile( bool_t checkonly,
            removeflag = BOOL_TRUE;

-       if (( !removeflag )&&  (checkonly == BOOL_FALSE)&&
-               ( invIndexEntry[i].ie_timeperiod.tp_start<  prunetime))
-       {
-           IdxCheckOnly = BOOL_FALSE;
-           printf("          Mount point match\n");
-       }

Why do you no longer print this in this case?

The message did not seem to serve a purpose. To reach the print
we already had to match on the mount point, and a message is
already issued for that.

As you probably noticed, the 'if' statement did not serve a
functional purpose since the prune time has to be checked for
each session anyway, and further the 'if' statement would have
to be reworked to consider whether this is a prune-by-session run.

As a side note, it's too bad that the consistency checking and
pruning is coupled together. It would be much cleaner to
implement the various pruning filters as separate functions.


-       if (CheckAndPruneStObjFile(IdxCheckOnly, invIndexEntry[i].ie_filename,
-                   prunetime, r_mf_label) == -1) {
+       if (CheckAndPruneStObjFile(checkonly, invIndexEntry[i].ie_filename,
+                   sessionp, prunetime, r_mf_label) == -1) {
            removeflag = BOOL_TRUE; /* The StObj is gone */

. . .

<Prev in Thread] Current Thread [Next in Thread>