Add integrity test for huge writes. Fix speed calc problem.
authorAlex Bligh <alex@alex.org.uk>
Sat, 28 May 2011 19:18:13 +0000 (20:18 +0100)
committerAlex Bligh <alex@alex.org.uk>
Sat, 28 May 2011 19:18:13 +0000 (20:18 +0100)
This commit

* Fixes a speed calculation on the integrity tests (we were
  assuming total transferred bytes was the size of the disk,
  which it is for throughputtest but is not for the integrity
  test)

* Adds a binary blob integrityhuge-test.tr, which contains
  instructions to do huge oversize reads and writes with
  flushes over a 50MB disk. This is added as a binary blob
  so everyone has the same data.

* Adds make-integrityhuge.c - a simple program which will make
  the above binary blob (though as it uses random data it will
  not be the same each time)

* Adds an /integrity_huge test to "make check"

* Modifies integritytest() to use properly buffered I/O so it
  does not deadlock.

* Modifies integritytest() to keep handles in a hash table
  to avoid spurious memory accesses if the server returns
  a duff handle.

Makefile.am
integrityhuge-test.tr [new file with mode: 0644]
make-integrityhuge.c [new file with mode: 0644]
nbd-tester-client.c
simple_test

index 41e6358..f6b8b8d 100644 (file)
@@ -3,7 +3,7 @@ bin_PROGRAMS = nbd-server
 sbin_PROGRAMS = @NBD_CLIENT_NAME@
 EXTRA_PROGRAMS = nbd-client knbd-client nbd-trdump
 TESTS_ENVIRONMENT=$(srcdir)/simple_test
-TESTS = cmd cfg1 cfgmulti cfgnew cfgsize write flush integrity
+TESTS = cmd cfg1 cfgmulti cfgnew cfgsize write flush integrity integrityhuge
 check_PROGRAMS = nbd-tester-client
 knbd_client_SOURCES = nbd-client.c cliserv.h
 nbd_client_SOURCES = nbd-client.c cliserv.h
@@ -15,7 +15,7 @@ nbd_tester_client_CFLAGS = @CFLAGS@ @GLIB_CFLAGS@
 nbd_trdump_CFLAGS = @CFLAGS@ @GLIB_CFLAGS@
 nbd_server_LDADD = @GLIB_LIBS@
 nbd_tester_client_LDADD = @GLIB_LIBS@
-EXTRA_DIST = gznbd simple_test integrity-test.tr maketr
+EXTRA_DIST = gznbd simple_test integrity-test.tr integrityhuge-test.tr maketr
 dist-hook:
        rm -Rf `find $(distdir) -name '.svn' -type d -print`
 cmd:
@@ -26,4 +26,5 @@ cfgsize:
 write:
 flush:
 integrity:
+integrityhuge:
 
