fix librados aio read buffer handling

Commit Message

Rutger ter Borg Nov. 27, 2013, 8:49 a.m. UTC

On 2013-10-01 00:52, Josh Durgin wrote:
>
> I'm fine applying this now (with one fix). It's a nice cleanup
> even if things change more soon.
>
> For the C interface, the return value stored in the AioCompletionImpl
> needs to be the length read, so the caller can tell if a short read
> occurred (this is only possible when trying to read past the end of an
> object). This was being set in C_aio_Ack::finish(), but was removed by
> this patch.

Please find attached a revised patch against 0.72.1. I've added a fix 
for setting the return value in C_aio_Ack::finish(),

if (c->bl.length() > 0) {
   c->rval = c->bl.length();
}

Cheers,

Rutger

Comments

Josh Durgin Dec. 30, 2013, 8 p.m. UTC | #1

On 11/27/2013 12:49 AM, Rutger ter Borg wrote:
> On 2013-10-01 00:52, Josh Durgin wrote:
>>
>> I'm fine applying this now (with one fix). It's a nice cleanup
>> even if things change more soon.
>>
>> For the C interface, the return value stored in the AioCompletionImpl
>> needs to be the length read, so the caller can tell if a short read
>> occurred (this is only possible when trying to read past the end of an
>> object). This was being set in C_aio_Ack::finish(), but was removed by
>> this patch.
>
> Please find attached a revised patch against 0.72.1. I've added a fix
> for setting the return value in C_aio_Ack::finish(),
>
> if (c->bl.length() > 0) {
>    c->rval = c->bl.length();
> }

Sorry for the delay - I just added this to the master branch.

Thanks!
Josh

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff -u -p ceph-0.72.1/src/librados/AioCompletionImpl.h ceph-0.72.1-patched/src/librados/AioCompletionImpl.h
--- ceph-0.72.1/src/librados/AioCompletionImpl.h	2013-11-22 10:15:58.000000000 +0100
+++ ceph-0.72.1-patched/src/librados/AioCompletionImpl.h	2013-11-25 11:32:06.178467454 +0100
@@ -39,8 +39,7 @@  struct librados::AioCompletionImpl {
 
   // for read
   bool is_read;
-  bufferlist bl, *pbl;
-  char *buf;
+  bufferlist bl;
   unsigned maxlen;
 
   IoCtxImpl *io;
@@ -54,7 +53,7 @@  struct librados::AioCompletionImpl {
 			callback_safe(0),
 			callback_complete_arg(0),
 			callback_safe_arg(0),
-			is_read(false), pbl(0), buf(0), maxlen(0),
+			is_read(false), maxlen(0),
 			io(NULL), aio_write_seq(0), aio_write_list_item(this) { }
 
   int set_complete_callback(void *cb_arg, rados_callback_t cb) {
diff -u -p ceph-0.72.1/src/librados/IoCtxImpl.cc ceph-0.72.1-patched/src/librados/IoCtxImpl.cc
--- ceph-0.72.1/src/librados/IoCtxImpl.cc	2013-11-22 10:15:58.000000000 +0100
+++ ceph-0.72.1-patched/src/librados/IoCtxImpl.cc	2013-11-25 11:40:41.952777192 +0100
@@ -570,7 +570,6 @@  int librados::IoCtxImpl::aio_operate_rea
 
   c->is_read = true;
   c->io = this;
-  c->pbl = pbl;
 
   Mutex::Locker l(*lock);
   objecter->read(oid, oloc,
@@ -612,11 +611,10 @@  int librados::IoCtxImpl::aio_read(const
 
   c->is_read = true;
   c->io = this;
-  c->pbl = pbl;
 
   Mutex::Locker l(*lock);
   objecter->read(oid, oloc,
-		 off, len, snapid, &c->bl, 0,
+		 off, len, snapid, pbl, 0,
 		 onack, &c->objver);
   return 0;
 }
@@ -632,8 +630,9 @@  int librados::IoCtxImpl::aio_read(const
 
   c->is_read = true;
   c->io = this;
-  c->buf = buf;
   c->maxlen = len;
+  c->bl.clear();
+  c->bl.push_back( buffer::create_static( len, buf ) );
 
   Mutex::Locker l(*lock);
   objecter->read(oid, oloc,
@@ -668,7 +667,6 @@  int librados::IoCtxImpl::aio_sparse_read
 
   c->is_read = true;
   c->io = this;
-  c->pbl = NULL;
 
   onack->m_ops.sparse_read(off, len, m, data_bl, NULL);
 
@@ -1179,14 +1177,9 @@  void librados::IoCtxImpl::C_aio_Ack::fin
     c->safe = true;
   c->cond.Signal();
 
-  if (c->buf && c->bl.length() > 0) {
-    unsigned l = MIN(c->bl.length(), c->maxlen);
-    c->bl.copy(0, l, c->buf);
+  if (c->bl.length() > 0) {
     c->rval = c->bl.length();
   }
-  if (c->pbl) {
-    *c->pbl = c->bl;
-  }
 
   if (c->callback_complete) {
     c->io->client->finisher.queue(new C_AioComplete(c));