Re: [PATCH] nbd: Zero from and len fields in NBD_CMD_DISCONNECT.

2014-05-25 Thread Hani Benhabiles 
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.

2014-05-25 Thread Paul Clements 
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.

2014-05-25 Thread Hani Benhabiles 
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.

2014-05-25 Thread Hani Benhabiles 
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 kroo...@gmail.com 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 h...@linux.com
   ---
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.

2014-05-25 Thread Paul Clements 
On Sun, May 25, 2014 at 6:18 AM, Hani Benhabiles kroo...@gmail.com 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 kroo...@gmail.com 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 h...@linux.com
   ---
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.

2014-05-25 Thread Hani Benhabiles 
On Sun, May 25, 2014 at 02:11:13PM -0400, Paul Clements wrote:
 On Sun, May 25, 2014 at 6:18 AM, Hani Benhabiles kroo...@gmail.com 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 kroo...@gmail.com 
   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 h...@linux.com
---
 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.

2014-05-18 Thread Hani Benhabiles 
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/

Re: [PATCH] nbd: Zero from and len fields in NBD_CMD_DISCONNECT.

2014-05-18 Thread Hani Benhabiles 
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 kroo...@gmail.com 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 h...@linux.com
  ---
   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.

2014-05-16 Thread Hani Benhabiles 
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.

2014-05-16 Thread Hani Benhabiles 
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 h...@linux.com
---
 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/