[LU-1154] CLIO does not send parent FID with OBD_MD_FLFID to OST on setattr

• Type: Bug
• Resolution: Fixed
• Priority: Critical
• Fix Version/s: Lustre 2.7.0
• Affects Version/s: Lustre 2.1.0, Lustre 2.2.0
• Labels: HB
• Story Points: 3
• Severity: 3
• Assignee: Zhenyu Xu
• Reporter: Andreas Dilger

Description

A filter_fid EA should be added to an OST object whenever the object is being modified - either its data or metadata. However, current 2.x clients send filter_fid/OBD_MD_FLFID information only with OST_WRITE RPCs. In contrast, 1.x clients pack the OBD_MD_FLFID information into setattr(mtime) and truncate RPCs. Ideally, the client will pass this information with every object RPC (read or write) so that the OST can verify that the client is accessing the correct OST object for its file IO (most importantly as a sanity check, but also a simple form of capability check). This will also be important for LFSCK Phase II, when the OST needs to verify that the object parent FID matches the MDT inode FID.

While there is some code in master client that appears to be sending this information to the OST, the twisty mess that is CLIO appears to lose this information between where the inode is available in ll_setattr_raw():

ll_setattr_raw()
  ->ll_setattr_ost()
    ->cl_setattr_ost()  # attributes are packed into "ost_lvb" here
      ->cl_io_loop()
        ->cl_io_start()
          ->lov_io_start()
            ->osc_io_setattr_start()
              ->cl_object_attr_set()
                ->osc_attr_set()  # attributes unpacked from ost_lvb
              ->osc_setattr_async_base() # RPC is sent here

but when the RPC is formed in the parent FID is not in the obdo. One might mistakenly assume that since ccc_req_attr_set() calls obdo_from_inode() (which always copies the OBD_MD_FLFID information when the inode FID is passed) that this data would make it into the setattr RPC, but this doesn't appear in the debug logs, nor is the "fid" xattr set on a file with just "touch".

There are many twisty functions, all of them look alike, which makes understanding and debugging this code more difficult than it needs to be:

cl_setattr_ost()
cl_object_attr_set()
cl_req_attr_set()
osc_req_attr_set()
osc_attr_set()
ccc_attr_set()
ccc_req_attr_set()
lov_attr_set()
lovsub_req_attr_set()
lovsub_attr_set()

Looking at callchain for ccc_req_attr_set() (starting from the bottom and reverse engineering the callers):

  ->osc_enter_cache()
    ->osc_check_rpcs()
      ->osc_check_rpcs0()
        ->osc_send_oap_rpc()
          ->osc_build_req()
            ->cl_req_attr_set()
              ->ccc_req_attr_set()
                ->obdo_from_inode()

it appears that this function is only used by write and not setattr. It may be possible to use something like the following:

        obdo_from_inode(oa, NULL, &cl_i2info(ccc_object_inode(obj))->lli_fid, 0);

It would be nice to remove the duplication of "cl_attr_set" and "cl_req_attr_set", but I'm not well enough versed in this code to understand the consequences yet.

I think the following functions are no longer used by the master client, but I'm not really sure. It may be that even the MDS does not use them anymore on the orion branch, so they might be removed to reduce confusion and obsolete code.

lov_setattr()
lov_setattr_async()
lov_prep_setattr_set()
lov_update_setattr_set()
lov_fini_setattr_set()
cb_setattr_set()
osc_setattr()
osc_setattr_async()
osc_setattr_interpret()
osc_setattr_async_base()
osc_io_setattr_start()
osc_io_setattr_end()
osc_setattr_upcall()