Re: bugs..

"Michael O'Reilly" writes:

>
>Squid.1.2b15 + patches:
>
>#1. disk.c: diskHandleWriteComplete
>[ .. ]
> fd_bytes(fd, len, FD_WRITE);
> if (len < 0) {
> if (!ignoreErrno(errno)) {
> status = errno == ENOSPC ? DISK_NO_SPACE_LEFT : DISK_ERROR;
> debug(50, 1) ("diskHandleWrite: FD %d: disk write error: %s\n",
> fd, xstrerror());
> /*

we have 'errno = errcode' at the start of that function.

?

>
>All occurences of errno in that function should actually be 'errcode'
>because with the async-IO, the errno is passed across from a different
>thread.
>
>#2. store.c: storeAbort
>[ .. ]
>#if USE_ASYNC_IO
> /* Need to cancel any pending ASYNC writes right now */
> if (mem->swapout.fd >= 0)
> aioCancel(mem->swapout.fd, NULL);
>#endif
> /* but dont close if a disk write is queued, the handler will close up */
> if (mem->swapout.queue_offset > mem->swapout.done_offset)
> return;
> /* we do */
> storeSwapOutFileClose(e);
>}
>
>But disk.c:diskHandleWriteComplete() simply frees the write_q blocks
>and returns without actually closing the file handle.

'handler' refers to storeSwapOutHandle() which should be closing
the file.

>#3. Is a small patch to improve the AIO sanity. I'm still seeing:
>
>grep diskHandleWriteComplete /var/log/s/cache.log
>1998/02/23 20:17:31| diskHandleWriteComplete: q->buf_offset > q->len (0xec3122
8,410, 108, 410 FD 183)
>1998/02/23 20:17:31| diskHandleWriteComplete: desc /w1/s/03/12/0000C907, type
2, open 1, flags 0x2
>1998/02/23 20:34:49| diskHandleWriteComplete: q->buf_offset > q->len (0xc8fd94
0,132, 98, 132 FD 127)
>1998/02/23 20:34:49| diskHandleWriteComplete: desc /w6/s/00/6A/0000351F, type
2, open 1, flags 0x2
>1998/02/24 12:22:20| diskHandleWriteComplete: q->buf_offset > q->len (0x1216c8
d8,864, 48, 864 FD 36)
>1998/02/24 12:22:20| diskHandleWriteComplete: desc /w2/s/swap.state, type 2, o
pen 1, flags 0x2
>
>so I've added the patch below, which checks the make sure the write
>request really does match the data passed back from the async IO
>call.
>
>note that I haven't actually rebooted with this patch yet, so
>'provisional' is an understatement.

got it.

Duane W.
Received on Tue Jul 29 2003 - 13:15:46 MDT