On Tue, Sep 06, 2016 at 08:21:55AM -0700, Christoph Hellwig wrote:
> > + __uint64_t cui_id;
> > + struct xfs_ail_cursor cur;
> > + struct xfs_ail *ailp = log->l_ailp;
> > +
> > + cud_formatp = item->ri_buf[0].i_addr;
> > + ASSERT(item->ri_buf[0].i_len == sizeof(struct xfs_cud_log_format));
>
> Should we return -EFSCORRUPTED here instead?
Yes. The RUD recovery routine should probably get that change too.
> > + /* XXX: do nothing for now */
>
> What else would be do in the future here?
>
> > +static void
> > +xfs_trans_set_refcount_flags(
> > + struct xfs_phys_extent *refc,
> > + enum xfs_refcount_intent_type type)
> > +{
> > + refc->pe_flags = 0;
> > + switch (type) {
> > + case XFS_REFCOUNT_INCREASE:
> > + refc->pe_flags |= XFS_REFCOUNT_EXTENT_INCREASE;
> > + break;
> > + case XFS_REFCOUNT_DECREASE:
> > + refc->pe_flags |= XFS_REFCOUNT_EXTENT_DECREASE;
> > + break;
> > + case XFS_REFCOUNT_ALLOC_COW:
> > + refc->pe_flags |= XFS_REFCOUNT_EXTENT_ALLOC_COW;
> > + break;
> > + case XFS_REFCOUNT_FREE_COW:
> > + refc->pe_flags |= XFS_REFCOUNT_EXTENT_FREE_COW;
> > + break;
>
> Is there any good reasons to use a type enum in core, but flags on
> disk?
I suppose since log structures aren't guaranteed to be platform agnostic
it's fine to just copy in the in-core enum here.
> > +int
> > +xfs_trans_log_finish_refcount_update(
> > + struct xfs_trans *tp,
> > + struct xfs_cud_log_item *cudp,
> > + enum xfs_refcount_intent_type type,
> > + xfs_fsblock_t startblock,
> > + xfs_extlen_t blockcount,
> > + struct xfs_btree_cur **pcur)
> > +{
> > + int error;
> > +
> > + /* XXX: leave this empty for now */
> > + error = -EFSCORRUPTED;
>
> Lift might be a lot easier if this patch and "xfs: connect refcount
> adjust functions to upper layers" were merged into one. It's not like
> they are testable independently anyway.
I'll think about it. I think it might not be too difficult to push "xfs: log
refcount intent items" down one and merge it with "xfs: connect refcount adjust
functions to upper layers", though my preference biases towards not stirring
things up just to reduce commit count.
(Basically, I'll give it a try after I'm done making the other fixes and commit
it if it doesn't make a total mess of things.)
--D
|