Possible bug when getting batched xattrs

Possible bug when getting batched xattrs

[Joaquim Rocha]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

Hi,

I need to get a list of xattrs from one object (so I'm batching them in 
one ObjectReadOperation) but I don't know in advance if all of those 
xattrs exist.

I was expecting to figure out the ones that don't exist from the return 
code of each getxattr call but I have however noticed that as soon one 
nonexistent xattr is processed, its return code gets correctly set to 
-ENODATA but the remaining xattrs are never checked. And what is worse, 
the remaining getxattr op's return codes get assigned to 0.

I am attaching a small test case I've quickly written that proves the 
case. (I am using Ceph 0.80.5)

Could you confirm that this is not the intended behavior (that all read 
op's calls should be processed)?


Thank you in advance,

--
Joaquim Rocha
http://www.joaquimrocha.com

[-- Attachment #2: test-rados-xattrs.cpp --]
[-- Type: text/x-c++src, Size: 1533 bytes --]

#include <rados/librados.hpp>
#include <sstream>
#include <cstdio>
#include <sys/errno.h>

int main(int argc, char **argv)
{
  if (argc < 5)
  {
    fprintf(stdout, "Usage: %s CONFIGURATION_FILE POOL_NAME OBJ_NAME "
                    "ONE_EXISTING_XATTRIBUTE \n",
            argv[0]);

    return EINVAL;
  }

  librados::Rados rados;

  char *conf = argv[1];
  char *pool = argv[2];
  char *obj = argv[3];
  char *xattr = argv[4];

  rados.init(0);
  rados.conf_read_file(conf);
  rados.connect();

  librados::IoCtx ioctx;
  rados.ioctx_create(pool, ioctx);

  const size_t numXAttrs(5);
  librados::bufferlist buffs[numXAttrs];
  int xattrRets[numXAttrs];

  librados::ObjectReadOperation op;

  fprintf(stdout, "Ret codes before the op: \n");

  for (size_t i = 0; i < numXAttrs; i++)
  {
    xattrRets[i] = -1;
    fprintf(stdout, "  RET#%d: %d\n", i, xattrRets[i]);
  }

  fprintf(stdout, "\n--------\nData and ret codes after the op: \n");

  for (size_t i = 0; i < numXAttrs; i++)
  {
    std::stringstream stream;

    if (i == 0)
      stream << xattr;
    else
      stream << "nonexistent-xattr" << i;

    op.getxattr(stream.str().c_str(), &buffs[i], &xattrRets[i]);
  }

  ioctx.operate(obj, &op, 0);

  for (size_t i = 0; i < numXAttrs; i++)
  {
    librados::bufferlist *buff = &buffs[i];
    std::string value;

    if (buff->length() > 0)
      value = std::string(buff->c_str(), buff->length());

    fprintf(stdout, "  RET#%d: %s %d\n", i, value.c_str(), xattrRets[i]);
  }

  rados.shutdown();

  return 0;
}

Re: Possible bug when getting batched xattrs

[Sebastien Ponce]

I can confirm that head of master has the same behavior.

On top, having had a look at the osd code (name ly ReplicatedPG.cc
around line 4756 in master), it seems to be intended : we are in a for
loop going through the operations and executing them and the code says :
  if (result < 0)
      break;
So we exit the loop on the first error and do not execute the remaining
ops.

Sebastien


On Fri, 2014-10-17 at 11:28 +0200, Joaquim Rocha wrote:
> Hi,
> 
> I need to get a list of xattrs from one object (so I'm batching them in 
> one ObjectReadOperation) but I don't know in advance if all of those 
> xattrs exist.
> 
> I was expecting to figure out the ones that don't exist from the return 
> code of each getxattr call but I have however noticed that as soon one 
> nonexistent xattr is processed, its return code gets correctly set to 
> -ENODATA but the remaining xattrs are never checked. And what is worse, 
> the remaining getxattr op's return codes get assigned to 0.
> 
> I am attaching a small test case I've quickly written that proves the 
> case. (I am using Ceph 0.80.5)
> 
> Could you confirm that this is not the intended behavior (that all read 
> op's calls should be processed)?
> 
> 
> Thank you in advance,
> 
> --
> Joaquim Rocha
> http://www.joaquimrocha.com

Re: Possible bug when getting batched xattrs

[Sage Weil]

On Fri, 17 Oct 2014, Joaquim Rocha wrote:
> Hi,
> 
> I need to get a list of xattrs from one object (so I'm batching them in one
> ObjectReadOperation) but I don't know in advance if all of those xattrs exist.
> 
> I was expecting to figure out the ones that don't exist from the return code
> of each getxattr call but I have however noticed that as soon one nonexistent
> xattr is processed, its return code gets correctly set to -ENODATA but the
> remaining xattrs are never checked. And what is worse, the remaining getxattr
> op's return codes get assigned to 0.
> 
> I am attaching a small test case I've quickly written that proves the case. (I
> am using Ceph 0.80.5)
> 
> Could you confirm that this is not the intended behavior (that all read op's
> calls should be processed)?

This is by design.

What you want is the 

    void set_op_flags(ObjectOperationFlags flags);

method on the ObjectOperation, and the OP_FAILOK flag.  That will set a 
bit on the last op (getxattr) you added that will make processing continue 
even if it got an error.  Then you'll get the behavior you were 
looking for.

sage