nbd: Don't export a block device with no medium.
PATCH 1/2 nbd: Don't export a block device with no medium.
The device is exported with erroneous values and can't be read.
Before the patch:
$ sudo nbd-client localhost -p 10809 /dev/nbd0 -name floppy0 Negotiation: ..size = 17592186044415MB bs=1024, sz=18446744073709547520 bytes
$ sudo mount /dev/nbd0 /mnt/tmp/ mount: block device /dev/nbd0 is write-protected, mounting read-only mount: /dev/nbd0: can't read superblock
After the patch:
(qemu) nbd_server_add ide0-hd0
(qemu) nbd_server_add floppy0
Device 'floppy0' has no medium
--- blockdev-nbd.c | 4 ++++ 1 file changed, 4 insertions(+)
blockdev-nbd.c +4 -0
@@ -91,6 +91,10 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,error_set (errp,QERR_DEVICE_NOT_FOUND , device);return ; } +if (!bdrv_is_inserted (bs)) { +error_set (errp,QERR_DEVICE_HAS_NO_MEDIUM , device); +return ; + }if (!has_writable) { writable = false;
Signature · 1 line
1.8.3.2
PATCH 2/2 nbd: Don't validate from and len in NBD_CMD_DISC.
These values aren't used in this case.
Currently, the from field in the request sent by the nbd kernel module leading to a false error message when ending the connection with the client.
$ qemu-nbd some.img -v // After nbd-client -d /dev/nbd0 nbd.c:nbd_trip():L1031: From: 18446744073709551104, Len: 0, Size: 20971520, Offset: 0 nbd.c:nbd_trip():L1032: requested operation past EOF--bad client? nbd.c:nbd_receive_request():L638: read failed
--- nbd.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
nbd.c +4 -3
@@ -1001,6 +1001,7 @@ static void nbd_trip(void *opaque)struct nbd_request request;struct nbd_reply reply;ssize_t ret; +uint32_t type;TRACE ("Reading request." );if (client->closing) {
@@ -1023,8 +1024,8 @@ static void nbd_trip(void *opaque) reply.error = -ret; goto error_reply; } - -if ((request.from + request.len) > exp->size) { + type = request.type &NBD_CMD_MASK_COMMAND ; +if (type !=NBD_CMD_DISC && (request.from + request.len) > exp->size) { LOG("From: %" PRIu64 ", Len: %u, Size: %" PRIu64 ", Offset: %" PRIu64 "\n", request.from, request.len,
@@ -1033,7 +1034,7 @@ static void nbd_trip(void *opaque) goto invalid_request; } -switch (request.type &NBD_CMD_MASK_COMMAND ) { +switch (type) {case NBD_CMD_READ :TRACE ("Request type is READ" );
Signature · 1 line
1.8.3.2
Re: [Qemu-devel] [PATCH 2/2] nbd: Don't validate from and len in NBD_CMD_DISC.
Il 18/05/2014 12:50, Hani Benhabiles ha scritto:
Quoted 42 lines
These values aren't used in this case.
Currently, the from field in the request sent by the nbd kernel module leading to a false error message when ending the connection with the client.
$ qemu-nbd some.img -v // After nbd-client -d /dev/nbd0 nbd.c:nbd_trip():L1031: From: 18446744073709551104, Len: 0, Size: 20971520, Offset: 0 nbd.c:nbd_trip():L1032: requested operation past EOF--bad client? nbd.c:nbd_receive_request():L638: read failedSigned-off-by Hani Benhabiles <[email protected]>--- nbd.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)nbd.c +3
-2@@ -1001,6 +1001,7 @@ static void nbd_trip(void *opaque)struct nbd_request request;struct nbd_reply reply;ssize_t ret; +uint32_t type;TRACE ("Reading request." );if (client->closing) {@@ -1023,8 +1024,8 @@ static void nbd_trip(void *opaque) reply.error = -ret; goto error_reply; } - -if ((request.from + request.len) > exp->size) { + type = request.type &NBD_CMD_MASK_COMMAND ; +if (type !=NBD_CMD_DISC && (request.from + request.len) > exp->size) { LOG("From: %" PRIu64 ", Len: %u, Size: %" PRIu64 ", Offset: %" PRIu64 "\n", request.from, request.len,
nbd.c static void nbd_trip(void *opaque) · line 1040
Quoted 8 lines
nbd.c +1
-1@@ -1033,7 +1034,7 @@ static void nbd_trip(void *opaque) goto invalid_request; } -switch (request.type &NBD_CMD_MASK_COMMAND ) { +switch (type) {case NBD_CMD_READ :TRACE ("Request type is READ" );
Applied after renaming the variable from type to command (for consistency with e.g. nbd_co_receive_request).
Paolo
Re: [Qemu-devel] [PATCH 2/2] nbd: Don't validate from and len in NBD_CMD_DISC.
Quoted 45 lines · Paolo Bonzini
Il 18/05/2014 12:50, Hani Benhabiles ha scritto:
Quoted 44 lines
These values aren't used in this case.
Currently, the from field in the request sent by the nbd kernel module leading to a false error message when ending the connection with the client.
$ qemu-nbd some.img -v // After nbd-client -d /dev/nbd0 nbd.c:nbd_trip():L1031: From: 18446744073709551104, Len: 0, Size: 20971520, Offset: 0 nbd.c:nbd_trip():L1032: requested operation past EOF--bad client? nbd.c:nbd_receive_request():L638: read failedSigned-off-by Hani Benhabiles <[email protected]>---
nbd.c| 7 ++++---1 file changed, 4 insertions(+), 3 deletions(-)nbd.c +3
-2@@ -1001,6 +1001,7 @@ static void nbd_trip(void *opaque)struct nbd_request request;struct nbd_reply reply;ssize_t ret; +uint32_t type;TRACE ("Reading request." );if (client->closing) {@@ -1023,8 +1024,8 @@ static void nbd_trip(void *opaque) reply.error = -ret; goto error_reply; } - -if ((request.from + request.len) > exp->size) { + type = request.type &NBD_CMD_MASK_COMMAND ; +if (type !=NBD_CMD_DISC && (request.from + request.len) > exp->size) { LOG("From: %" PRIu64 ", Len: %u, Size: %" PRIu64 ", Offset: %" PRIu64 "\n", request.from, request.len,@@ -1033,7 +1034,7 @@ static void nbd_trip(void *opaque) goto invalid_request; }
Quoted 9 lines
nbd.c static void nbd_trip(void *opaque) · line 1040
Quoted 6 lines
nbd.c +1
-1-switch (request.type &NBD_CMD_MASK_COMMAND ) { +switch (type) {case NBD_CMD_READ :TRACE ("Request type is READ" );Applied after renaming the variable from type to command (for consistency with e.g.
nbd_co_receive_request).
No issue. Thanks!
Quoted 7 lines · Hani Benhabiles
The device is exported with erroneous values and can't be read.
Before the patch:
$ sudo nbd-client localhost -p 10809 /dev/nbd0 -name floppy0 Negotiation: ..size = 17592186044415MB bs=1024, sz=18446744073709547520 bytes
Quoted 8 lines
$ sudo mount /dev/nbd0 /mnt/tmp/ mount: block device /dev/nbd0 is write-protected, mounting read-only mount: /dev/nbd0: can't read superblockAfter the patch:
(qemu)nbd_server_addide0-hd0
(qemu)nbd_server_addfloppy0
Device 'floppy0' has no medium
I'm not sure this is the right direction. Maybe we should actually allow exporting devices with no medium to be able to insert medium later without re-starting the server? Especially with -t (persistent). I dunno. At least this is the decision which should not be left to a -trivial tree.
Thank you!
/mjt
Il 19/05/2014 10:00, Michael Tokarev ha scritto:
Quoted 17 lines
Quoted 7 lines · Hani Benhabiles
The device is exported with erroneous values and can't be read.
Before the patch:
$ sudo nbd-client localhost -p 10809 /dev/nbd0 -name floppy0 Negotiation: ..size = 17592186044415MB bs=1024, sz=18446744073709547520 bytesQuoted 8 lines
$ sudo mount /dev/nbd0 /mnt/tmp/ mount: block device /dev/nbd0 is write-protected, mounting read-only mount: /dev/nbd0: can't read superblockAfter the patch:
(qemu)nbd_server_addide0-hd0
(qemu)nbd_server_addfloppy0
Device 'floppy0' has no medium
Quoted 5 lines
I'm not sure this is the right direction. Maybe we should actually allow exporting devices with no medium to be able to insert medium later without re-starting the server? Especially with -t (persistent). I dunno. At least this is the decision which should not be left to a -trivial tree.
The protocol has no support for marking a device as no-medium, or for reporting a medium change, so there's nothing sane that we can do. (Upon a medium change, for example, the embedded NBD server will shut down all connections).
In particular, for qemu-nbd there is no support for named exports, so even with "-t" it makes sense to just exit with an error condition and ask the user to reinvoke qemu-nbd later.
I'll revive the nbd-next branch for these two patches. Thanks Michael for carrying the other two.
Paolo