diff --git a/integrityhuge-test.tr b/integrityhuge-test.tr
new file mode 100644 (file)
index 0000000..e8d39e9
Binary files /dev/null and b/integrityhuge-test.tr differ
diff --git a/make-integrityhuge.c b/make-integrityhuge.c
new file mode 100644 (file)
index 0000000..ab60223
--- /dev/null
@@ -0,0 +1,94 @@
+/*
+ * make-integrityhuge
+ *
+ * Make a file to test oversize writes
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <stdint.h>
+#include <unistd.h>
+#include "config.h"
+#include "cliserv.h"
+#include "nbd.h"
+
+const uint64_t filesize=50*1000*1000;
+const uint64_t transactions = 250;
+
+static inline void dowrite(int f, void *buf, size_t len) {
+        ssize_t res;
+
+        while(len>0) {
+                if((res=write(f, buf, len)) <=0) {
+                       perror ("Error writing transactions");
+                       exit(1);
+                }
+                len-=res;
+                buf+=res;
+        }
+}
+
+static inline uint64_t getrandomuint64() {
+       uint64_t r=0;
+        int i;
+       /* RAND_MAX may be as low as 2^15 */
+       for (i= 1 ; i<=5; i++)
+               r ^= random() ^ (r << 15); 
+       return r;
+}
+
+int main(int argc, char**argv) {
+       struct nbd_request req;
+       struct nbd_reply rep;
+       uint64_t handle;
+       int writefd = 1; /*stdout*/
+
+       req.magic = htonl(NBD_REQUEST_MAGIC);
+       rep.magic = htonl(NBD_REPLY_MAGIC);
+       rep.error = 0;
+
+       for (handle = 0; handle < transactions; handle++)
+       {
+               uint64_t offset;
+               uint64_t length;
+               uint64_t flags;
+               uint32_t command;
+
+               /* make the length between 0x400 and the length of the disk -08x800, with all
+                * the bottom bits clear */
+               length = ((getrandomuint64() % (filesize-0x800)) & ~((uint64_t)0x3ff)) + 0x400;
+               /* generate an offset that will fit the length */
+               offset = (getrandomuint64() % (filesize-length)) & ~((uint64_t)0x3ff);
+               flags = getrandomuint64();
+
+               command = (flags & 0x01)?NBD_CMD_READ:NBD_CMD_WRITE;
+
+               if (!(flags & 0x0f))
+                       command = NBD_CMD_FLAG_FUA | NBD_CMD_WRITE;
+
+               if (!(flags & 0xf0)) {
+                       offset = 0;
+                       length = 0;
+                       command = NBD_CMD_FLUSH;
+               }
+
+               *(uint64_t *)(req.handle) = htonll(handle);
+               *(uint64_t *)(rep.handle) = htonll(handle);
+               req.type = htonl(command);
+               req.from = htonll(offset);
+               req.len = htonl(length);
+
+               dowrite(writefd, &req, sizeof(req));
+               dowrite(writefd, &rep, sizeof(rep));
+       }
+
+       req.type = htonl(NBD_CMD_DISC);
+       req.from = 0;
+       req.len = 0;
+       dowrite(writefd, &req, sizeof(req));
+       
+       return 0;
+}
index b5801ee..ed9214e 100644 (file)
@@ -63,6 +63,7 @@ typedef enum {
 
 struct reqcontext {
        uint64_t seq;
+       char orighandle[8];
        struct nbd_request req;
        struct reqcontext * next;
        struct reqcontext * prev;
@@ -74,6 +75,22 @@ struct rclist {
        int numitems;
 };
 
+struct chunk {
+       char * buffer;
+       char * readptr;
+       char * writeptr;
+       uint64_t space;
+       uint64_t length;
+       struct chunk * next;
+       struct chunk * prev;
+};
+
+struct chunklist {
+       struct chunk * head;
+       struct chunk * tail;
+       int numitems;
+};
+
 void rclist_unlink(struct rclist * l, struct reqcontext * p) {
        if (p && l) {
                struct reqcontext * prev = p->prev;
@@ -97,8 +114,7 @@ void rclist_unlink(struct rclist * l, struct reqcontext * p) {
 }                                                                      
 
 /* Add a new list item to the tail */
-void rclist_addtail(struct rclist * l, struct reqcontext * p)
-{
+void rclist_addtail(struct rclist * l, struct reqcontext * p) {
        if (!p || !l)
                return;
        if (l->tail) {
@@ -119,6 +135,127 @@ void rclist_addtail(struct rclist * l, struct reqcontext * p)
        l->numitems++;
 }
 
+void chunklist_unlink(struct chunklist * l, struct chunk * p) {
+       if (p && l) {
+               struct chunk * prev = p->prev;
+               struct chunk * next = p->next;
+               
+               /* Fix link to previous */
+               if (prev)
+                       prev->next = next;
+               else
+                       l->head = next;
+               
+               if (next)
+                       next->prev = prev;
+               else
+                       l->tail = prev;
+
+               p->prev = NULL;
+               p->next = NULL;
+               l->numitems--;
+       }                                                       
+}                                                                      
+
+/* Add a new list item to the tail */
+void chunklist_addtail(struct chunklist * l, struct chunk * p) {
+       if (!p || !l)
+               return;
+       if (l->tail) {
+               if (l->tail->next)
+                       g_warning("addtail found list tail has a next pointer");
+               l->tail->next = p;
+               p->next = NULL;
+               p->prev = l->tail;
+               l->tail = p;
+       } else {
+               if (l->head)
+                       g_warning("addtail found no list tail but a list head");
+               l->head = p;
+               l->tail = p;
+               p->prev = NULL;
+               p->next = NULL;
+       }
+       l->numitems++;
+}
+
+/* Add some new bytes to a chunklist */
+void addbuffer(struct chunklist * l, void * data, uint64_t len) {
+       void * buf;
+       uint64_t size = 64*1024;
+       struct chunk * pchunk;
+
+       while (len>0)
+       {
+               /* First see if there is a current chunk, and if it has space */
+               if (l->tail && l->tail->space) {
+                       uint64_t towrite = len;
+                       if (towrite > l->tail->space)
+                               towrite = l->tail->space;
+                       memcpy(l->tail->writeptr, data, towrite);
+                       l->tail->length += towrite;
+                       l->tail->space -= towrite;
+                       l->tail->writeptr += towrite;
+                       len -= towrite;
+                       data += towrite;
+               }
+
+               if (len>0) {
+                       /* We still need to write more, so prepare a new chunk */
+                       if ((NULL == (buf = malloc(size))) || (NULL == (pchunk = calloc(1, sizeof(struct chunk))))) {
+                               g_critical("Out of memory");
+                               exit (1);
+                       }
+
+                       pchunk->buffer = buf;
+                       pchunk->readptr = buf;
+                       pchunk->writeptr = buf;
+                       pchunk->space = size;
+                       chunklist_addtail(l, pchunk);
+               }
+       }
+
+}
+
+/* returns 0 on success, -1 on failure */
+int writebuffer(int fd, struct chunklist * l) {
+
+       struct chunk * pchunk = NULL;
+       int res;
+       if (!l)
+               return 0;
+
+       while (!pchunk)
+       {
+               pchunk = l->head;
+               if (!pchunk)
+                       return 0;
+               if (!(pchunk->length) || !(pchunk->readptr)) {
+                       chunklist_unlink(l, pchunk);
+                       free(pchunk->buffer);
+                       free(pchunk);
+                       pchunk = NULL;
+               }
+       }
+       
+       /* OK we have a chunk with some data in */
+       res = write(fd, pchunk->readptr, pchunk->length);
+       if (res==0)
+               errno = EAGAIN;
+       if (res<=0)
+               return -1;
+       pchunk->length -= res;
+       pchunk->readptr += res;
+       if (!pchunk->length) {
+               chunklist_unlink(l, pchunk);
+               free(pchunk->buffer);
+               free(pchunk);
+       }
+       return 0;
+}
+
+
+
 #define TEST_WRITE (1<<0)
 #define TEST_FLUSH (1<<1)
 
@@ -604,6 +741,18 @@ static inline void dumpcommand(char * text, uint32_t command)
 #endif
 }
 
+/* return an unused handle */
+uint64_t getrandomhandle(GHashTable *phash) {
+       uint64_t handle = 0;
+       int i;
+       do {
+               /* RAND_MAX may be as low as 2^15 */
+               for (i= 1 ; i<=5; i++)
+                       handle ^= random() ^ (handle << 15); 
+       } while (g_hash_table_lookup(phash, &handle));
+       return handle;
+}
+
 int integrity_test(gchar* hostname, int port, char* name, int sock,
                   char sock_is_open, char close_sock, int testflags) {
        struct nbd_reply rep;
@@ -625,9 +774,13 @@ int integrity_test(gchar* hostname, int port, char* name, int sock,
        uint64_t seq=1;
        uint64_t processed=0;
        uint64_t printer=0;
+       uint64_t xfer=0;
        int readtransactionfile = 1;
        struct rclist txqueue={NULL, NULL, 0};
        struct rclist inflight={NULL, NULL, 0};
+       struct chunklist txbuf={NULL, NULL, 0};
+
+       GHashTable *handlehash = g_hash_table_new(g_int64_hash, g_int64_equal);
 
        size=0;
        if(!sock_is_open) {
@@ -702,7 +855,7 @@ int integrity_test(gchar* hostname, int port, char* name, int sock,
                goto err_open;
        }
 
-       while (readtransactionfile || txqueue.numitems || inflight.numitems) {
+       while (readtransactionfile || txqueue.numitems || txbuf.numitems || inflight.numitems) {
                int ret;
 
                uint32_t magic;
@@ -717,7 +870,7 @@ int integrity_test(gchar* hostname, int port, char* name, int sock,
                FD_ZERO(&rset);
                if (readtransactionfile)
                        FD_SET(logfd, &rset);
-               if (txqueue.numitems)
+               if (txqueue.numitems || txbuf.numitems)
                        FD_SET(sock, &wset);
                if (inflight.numitems)
                        FD_SET(sock, &rset);
@@ -760,6 +913,7 @@ int integrity_test(gchar* hostname, int port, char* name, int sock,
                                                "Could not read transaction log: %s",
                                                strerror(errno));
                                prc->req.magic = htonl(NBD_REQUEST_MAGIC);
+                               memcpy(prc->orighandle, prc->req.handle, 8);
                                prc->seq=seq++;
                                if ((ntohl(prc->req.type) & NBD_CMD_MASK_COMMAND) == NBD_CMD_DISC) {
                                        /* no more to read; don't enqueue as no reply
@@ -800,12 +954,12 @@ int integrity_test(gchar* hostname, int port, char* name, int sock,
                /* See if we have a write we can do */
                if (FD_ISSET(sock, &wset))
                {
-                       prc = txqueue.head;
-                       if (!prc)
+                       if (!(txqueue.head) && !(txbuf.head))
                                g_warning("Socket write FD set but we shouldn't have been interested");
-                       else
+
+                       /* If there is no buffered data, generate some */
+                       if (!(txbuf.head) && (NULL != (prc = txqueue.head)))
                        {
-                       
                                rclist_unlink(&txqueue, prc);
                                rclist_addtail(&inflight, prc);
                                
@@ -820,15 +974,12 @@ int integrity_test(gchar* hostname, int port, char* name, int sock,
                                from = ntohll(prc->req.from);
                                len = ntohl(prc->req.len);
                                /* we rewrite the handle as they otherwise may not be unique */
-                               *((uint64_t*)(prc->req.handle))=htonll((uint64_t)prc);
-                               WRITE_ALL_ERRCHK(sock,
-                                                &(prc->req),
-                                                sizeof(struct nbd_request),
-                                                err_open,
-                                                "Could not write command: %s",
-                                                strerror(errno));
+                               *((uint64_t*)(prc->req.handle))=getrandomhandle(handlehash);
+                               g_hash_table_insert(handlehash, prc->req.handle, prc);
+                               addbuffer(&txbuf, &(prc->req), sizeof(struct nbd_request));
                                switch (command & NBD_CMD_MASK_COMMAND) {
                                case NBD_CMD_WRITE:
+                                       xfer+=len;
                                        while (len > 0) {
                                                uint64_t blknum = from>>9;
                                                char dbuf[512];
@@ -839,18 +990,15 @@ int integrity_test(gchar* hostname, int port, char* name, int sock,
                                                }
                                                /* work out what we should be writing */
                                                makebuf(dbuf, prc->seq, blknum);
-                                               WRITE_ALL_ERRCHK(sock,
-                                                                dbuf,
-                                                                512,
-                                                                err_open,
-                                                                "Could not write data: %s",
-                                                                strerror(errno));
+                                               addbuffer(&txbuf, dbuf, 512);
                                                from += 512;
                                                len -= 512;
                                        }
-                                       
-                               case NBD_CMD_DISC:
+                                       break;
                                case NBD_CMD_READ:
+                                       xfer+=len;
+                                       break;
+                               case NBD_CMD_DISC:
                                case NBD_CMD_FLUSH:
                                        break;
                                default:
@@ -862,6 +1010,13 @@ int integrity_test(gchar* hostname, int port, char* name, int sock,
                                
                                prc = NULL;
                        }
+
+                       /* there should be some now */
+                       if (writebuffer(sock, &txbuf)<0) {
+                               retval=-1;
+                               snprintf(errstr, errstr_len, "Failed to write to socket buffer: %s", strerror(errno));
+                               goto err_open;
+                       }
                        
                }
 
@@ -889,7 +1044,18 @@ int integrity_test(gchar* hostname, int port, char* name, int sock,
                                goto err_open;
                        }
                                
-                       prc=(struct reqcontext *)ntohll(*((uint64_t *)rep.handle));
+                       prc = g_hash_table_lookup(handlehash, rep.handle);
+                       if (!prc) {
+                               retval=-1;
+                               snprintf(errstr, errstr_len, "Unrecognised handle in reply: 0x%llX", *(long long unsigned int*)(rep.handle));
+                               goto err_open;
+                       }
+                       if (!g_hash_table_remove(handlehash, rep.handle)) {
+                               retval=-1;
+                               snprintf(errstr, errstr_len, "Could not remove handle from hash: 0x%llX", *(long long unsigned int*)(rep.handle));
+                               goto err_open;
+                       }
+
                        if (prc->req.magic != htonl(NBD_REQUEST_MAGIC)) {
                                retval=-1;
                                snprintf(errstr, errstr_len, "Bad magic in inflight data: %08x", prc->req.magic);
@@ -964,7 +1130,7 @@ int integrity_test(gchar* hostname, int port, char* name, int sock,
                goto err_open;
        }
        timespan=timeval_diff_to_double(&stop, &start);
-       speed=size/timespan;
+       speed=xfer/timespan;
        if(speed>1024) {
                speed=speed/1024.0;
                speedchar[0]='K';
@@ -999,6 +1165,8 @@ err:
        if (*errstr)
                g_warning("%s",errstr);
 
+       g_hash_table_destroy(handlehash);
+
        return retval;
 }
 
index 1ccf84b..c790311 100755 (executable)
@@ -5,6 +5,7 @@ tmpdir=`mktemp -d`
 conffile=${tmpdir}/nbd.conf
 pidfile=${tmpdir}/nbd.pid
 tmpnam=${tmpdir}/nbd.dd
+mydir=$(dirname "`readlink -f $0`")
 
 ulimit -c unlimited
 
@@ -145,7 +146,25 @@ EOF
                ./nbd-server -C ${conffile} -p ${pidfile} &
                PID=$!
                sleep 1
-               ./nbd-tester-client localhost -N export1 -i -t $(dirname $1)/integrity-test.tr
+               ./nbd-tester-client localhost -N export1 -i -t ${mydir}/integrity-test.tr
+               retval=$?
+       ;;
+       */integrityhuge)
+               # Integrity test
+               cat >${conffile} <<EOF
+[generic]
+[export1]
+       exportname = $tmpnam
+       flush = true
+       fua = true
+       rotational = true
+EOF
+               # we need a bigger disk
+               dd if=/dev/zero of=$tmpnam bs=1M count=50 >/dev/null 2>&1
+               ./nbd-server -C ${conffile} -p ${pidfile} &
+               PID=$!
+               sleep 1
+               ./nbd-tester-client localhost -N export1 -i -t ${mydir}/integrityhuge-test.tr
                retval=$?
        ;;
        *)