Re: [PATCH] nbd: Zero from and len fields in NBD_CMD_DISCONNECT.
On Sun, May 25, 2014 at 02:11:13PM -0400, Paul Clements wrote: > On Sun, May 25, 2014 at 6:18 AM, Hani Benhabiles wrote: > > On Sun, May 18, 2014 at 10:11:13AM +0100, Hani Benhabiles wrote: > >> On Fri, May 16, 2014 at 10:56:33PM -0400, Paul Clements wrote: > >> > Agreed. But better yet, the request structure should just be zeroed when > >> > it's allocated. > >> > > >> > >> It is already initialized in __nbd_ioctl() with the blk_rq_init() call > >> which > >> sets the __sector value to -1 (which is 0xfe00 after the left > >> shifts.) > >> > >> This is the only (non-ugly / non-intrusive) way to do it afaict. > >> > > > > Ping! > > > > Anything blocking this patch ? > > It's cleaner to just zero the struct and get rid of the conditional > zeroing of specific fields. I'll prepare a patch in the next few days. > Sorry, I misunderstood which struct you were talking about! Will send a v2 shortly. > Thanks, > Paul > > >> > On Fri, May 16, 2014 at 7:43 PM, Hani Benhabiles > >> > wrote: > >> > > >> > > Len field is already set to zero, but not the from field which is sent > >> > > as > >> > > 0xfe00. This makes no sense, and may cause confuse server > >> > > implementations doing sanity checks (qemu-nbd is an example.) > >> > > > >> > > Signed-off-by: Hani Benhabiles > >> > > --- > >> > > drivers/block/nbd.c | 2 +- > >> > > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > > >> > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > >> > > index 3a70ea2..657bdac 100644 > >> > > --- a/drivers/block/nbd.c > >> > > +++ b/drivers/block/nbd.c > >> > > @@ -246,7 +246,7 @@ static int nbd_send_req(struct nbd_device *nbd, > >> > > struct > >> > > request *req) > >> > > request.magic = htonl(NBD_REQUEST_MAGIC); > >> > > request.type = htonl(nbd_cmd(req)); > >> > > > >> > > - if (nbd_cmd(req) == NBD_CMD_FLUSH) { > >> > > + if (nbd_cmd(req) == NBD_CMD_FLUSH || nbd_cmd(req) == > >> > > NBD_CMD_DISC) > >> > > { > >> > > /* Other values are reserved for FLUSH requests. */ > >> > > request.from = 0; > >> > > request.len = 0; > >> > > -- > >> > > 1.8.3.2 > >> > > > >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] nbd: Zero from and len fields in NBD_CMD_DISCONNECT.
On Sun, May 25, 2014 at 6:18 AM, Hani Benhabiles wrote: > On Sun, May 18, 2014 at 10:11:13AM +0100, Hani Benhabiles wrote: >> On Fri, May 16, 2014 at 10:56:33PM -0400, Paul Clements wrote: >> > Agreed. But better yet, the request structure should just be zeroed when >> > it's allocated. >> > >> >> It is already initialized in __nbd_ioctl() with the blk_rq_init() call which >> sets the __sector value to -1 (which is 0xfe00 after the left >> shifts.) >> >> This is the only (non-ugly / non-intrusive) way to do it afaict. >> > > Ping! > > Anything blocking this patch ? It's cleaner to just zero the struct and get rid of the conditional zeroing of specific fields. I'll prepare a patch in the next few days. Thanks, Paul >> > On Fri, May 16, 2014 at 7:43 PM, Hani Benhabiles wrote: >> > >> > > Len field is already set to zero, but not the from field which is sent as >> > > 0xfe00. This makes no sense, and may cause confuse server >> > > implementations doing sanity checks (qemu-nbd is an example.) >> > > >> > > Signed-off-by: Hani Benhabiles >> > > --- >> > > drivers/block/nbd.c | 2 +- >> > > 1 file changed, 1 insertion(+), 1 deletion(-) >> > > >> > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c >> > > index 3a70ea2..657bdac 100644 >> > > --- a/drivers/block/nbd.c >> > > +++ b/drivers/block/nbd.c >> > > @@ -246,7 +246,7 @@ static int nbd_send_req(struct nbd_device *nbd, >> > > struct >> > > request *req) >> > > request.magic = htonl(NBD_REQUEST_MAGIC); >> > > request.type = htonl(nbd_cmd(req)); >> > > >> > > - if (nbd_cmd(req) == NBD_CMD_FLUSH) { >> > > + if (nbd_cmd(req) == NBD_CMD_FLUSH || nbd_cmd(req) == >> > > NBD_CMD_DISC) >> > > { >> > > /* Other values are reserved for FLUSH requests. */ >> > > request.from = 0; >> > > request.len = 0; >> > > -- >> > > 1.8.3.2 >> > > >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] nbd: Zero from and len fields in NBD_CMD_DISCONNECT.
On Sun, May 18, 2014 at 10:11:13AM +0100, Hani Benhabiles wrote: > On Fri, May 16, 2014 at 10:56:33PM -0400, Paul Clements wrote: > > Agreed. But better yet, the request structure should just be zeroed when > > it's allocated. > > > > It is already initialized in __nbd_ioctl() with the blk_rq_init() call which > sets the __sector value to -1 (which is 0xfe00 after the left > shifts.) > > This is the only (non-ugly / non-intrusive) way to do it afaict. > Ping! Anything blocking this patch ? Thanks. > > -- > > Paul > > > > > > On Fri, May 16, 2014 at 7:43 PM, Hani Benhabiles wrote: > > > > > Len field is already set to zero, but not the from field which is sent as > > > 0xfe00. This makes no sense, and may cause confuse server > > > implementations doing sanity checks (qemu-nbd is an example.) > > > > > > Signed-off-by: Hani Benhabiles > > > --- > > > drivers/block/nbd.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > > > index 3a70ea2..657bdac 100644 > > > --- a/drivers/block/nbd.c > > > +++ b/drivers/block/nbd.c > > > @@ -246,7 +246,7 @@ static int nbd_send_req(struct nbd_device *nbd, struct > > > request *req) > > > request.magic = htonl(NBD_REQUEST_MAGIC); > > > request.type = htonl(nbd_cmd(req)); > > > > > > - if (nbd_cmd(req) == NBD_CMD_FLUSH) { > > > + if (nbd_cmd(req) == NBD_CMD_FLUSH || nbd_cmd(req) == NBD_CMD_DISC) > > > { > > > /* Other values are reserved for FLUSH requests. */ > > > request.from = 0; > > > request.len = 0; > > > -- > > > 1.8.3.2 > > > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] nbd: Zero from and len fields in NBD_CMD_DISCONNECT.
On Fri, May 16, 2014 at 10:56:33PM -0400, Paul Clements wrote: > Agreed. But better yet, the request structure should just be zeroed when > it's allocated. > It is already initialized in __nbd_ioctl() with the blk_rq_init() call which sets the __sector value to -1 (which is 0xfe00 after the left shifts.) This is the only (non-ugly / non-intrusive) way to do it afaict. > -- > Paul > > > On Fri, May 16, 2014 at 7:43 PM, Hani Benhabiles wrote: > > > Len field is already set to zero, but not the from field which is sent as > > 0xfe00. This makes no sense, and may cause confuse server > > implementations doing sanity checks (qemu-nbd is an example.) > > > > Signed-off-by: Hani Benhabiles > > --- > > drivers/block/nbd.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > > index 3a70ea2..657bdac 100644 > > --- a/drivers/block/nbd.c > > +++ b/drivers/block/nbd.c > > @@ -246,7 +246,7 @@ static int nbd_send_req(struct nbd_device *nbd, struct > > request *req) > > request.magic = htonl(NBD_REQUEST_MAGIC); > > request.type = htonl(nbd_cmd(req)); > > > > - if (nbd_cmd(req) == NBD_CMD_FLUSH) { > > + if (nbd_cmd(req) == NBD_CMD_FLUSH || nbd_cmd(req) == NBD_CMD_DISC) > > { > > /* Other values are reserved for FLUSH requests. */ > > request.from = 0; > > request.len = 0; > > -- > > 1.8.3.2 > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] nbd: Zero from and len fields in NBD_CMD_DISCONNECT.
Len field is already set to zero, but not the from field which is sent as 0xfe00. This makes no sense, and may cause confuse server implementations doing sanity checks (qemu-nbd is an example.) Signed-off-by: Hani Benhabiles --- drivers/block/nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 3a70ea2..657bdac 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -246,7 +246,7 @@ static int nbd_send_req(struct nbd_device *nbd, struct request *req) request.magic = htonl(NBD_REQUEST_MAGIC); request.type = htonl(nbd_cmd(req)); - if (nbd_cmd(req) == NBD_CMD_FLUSH) { + if (nbd_cmd(req) == NBD_CMD_FLUSH || nbd_cmd(req) == NBD_CMD_DISC) { /* Other values are reserved for FLUSH requests. */ request.from = 0; request.len = 0; -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/