[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 23/29] tools/libxl: Extend datacopier to support reading into a buffer



On 09/12/2014 09:49 AM, Wen Congyang wrote:
On 09/11/2014 01:11 AM, Andrew Cooper wrote:
From: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>

Implement a read-only mode for libxl__datacopier.  The mode is invoked
when readbuf is set and writefd is < 0.  On success, the callback passes
the number of bytes read.

Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
---
  tools/libxl/libxl_aoutils.c  |   59 +++++++++++++++++++++++++-----------------
  tools/libxl/libxl_internal.h |    4 ++-
  2 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 6502325..9183716 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -134,7 +134,7 @@ static void datacopier_check_state(libxl__egc *egc, 
libxl__datacopier_state *dc)
      STATE_AO_GC(dc->ao);
      int rc;

-    if (dc->used) {
+    if (dc->used && !dc->readbuf) {
          if (!libxl__ev_fd_isregistered(&dc->towrite)) {
              rc = libxl__ev_fd_register(gc, &dc->towrite, datacopier_writable,
                                         dc->writefd, POLLOUT);
@@ -147,7 +147,7 @@ static void datacopier_check_state(libxl__egc *egc, 
libxl__datacopier_state *dc)
          }
      } else if (!libxl__ev_fd_isregistered(&dc->toread) || dc->maxread == 0) {
          /* we have had eof */
-        datacopier_callback(egc, dc, 0, 0);
+        datacopier_callback(egc, dc, 0, dc->readbuf ? dc->used : 0);
          return;
      } else {
          /* nothing buffered, but still reading */
@@ -215,26 +215,31 @@ static void datacopier_readable(libxl__egc *egc, 
libxl__ev_fd *ev,
      }
      assert(revents & POLLIN);
      for (;;) {
-        while (dc->used >= dc->maxsz) {
-            libxl__datacopier_buf *rm = LIBXL_TAILQ_FIRST(&dc->bufs);
-            dc->used -= rm->used;
-            assert(dc->used >= 0);
-            LIBXL_TAILQ_REMOVE(&dc->bufs, rm, entry);
-            free(rm);
-        }
+        libxl__datacopier_buf *buf = NULL;
+        int r;
+
+        if (dc->readbuf) {
+            r = read(ev->fd, dc->readbuf + dc->used, dc->maxread);
+        } else {
+            while (dc->used >= dc->maxsz) {
+                libxl__datacopier_buf *rm = LIBXL_TAILQ_FIRST(&dc->bufs);
+                dc->used -= rm->used;
+                assert(dc->used >= 0);
+                LIBXL_TAILQ_REMOVE(&dc->bufs, rm, entry);
+                free(rm);
+            }

-        libxl__datacopier_buf *buf =
-            LIBXL_TAILQ_LAST(&dc->bufs, libxl__datacopier_bufs);
-        if (!buf || buf->used >= sizeof(buf->buf)) {
-            buf = malloc(sizeof(*buf));
-            if (!buf) libxl__alloc_failed(CTX, __func__, 1, sizeof(*buf));
-            buf->used = 0;
-            LIBXL_TAILQ_INSERT_TAIL(&dc->bufs, buf, entry);
-        }
-        int r = read(ev->fd,
-                     buf->buf + buf->used,
+            buf = LIBXL_TAILQ_LAST(&dc->bufs, libxl__datacopier_bufs);
+            if (!buf || buf->used >= sizeof(buf->buf)) {
+                buf = malloc(sizeof(*buf));
+                if (!buf) libxl__alloc_failed(CTX, __func__, 1, sizeof(*buf));
+                buf->used = 0;
+                LIBXL_TAILQ_INSERT_TAIL(&dc->bufs, buf, entry);
+            }
+            r = read(ev->fd, buf->buf + buf->used,
                       (sizeof(buf->buf) - buf->used) < dc->maxread ?
                           (sizeof(buf->buf) - buf->used) : dc->maxread);
+        }
          if (r < 0) {
              if (errno == EINTR) continue;
              if (errno == EWOULDBLOCK) break;
@@ -257,10 +262,12 @@ static void datacopier_readable(libxl__egc *egc, 
libxl__ev_fd *ev,
                  return;
              }
          }
-        buf->used += r;
+        if (!dc->readbuf) {
+            buf->used += r;
+            assert(buf->used <= sizeof(buf->buf));
+        }
          dc->used += r;
          dc->maxread -= r;
-        assert(buf->used <= sizeof(buf->buf));
          assert(dc->maxread >= 0);
          if (dc->maxread == 0)
              break;
@@ -324,9 +331,13 @@ int libxl__datacopier_start(libxl__datacopier_state *dc)
          if (rc) goto out;
      }

-    rc = libxl__ev_fd_register(gc, &dc->towrite, datacopier_writable,
-                               dc->writefd, POLLOUT);
-    if (rc) goto out;
+    if (dc->writefd >= 0) {
+        rc = libxl__ev_fd_register(gc, &dc->towrite, datacopier_writable,
+                                   dc->writefd, POLLOUT);
+        if (rc) goto out;
+    }
+
+    assert(dc->readfd >= 0 || dc->writefd >= 0);

If readfd and writefd are valid, and readbuf is not NULL, what it the behavior?


I'd say that's an invalid use of the API. There should probably be an assert to check this.

Regards
--
Ross Lagerwall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.