Re: [PATCH] nbd in 2.5.3 does not work, and can cause severe damage when read-write

Pavel Machek (pavel@suse.cz)
Thu, 31 Jan 2002 22:21:57 +0100


Hi!

> I've got strange idea and tried to build diskless machine around
> 2.5.3... Besides problem with segfaulting crc32 (it is initialized after
> net/ipv4/ipconfig.c due to lib/lib.a being a library... I had to hardcode
> lib/crc32.o before --start-group in main Makefile, but it is another
> story) there is bad problem with NBD caused by BIO changes:
>
> (1) request flags were immediately put into on-wire request format.
> In the past, we had 0=READ, !0=WRITE. Now only REQ_RW bit determines
> direction. As nbd-server from nbd distribution package treats any
> non-zero value as write, it performs writes instead of read. Fortunately
> it will die due to other consistency checks on incoming request,
> but...

I have no problem with this.

> (2) nbd servers handle only up to 10240 byte requests. So setting max_sectors
> to 20 is needed, as otherwise nbd server commits suicide. Maximum request size
> should be handshaked during nbd initialization, but currently just use
> hardwired 20 sectors, so it will behave like it did in the past.

But please do not apply this one. Nbd servers should be fixed, and I
already have fix in cvs. (Besides, its trivial). Just make buffer in
server 1MB big.

I do not like idea of handshake.

So, first hunk is fine, but I do not like second one.
Pavel

> diff -urdN linux/drivers/block/nbd.c linux/drivers/block/nbd.c
> --- linux/drivers/block/nbd.c Thu Jan 10 18:15:38 2002
> +++ linux/drivers/block/nbd.c Thu Jan 31 00:24:50 2002
> @@ -155,14 +155,15 @@
> unsigned long size = req->nr_sectors << 9;
>
> DEBUG("NBD: sending control, ");
> +
> + rw = rq_data_dir(req);
> +
> request.magic = htonl(NBD_REQUEST_MAGIC);
> - request.type = htonl(req->flags);
> + request.type = htonl((rw & WRITE) ? 1 : 0);
> request.from = cpu_to_be64( (u64) req->sector << 9);
> request.len = htonl(size);
> memcpy(request.handle, &req, sizeof(req));
>
> - rw = rq_data_dir(req);
> -
> result = nbd_xmit(1, sock, (char *) &request, sizeof(request), rw & WRITE ? MSG_MORE : 0);
> if (result <= 0)
> FAIL("Sendmsg failed for control.");
> @@ -517,6 +518,7 @@
> blksize_size[MAJOR_NR] = nbd_blksizes;
> blk_size[MAJOR_NR] = nbd_sizes;
> blk_init_queue(BLK_DEFAULT_QUEUE(MAJOR_NR), do_nbd_request, &nbd_lock);
> + blk_queue_max_sectors(BLK_DEFAULT_QUEUE(MAJOR_NR), 20);
> for (i = 0; i < MAX_NBD; i++) {
> nbd_dev[i].refcnt = 0;
> nbd_dev[i].file = NULL;

-- 
(about SSSCA) "I don't say this lightly.  However, I really think that the U.S.
no longer is classifiable as a democracy, but rather as a plutocracy." --hpa

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/