On Thu, Mar 07, 2013 at 12:50:42PM -0700, Zach Brown wrote:
> 
> > +static inline int btrfs_fs_incompat(struct btrfs_fs_info *fs_info, u64 
> > flag)
> > +{
> > +   struct btrfs_super_block *disk_super;
> > +   disk_super = fs_info->super_copy;
> > +   return (btrfs_super_incompat_flags(disk_super) & flag);
> > +}
> 
> That'll fail if there are ever flag bits that don't fit in an int.  The
> sneakest fix is wrapping the conditional in the obscure !!().  Making it
> a bool and really returning true and false would probably be easier on
> the brain.
> 

We can't do bool but i'll do the !!() thing.

> 
> >     ret = btrfs_search_slot(trans, root, &key, path, extra_size, 1);
> >     if (ret < 0) {
> >             err = ret;
> >             goto out;
> >     }
> > +
> > +   /*
> > +    * We may be a newly converted file system which still has the old fat
> > +    * extent entries for metadata, so try and see if we have one of those.
> > +    */
> > +   if (ret > 0 && skinny_metadata) {
> > +           skinny_metadata = false;
> > +           if (path->slots[0]) {
> > +                   path->slots[0]--;
> > +                   btrfs_item_key_to_cpu(path->nodes[0], &key,
> > +                                         path->slots[0]);
> > +                   if (key.objectid == bytenr &&
> > +                       key.type == BTRFS_EXTENT_ITEM_KEY &&
> > +                       key.offset == num_bytes)
> > +                           ret = 0;
> > +           }
> > +           if (ret) {
> > +                   key.type = BTRFS_EXTENT_ITEM_KEY;
> > +                   key.offset = num_bytes;
> > +                   btrfs_release_path(path);
> > +                   goto again;
> > +           }
> > +   }
> > +
> 
> >                     ret = btrfs_search_slot(trans, extent_root,
> >                                             &key, path, -1, 1);
> > +                   if (ret > 0 && skinny_metadata && path->slots[0]) {
> > +                           /*
> > +                            * Couldn't find our skinny metadata item,
> > +                            * see if we have ye olde extent item.
> > +                            */
> > +                           path->slots[0]--;
> > +                           btrfs_item_key_to_cpu(path->nodes[0], &key,
> > +                                                 path->slots[0]);
> > +                           if (key.objectid == bytenr &&
> > +                               key.type == BTRFS_EXTENT_ITEM_KEY &&
> > +                               key.offset == num_bytes)
> > +                                   ret = 0;
> > +                   }
> > +
> > +                   if (ret > 0 && skinny_metadata) {
> > +                           skinny_metadata = false;
> > +                           key.type = BTRFS_EXTENT_ITEM_KEY;
> > +                           key.offset = num_bytes;
> > +                           btrfs_release_path(path);
> > +                           ret = btrfs_search_slot(trans, extent_root,
> > +                                                   &key, path, -1, 1);
> > +                   }
> > +
> 
> These blobs of similar code sure look like they should be in a function.
> 

The searches do different things in different places so we have to have samey
blobs like this until we get everybody on skinny metadata and can delete the old
code.  Thanks,

Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to