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

[Bill Kendall <wkendall@sgi.com>]

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@sgi.com>
>
>> Signed-off-by: Bill Kendall<wkendall@sgi.com>
>>
>> ---
> . . .
>
>> 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.

Bill

>
>> -	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 */
>>   	}
>>
>
> . . .

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs