From cfbb9a0368a267a00f147d08253af5b48ca66ae7 Mon Sep 17 00:00:00 2001 From: Alex Bligh Date: Sat, 28 May 2011 20:18:13 +0100 Subject: [PATCH] Add integrity test for huge writes. Fix speed calc problem. 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 | 5 +- integrityhuge-test.tr | Bin 0 -> 11028 bytes make-integrityhuge.c | 94 +++++++++++++++++++++ nbd-tester-client.c | 218 +++++++++++++++++++++++++++++++++++++++++++------ simple_test | 21 ++++- 5 files changed, 310 insertions(+), 28 deletions(-) create mode 100644 integrityhuge-test.tr create mode 100644 make-integrityhuge.c diff --git a/Makefile.am b/Makefile.am index 41e6358..f6b8b8d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -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 index 0000000000000000000000000000000000000000..e8d39e9758cc87823b398f7aa2e37d708cf9d291 GIT binary patch literal 11028 zcmZYFeOOh+76$M&6D0{L8W|cD1}Y|+7UfZo3TA~xi9tn0MT%cVib91(b|f+_GfGTU z46HOLG*n7bO!6x-Q&cJ{G)yuoOiC=e*KT*sVxNb5-`Rg`&-3{6Wu4hGYvyq5lf3L= z!pk?2ly3}89XloefB&k_|63)0*Cmx`hX<{4Y`Y=S#;SWbH#-Nd278|5zMny=r3USJ zV0|QaOFdDMFIpd{_k6LD5v0lj(Vh>v7l?H=le*NZ;R3Lo#P+Kql~sYcouCF^V`(#w zg>@q5Ua~c|Fj=-dqD1`V#h=gZLn&%6mo;a21b#Z7=*b&V0(&9OyzZ9jYChcy~MgA zNyRr~ZZD|!GO?6Fx}_fNWnhDiRV#U2j3`GN3^iOXxiff9mRRfJas!2aH4Q+3z_bRbneMqmj=EGHBL&ZLw zKsqiCb3?&~8LN78UmqBNHVkYZ$^D*t$hTH*SOOkoqtll>OY=qRX zliQld8nXzn1I6~PBRyj<)-Vw29VGUnT2cqC=Qap(uMwNDnA9iMK7Wm4+xKju!D5{o zoF4{54MT+O<Qu)jI@gxYk&;?i{IrWPESeg1t_11M5jGjYE4K)DS6lI`^oxK4>E$ z_j;+}w2Nr88}0RwdxO}Q`0lZK)D4h3RBD*XeW$Tqm88yo zY(^gM-N9(@06Sjnv>;NKC!ieDlKn_fD|!#;WCfG>Bb{HXdw(N|;$d#_jz&xbi4%)J-vea5QbGE(`)Xzzm> z?w8zG*$27M-VeD+VzYTZ9Ee4m1i250jpFlzzcpqLK<JQ0K!^2|J<9NPX{r6$86UDAcAw4n$>zxQSOcEQ`M0%<<4wJy9 zi0#x&`g9%UrhuI+_BihscekOP4E0VCJCA$yf2@6I3gk{TR`sYO+Lwd5Qz3VnSuI$@V_;`U?jl|u?l`nFz|NH14E~%oY(LtWjupZ;E|kZ%QvuOJ%cg;y zC3b!-QEm+8&T_1`_suredqMA;4fXQV8G51NZnevbNa;}au4BD>%TtoOg@1oG!dh=nfz6WK@dc#2m11rd)cds9&2FM?R_@bK z?>w>24eFf-H9TXOTH#C7r5DC(ZK<<3WEi5BCYK`xFu-TH^o%fdP zRIDKz?6YDobdmA|qJ0)@j@0l~F;S}3&vL*%C%G}rq=s7cJ_mMzSn?q?CK78{;8^e6 zdA{AY-{Dz2FSetnE)>->i`0=c%w6c@dgB(E7iw@ge`Y8^y9n$HV(0EB#d8g6c)_vW z`TL^$fctq3C0ld-MaW$&mb-+?9gMk)A=iA`z$LYiFIx;A6|2;cRnmPOsN)9XRLLx z9BjTsrg=zBh{AgFq23je+r^*gUsmo4$XzLR6tCkzYkXINT_rWdb5D)Vz#3M8<&Bsx zU$z;QXbYg;)nZrkXR>?a(5?piy4X%Fq)R+#Uk6(#_8UG&-B*FOP;5tkC=y$dOX_QD z4HY@L-siSP?4mY4uUmV|8nADO&F9b3a~rXSH=u?$jnxxe@_QzjJ8wenTVh@HM8RR0 z`xfM`HCE;E7Tzr%?OG?-5Ob}Qdo+JW&uc@w4(!{;syR_aO_qHdEKe}L0(X->9ErKb zj`fajiP)3855)(dErHy3j8zkJ_#9=8!#iN#70bU(RHLgf_g$!g8v|b(>q)t-XIv__ zWA3aM`vup#+!u4#gWX`P$}J*Fib1;pYIsj_yTy{OwbuN5j`j8(J`8kRFn7uzccV!D z96;BUVGSF>ZZcNA%DrS;BHB%0H;XN-AM6KW+ww@IzlHV#sCSE4{w?i~R`F+UI$`zStzfr_eRwxfNe$+116wIIl=7Smwbo`O*zIEdxSwTD!`$s)KN5Rh z9G^9;Ir))ez3X^~*v{ujH~VAm4#=%CR=vpQv&k;BRbW3ByN{PBcUPF}AA{X#tPY7I z8f(?N6YAY%teKhW@7v`~ zcOXZg-dbbz`MfQshhYu1V84*uq33vQR-pX??3ZHa#`2zQjoFt_?^nhuUBPGIR?Phh za_b~Fu8q%>zG&+p_iM46ib$ne&-iP|{YGkNQC?zfVAhR5MV7TRyY^1+PX z+W;Qltxag_p@yT98(d6!g|!D972B~kkBJ?WPdd$dZ^t0_JFyGuN#9tHHGBv4Hi#Wx zLu#tkOB%o)7h6@p-wy?2?s2H$d$GOv4D)R)+V3IvgxHz2q>d+{Jpr~+>?mGC4@RMF z1p9-rI^MvLyN6-{3F!RB)0o*QhltmMib=z zBsCOzNNsGxdVd1@v)BjR+$Syjv)GQg^NZL@KHFbyJ&Ru;w^{6}2>x!o0&8dndrGWM zA^p2$Pl5ea?CnAP9bh2l{tEVVdu}rCLuqJFJJx&FXfamz_wcvm`_Z<5{Y`SW^BHlt zKic1*hTn}<%VLSvR-^qL>>07)k)$?R@9hj&-c +#include +#include +#include +#include +#include +#include +#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; +} diff --git a/nbd-tester-client.c b/nbd-tester-client.c index b5801ee..ed9214e 100644 --- a/nbd-tester-client.c +++ b/nbd-tester-client.c @@ -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; } diff --git a/simple_test b/simple_test index 1ccf84b..c790311 100755 --- a/simple_test +++ b/simple_test @@ -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} </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=$? ;; *) -- 1.7.10.4