All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] block: change default for discard and write zeroes to INT_MAX
@ 2015-02-02 14:48 Peter Lieven
  2015-02-02 16:13 ` Kevin Wolf
  2015-02-02 16:23 ` [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard Denis V. Lunev
  0 siblings, 2 replies; 12+ messages in thread
From: Peter Lieven @ 2015-02-02 14:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, den, Peter Lieven

do not trim requests if the driver does not supply a limit
through BlockLimits. For write zeroes we still keep a limit
for the unsupported path to avoid allocating a big bounce buffer.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Suggested-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Peter Lieven <pl@kamp.de>
---
 block.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/block.c b/block.c
index d45e4dd..ee7ff2c 100644
--- a/block.c
+++ b/block.c
@@ -3192,10 +3192,7 @@ int coroutine_fn bdrv_co_copy_on_readv(BlockDriverState *bs,
                             BDRV_REQ_COPY_ON_READ);
 }
 
-/* if no limit is specified in the BlockLimits use a default
- * of 32768 512-byte sectors (16 MiB) per request.
- */
-#define MAX_WRITE_ZEROES_DEFAULT 32768
+#define MAX_WRITE_ZEROES_BOUNCE_BUFFER 32768
 
 static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
     int64_t sector_num, int nb_sectors, BdrvRequestFlags flags)
@@ -3206,7 +3203,7 @@ static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
     int ret = 0;
 
     int max_write_zeroes = bs->bl.max_write_zeroes ?
-                           bs->bl.max_write_zeroes : MAX_WRITE_ZEROES_DEFAULT;
+                           bs->bl.max_write_zeroes : INT_MAX;
 
     while (nb_sectors > 0 && !ret) {
         int num = nb_sectors;
@@ -3242,7 +3239,7 @@ static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
         if (ret == -ENOTSUP) {
             /* Fall back to bounce buffer if write zeroes is unsupported */
             int max_xfer_len = MIN_NON_ZERO(bs->bl.max_transfer_length,
-                                            MAX_WRITE_ZEROES_DEFAULT);
+                                            MAX_WRITE_ZEROES_BOUNCE_BUFFER);
             num = MIN(num, max_xfer_len);
             iov.iov_len = num * BDRV_SECTOR_SIZE;
             if (iov.iov_base == NULL) {
@@ -5097,11 +5094,6 @@ static void coroutine_fn bdrv_discard_co_entry(void *opaque)
     rwco->ret = bdrv_co_discard(rwco->bs, rwco->sector_num, rwco->nb_sectors);
 }
 
-/* if no limit is specified in the BlockLimits use a default
- * of 32768 512-byte sectors (16 MiB) per request.
- */
-#define MAX_DISCARD_DEFAULT 32768
-
 int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
                                  int nb_sectors)
 {
@@ -5126,7 +5118,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
         return 0;
     }
 
-    max_discard = bs->bl.max_discard ?  bs->bl.max_discard : MAX_DISCARD_DEFAULT;
+    max_discard = bs->bl.max_discard ?  bs->bl.max_discard : INT_MAX;
     while (nb_sectors > 0) {
         int ret;
         int num = nb_sectors;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH] block: change default for discard and write zeroes to INT_MAX
  2015-02-02 14:48 [Qemu-devel] [PATCH] block: change default for discard and write zeroes to INT_MAX Peter Lieven
@ 2015-02-02 16:13 ` Kevin Wolf
  2015-02-02 16:25   ` Denis V. Lunev
  2015-02-02 16:23 ` [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard Denis V. Lunev
  1 sibling, 1 reply; 12+ messages in thread
From: Kevin Wolf @ 2015-02-02 16:13 UTC (permalink / raw)
  To: Peter Lieven; +Cc: den, qemu-devel

Am 02.02.2015 um 15:48 hat Peter Lieven geschrieben:
> do not trim requests if the driver does not supply a limit
> through BlockLimits. For write zeroes we still keep a limit
> for the unsupported path to avoid allocating a big bounce buffer.
> 
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Suggested-by: Denis V. Lunev <den@openvz.org>
> Signed-off-by: Peter Lieven <pl@kamp.de>

Thanks, applied to the block branch (and removed 'block/raw-posix: set
max_write_zeroes to INT_MAX for regular files' from the queue).

Kevin

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard
  2015-02-02 14:48 [Qemu-devel] [PATCH] block: change default for discard and write zeroes to INT_MAX Peter Lieven
  2015-02-02 16:13 ` Kevin Wolf
@ 2015-02-02 16:23 ` Denis V. Lunev
  2015-02-02 16:23   ` [Qemu-devel] [PATCH 2/2] nbd: " Denis V. Lunev
  1 sibling, 1 reply; 12+ messages in thread
From: Denis V. Lunev @ 2015-02-02 16:23 UTC (permalink / raw)
  Cc: Kevin Wolf, Denis V. Lunev, Peter Lieven, qemu-devel

qemu_gluster_co_discard calculates size to discard as follows
    size_t size = nb_sectors * BDRV_SECTOR_SIZE;
    ret = glfs_discard_async(s->fd, offset, size, &gluster_finish_aiocb, acb);

glfs_discard_async is declared as follows:
  int glfs_discard_async (glfs_fd_t *fd, off_t length, size_t lent,
                          glfs_io_cbk fn, void *data) __THROW
This is problematic on i686 as sizeof(size_t) == 4.

Set bl_max_discard to INT_MAX >> BDRV_SECTOR_BITS to avoid overflow.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Peter Lieven <pl@kamp.de>
---
 block/gluster.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/block/gluster.c b/block/gluster.c
index 1eb3a8c..ed09691 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -622,6 +622,11 @@ out:
     return ret;
 }
 
+static void qemu_gluster_refresh_limits(BlockDriverState *bs, Error **errp)
+{
+    bs->bl.max_discard = INT_MAX >> BDRV_SECTOR_BITS;
+}
+
 #ifdef CONFIG_GLUSTERFS_DISCARD
 static coroutine_fn int qemu_gluster_co_discard(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors)
@@ -735,6 +740,7 @@ static BlockDriver bdrv_gluster = {
 #ifdef CONFIG_GLUSTERFS_DISCARD
     .bdrv_co_discard              = qemu_gluster_co_discard,
 #endif
+    .bdrv_refresh_limits          = qemu_gluster_refresh_limits,
 #ifdef CONFIG_GLUSTERFS_ZEROFILL
     .bdrv_co_write_zeroes         = qemu_gluster_co_write_zeroes,
 #endif
@@ -762,6 +768,7 @@ static BlockDriver bdrv_gluster_tcp = {
 #ifdef CONFIG_GLUSTERFS_DISCARD
     .bdrv_co_discard              = qemu_gluster_co_discard,
 #endif
+    .bdrv_refresh_limits          = qemu_gluster_refresh_limits,
 #ifdef CONFIG_GLUSTERFS_ZEROFILL
     .bdrv_co_write_zeroes         = qemu_gluster_co_write_zeroes,
 #endif
@@ -789,6 +796,7 @@ static BlockDriver bdrv_gluster_unix = {
 #ifdef CONFIG_GLUSTERFS_DISCARD
     .bdrv_co_discard              = qemu_gluster_co_discard,
 #endif
+    .bdrv_refresh_limits          = qemu_gluster_refresh_limits,
 #ifdef CONFIG_GLUSTERFS_ZEROFILL
     .bdrv_co_write_zeroes         = qemu_gluster_co_write_zeroes,
 #endif
@@ -816,6 +824,7 @@ static BlockDriver bdrv_gluster_rdma = {
 #ifdef CONFIG_GLUSTERFS_DISCARD
     .bdrv_co_discard              = qemu_gluster_co_discard,
 #endif
+    .bdrv_refresh_limits          = qemu_gluster_refresh_limits,
 #ifdef CONFIG_GLUSTERFS_ZEROFILL
     .bdrv_co_write_zeroes         = qemu_gluster_co_write_zeroes,
 #endif
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 2/2] nbd: fix max_discard
  2015-02-02 16:23 ` [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard Denis V. Lunev
@ 2015-02-02 16:23   ` Denis V. Lunev
  0 siblings, 0 replies; 12+ messages in thread
From: Denis V. Lunev @ 2015-02-02 16:23 UTC (permalink / raw)
  Cc: Kevin Wolf, Denis V. Lunev, Peter Lieven, qemu-devel

nbd_co_discard calls nbd_client_session_co_discard which uses uint32_t
as the length in bytes of the data to discard due to the following
definition:

struct nbd_request {
    uint32_t magic;
    uint32_t type;
    uint64_t handle;
    uint64_t from;
    uint32_t len; <-- the length of data to be discarded, in bytes
} QEMU_PACKED;

Thus we should limit bl_max_discard to INT_MAX >> BDRV_SECTOR_BITS to avoid
overflow.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Peter Lieven <pl@kamp.de>
---
 block/nbd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/block/nbd.c b/block/nbd.c
index 04cc845..008d52e 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -301,6 +301,11 @@ static int nbd_co_flush(BlockDriverState *bs)
     return nbd_client_session_co_flush(&s->client);
 }
 
+static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)
+{
+    bs->bl.max_discard = INT_MAX >> BDRV_SECTOR_BITS;
+}
+
 static int nbd_co_discard(BlockDriverState *bs, int64_t sector_num,
                           int nb_sectors)
 {
@@ -396,6 +401,7 @@ static BlockDriver bdrv_nbd = {
     .bdrv_close                 = nbd_close,
     .bdrv_co_flush_to_os        = nbd_co_flush,
     .bdrv_co_discard            = nbd_co_discard,
+    .bdrv_refresh_limits        = nbd_refresh_limits,
     .bdrv_getlength             = nbd_getlength,
     .bdrv_detach_aio_context    = nbd_detach_aio_context,
     .bdrv_attach_aio_context    = nbd_attach_aio_context,
@@ -413,6 +419,7 @@ static BlockDriver bdrv_nbd_tcp = {
     .bdrv_close                 = nbd_close,
     .bdrv_co_flush_to_os        = nbd_co_flush,
     .bdrv_co_discard            = nbd_co_discard,
+    .bdrv_refresh_limits        = nbd_refresh_limits,
     .bdrv_getlength             = nbd_getlength,
     .bdrv_detach_aio_context    = nbd_detach_aio_context,
     .bdrv_attach_aio_context    = nbd_attach_aio_context,
@@ -430,6 +437,7 @@ static BlockDriver bdrv_nbd_unix = {
     .bdrv_close                 = nbd_close,
     .bdrv_co_flush_to_os        = nbd_co_flush,
     .bdrv_co_discard            = nbd_co_discard,
+    .bdrv_refresh_limits        = nbd_refresh_limits,
     .bdrv_getlength             = nbd_getlength,
     .bdrv_detach_aio_context    = nbd_detach_aio_context,
     .bdrv_attach_aio_context    = nbd_attach_aio_context,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH] block: change default for discard and write zeroes to INT_MAX
  2015-02-02 16:13 ` Kevin Wolf
@ 2015-02-02 16:25   ` Denis V. Lunev
  2015-02-02 16:45     ` Kevin Wolf
  0 siblings, 1 reply; 12+ messages in thread
From: Denis V. Lunev @ 2015-02-02 16:25 UTC (permalink / raw)
  To: Kevin Wolf, Peter Lieven; +Cc: qemu-devel

On 02/02/15 19:13, Kevin Wolf wrote:
> Am 02.02.2015 um 15:48 hat Peter Lieven geschrieben:
>> do not trim requests if the driver does not supply a limit
>> through BlockLimits. For write zeroes we still keep a limit
>> for the unsupported path to avoid allocating a big bounce buffer.
>>
>> Suggested-by: Kevin Wolf <kwolf@redhat.com>
>> Suggested-by: Denis V. Lunev <den@openvz.org>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
> Thanks, applied to the block branch (and removed 'block/raw-posix: set
> max_write_zeroes to INT_MAX for regular files' from the queue).
>
> Kevin
double checked the code.

There are 2 things to patch for discard, write_zeroes is OK for me.
Sorry, for not paying attention for discard branch :(

Den

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH] block: change default for discard and write zeroes to INT_MAX
  2015-02-02 16:25   ` Denis V. Lunev
@ 2015-02-02 16:45     ` Kevin Wolf
  2015-02-02 17:00       ` Denis V. Lunev
  0 siblings, 1 reply; 12+ messages in thread
From: Kevin Wolf @ 2015-02-02 16:45 UTC (permalink / raw)
  To: Denis V. Lunev; +Cc: Peter Lieven, qemu-devel

Am 02.02.2015 um 17:25 hat Denis V. Lunev geschrieben:
> On 02/02/15 19:13, Kevin Wolf wrote:
> >Am 02.02.2015 um 15:48 hat Peter Lieven geschrieben:
> >>do not trim requests if the driver does not supply a limit
> >>through BlockLimits. For write zeroes we still keep a limit
> >>for the unsupported path to avoid allocating a big bounce buffer.
> >>
> >>Suggested-by: Kevin Wolf <kwolf@redhat.com>
> >>Suggested-by: Denis V. Lunev <den@openvz.org>
> >>Signed-off-by: Peter Lieven <pl@kamp.de>
> >Thanks, applied to the block branch (and removed 'block/raw-posix: set
> >max_write_zeroes to INT_MAX for regular files' from the queue).
> >
> >Kevin
> double checked the code.
> 
> There are 2 things to patch for discard, write_zeroes is OK for me.
> Sorry, for not paying attention for discard branch :(

Good catch, thanks!

But shouldn't we use the actual limits instead of INT_MAX, i.e. SIZE_MAX
for gluster and UINT32_MAX for nbd?

Kevin

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH] block: change default for discard and write zeroes to INT_MAX
  2015-02-02 16:45     ` Kevin Wolf
@ 2015-02-02 17:00       ` Denis V. Lunev
  2015-02-02 17:34         ` Paolo Bonzini
  0 siblings, 1 reply; 12+ messages in thread
From: Denis V. Lunev @ 2015-02-02 17:00 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: Peter Lieven, qemu-devel

On 02/02/15 19:45, Kevin Wolf wrote:
> Am 02.02.2015 um 17:25 hat Denis V. Lunev geschrieben:
>> On 02/02/15 19:13, Kevin Wolf wrote:
>>> Am 02.02.2015 um 15:48 hat Peter Lieven geschrieben:
>>>> do not trim requests if the driver does not supply a limit
>>>> through BlockLimits. For write zeroes we still keep a limit
>>>> for the unsupported path to avoid allocating a big bounce buffer.
>>>>
>>>> Suggested-by: Kevin Wolf <kwolf@redhat.com>
>>>> Suggested-by: Denis V. Lunev <den@openvz.org>
>>>> Signed-off-by: Peter Lieven <pl@kamp.de>
>>> Thanks, applied to the block branch (and removed 'block/raw-posix: set
>>> max_write_zeroes to INT_MAX for regular files' from the queue).
>>>
>>> Kevin
>> double checked the code.
>>
>> There are 2 things to patch for discard, write_zeroes is OK for me.
>> Sorry, for not paying attention for discard branch :(
> Good catch, thanks!
>
> But shouldn't we use the actual limits instead of INT_MAX, i.e. SIZE_MAX
> for gluster and UINT32_MAX for nbd?
>
> Kevin
You are absolutely correct for NBD case but I do not get the
point about SIZE_MAX for gluster. There is no such definition
in their git at git://github.com/gluster/glusterfs nor in
public API headers in Ubuntu :(

Regards,
     Den

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH] block: change default for discard and write zeroes to INT_MAX
  2015-02-02 17:00       ` Denis V. Lunev
@ 2015-02-02 17:34         ` Paolo Bonzini
  2015-02-02 17:40           ` Denis V. Lunev
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2015-02-02 17:34 UTC (permalink / raw)
  To: Denis V. Lunev, Kevin Wolf; +Cc: Peter Lieven, qemu-devel



On 02/02/2015 18:00, Denis V. Lunev wrote:
>>
> You are absolutely correct for NBD case but I do not get the
> point about SIZE_MAX for gluster. There is no such definition
> in their git at git://github.com/gluster/glusterfs nor in
> public API headers in Ubuntu :(

SIZE_MAX is defined in stdint.h (provided by the C library).

Paolo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH] block: change default for discard and write zeroes to INT_MAX
  2015-02-02 17:34         ` Paolo Bonzini
@ 2015-02-02 17:40           ` Denis V. Lunev
  0 siblings, 0 replies; 12+ messages in thread
From: Denis V. Lunev @ 2015-02-02 17:40 UTC (permalink / raw)
  To: Paolo Bonzini, Denis V. Lunev, Kevin Wolf; +Cc: Peter Lieven, qemu-devel

On 02/02/15 20:34, Paolo Bonzini wrote:
>
>
> On 02/02/2015 18:00, Denis V. Lunev wrote:
>>>
>> You are absolutely correct for NBD case but I do not get the
>> point about SIZE_MAX for gluster. There is no such definition
>> in their git at git://github.com/gluster/glusterfs nor in
>> public API headers in Ubuntu :(
>
> SIZE_MAX is defined in stdint.h (provided by the C library).
>
> Paolo
>
In this case the code should avoid overflow on 64bit arch.

OK, re-spinning.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 2/2] nbd: fix max_discard
  2015-02-02 20:09 [Qemu-devel] [PATCH v3 0/2] fix max_discard for NBD/gluster block drivers Denis V. Lunev
@ 2015-02-02 20:09 ` Denis V. Lunev
  0 siblings, 0 replies; 12+ messages in thread
From: Denis V. Lunev @ 2015-02-02 20:09 UTC (permalink / raw)
  Cc: Kevin Wolf, Denis V. Lunev, qemu-devel

nbd_co_discard calls nbd_client_session_co_discard which uses uint32_t
as the length in bytes of the data to discard due to the following
definition:

struct nbd_request {
    uint32_t magic;
    uint32_t type;
    uint64_t handle;
    uint64_t from;
    uint32_t len; <-- the length of data to be discarded, in bytes
} QEMU_PACKED;

Thus we should limit bl_max_discard to UINT32_MAX >> BDRV_SECTOR_BITS to
avoid overflow.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Peter Lieven <pl@kamp.de>
CC: Kevin Wolf <kwolf@redhat.com>
---
 block/nbd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/block/nbd.c b/block/nbd.c
index 04cc845..99af713 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -301,6 +301,11 @@ static int nbd_co_flush(BlockDriverState *bs)
     return nbd_client_session_co_flush(&s->client);
 }
 
+static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)
+{
+    bs->bl.max_discard = UINT32_MAX >> BDRV_SECTOR_BITS;
+}
+
 static int nbd_co_discard(BlockDriverState *bs, int64_t sector_num,
                           int nb_sectors)
 {
@@ -396,6 +401,7 @@ static BlockDriver bdrv_nbd = {
     .bdrv_close                 = nbd_close,
     .bdrv_co_flush_to_os        = nbd_co_flush,
     .bdrv_co_discard            = nbd_co_discard,
+    .bdrv_refresh_limits        = nbd_refresh_limits,
     .bdrv_getlength             = nbd_getlength,
     .bdrv_detach_aio_context    = nbd_detach_aio_context,
     .bdrv_attach_aio_context    = nbd_attach_aio_context,
@@ -413,6 +419,7 @@ static BlockDriver bdrv_nbd_tcp = {
     .bdrv_close                 = nbd_close,
     .bdrv_co_flush_to_os        = nbd_co_flush,
     .bdrv_co_discard            = nbd_co_discard,
+    .bdrv_refresh_limits        = nbd_refresh_limits,
     .bdrv_getlength             = nbd_getlength,
     .bdrv_detach_aio_context    = nbd_detach_aio_context,
     .bdrv_attach_aio_context    = nbd_attach_aio_context,
@@ -430,6 +437,7 @@ static BlockDriver bdrv_nbd_unix = {
     .bdrv_close                 = nbd_close,
     .bdrv_co_flush_to_os        = nbd_co_flush,
     .bdrv_co_discard            = nbd_co_discard,
+    .bdrv_refresh_limits        = nbd_refresh_limits,
     .bdrv_getlength             = nbd_getlength,
     .bdrv_detach_aio_context    = nbd_detach_aio_context,
     .bdrv_attach_aio_context    = nbd_attach_aio_context,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] nbd: fix max_discard
  2015-02-02 18:29 ` [Qemu-devel] [PATCH 2/2] nbd: fix max_discard Denis V. Lunev
@ 2015-02-02 19:55   ` Peter Lieven
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Lieven @ 2015-02-02 19:55 UTC (permalink / raw)
  To: Denis V. Lunev; +Cc: Kevin Wolf, qemu-devel

Am 02.02.2015 um 19:29 schrieb Denis V. Lunev:
> nbd_co_discard calls nbd_client_session_co_discard which uses uint32_t
> as the length in bytes of the data to discard due to the following
> definition:
>
> struct nbd_request {
>     uint32_t magic;
>     uint32_t type;
>     uint64_t handle;
>     uint64_t from;
>     uint32_t len; <-- the length of data to be discarded, in bytes
> } QEMU_PACKED;
>
> Thus we should limit bl_max_discard to UINT32_MAX >> BDRV_SECTOR_BITS to
> avoid overflow.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Peter Lieven <pl@kamp.de>
> ---
>  block/nbd.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/block/nbd.c b/block/nbd.c
> index 04cc845..99af713 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -301,6 +301,11 @@ static int nbd_co_flush(BlockDriverState *bs)
>      return nbd_client_session_co_flush(&s->client);
>  }
>  
> +static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)
> +{
> +    bs->bl.max_discard = UINT32_MAX >> BDRV_SECTOR_BITS;
> +}
> +
>  static int nbd_co_discard(BlockDriverState *bs, int64_t sector_num,
>                            int nb_sectors)
>  {
> @@ -396,6 +401,7 @@ static BlockDriver bdrv_nbd = {
>      .bdrv_close                 = nbd_close,
>      .bdrv_co_flush_to_os        = nbd_co_flush,
>      .bdrv_co_discard            = nbd_co_discard,
> +    .bdrv_refresh_limits        = nbd_refresh_limits,
>      .bdrv_getlength             = nbd_getlength,
>      .bdrv_detach_aio_context    = nbd_detach_aio_context,
>      .bdrv_attach_aio_context    = nbd_attach_aio_context,
> @@ -413,6 +419,7 @@ static BlockDriver bdrv_nbd_tcp = {
>      .bdrv_close                 = nbd_close,
>      .bdrv_co_flush_to_os        = nbd_co_flush,
>      .bdrv_co_discard            = nbd_co_discard,
> +    .bdrv_refresh_limits        = nbd_refresh_limits,
>      .bdrv_getlength             = nbd_getlength,
>      .bdrv_detach_aio_context    = nbd_detach_aio_context,
>      .bdrv_attach_aio_context    = nbd_attach_aio_context,
> @@ -430,6 +437,7 @@ static BlockDriver bdrv_nbd_unix = {
>      .bdrv_close                 = nbd_close,
>      .bdrv_co_flush_to_os        = nbd_co_flush,
>      .bdrv_co_discard            = nbd_co_discard,
> +    .bdrv_refresh_limits        = nbd_refresh_limits,
>      .bdrv_getlength             = nbd_getlength,
>      .bdrv_detach_aio_context    = nbd_detach_aio_context,
>      .bdrv_attach_aio_context    = nbd_attach_aio_context,

Reviewed-by: Peter Lieven <pl@kamp.de>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 2/2] nbd: fix max_discard
  2015-02-02 18:29 [Qemu-devel] [PATCH v2 0/2] fix max_discard for NBD/gluster block drivers Denis V. Lunev
@ 2015-02-02 18:29 ` Denis V. Lunev
  2015-02-02 19:55   ` Peter Lieven
  0 siblings, 1 reply; 12+ messages in thread
From: Denis V. Lunev @ 2015-02-02 18:29 UTC (permalink / raw)
  Cc: Kevin Wolf, Denis V. Lunev, Peter Lieven, qemu-devel

nbd_co_discard calls nbd_client_session_co_discard which uses uint32_t
as the length in bytes of the data to discard due to the following
definition:

struct nbd_request {
    uint32_t magic;
    uint32_t type;
    uint64_t handle;
    uint64_t from;
    uint32_t len; <-- the length of data to be discarded, in bytes
} QEMU_PACKED;

Thus we should limit bl_max_discard to UINT32_MAX >> BDRV_SECTOR_BITS to
avoid overflow.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Peter Lieven <pl@kamp.de>
---
 block/nbd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/block/nbd.c b/block/nbd.c
index 04cc845..99af713 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -301,6 +301,11 @@ static int nbd_co_flush(BlockDriverState *bs)
     return nbd_client_session_co_flush(&s->client);
 }
 
+static void nbd_refresh_limits(BlockDriverState *bs, Error **errp)
+{
+    bs->bl.max_discard = UINT32_MAX >> BDRV_SECTOR_BITS;
+}
+
 static int nbd_co_discard(BlockDriverState *bs, int64_t sector_num,
                           int nb_sectors)
 {
@@ -396,6 +401,7 @@ static BlockDriver bdrv_nbd = {
     .bdrv_close                 = nbd_close,
     .bdrv_co_flush_to_os        = nbd_co_flush,
     .bdrv_co_discard            = nbd_co_discard,
+    .bdrv_refresh_limits        = nbd_refresh_limits,
     .bdrv_getlength             = nbd_getlength,
     .bdrv_detach_aio_context    = nbd_detach_aio_context,
     .bdrv_attach_aio_context    = nbd_attach_aio_context,
@@ -413,6 +419,7 @@ static BlockDriver bdrv_nbd_tcp = {
     .bdrv_close                 = nbd_close,
     .bdrv_co_flush_to_os        = nbd_co_flush,
     .bdrv_co_discard            = nbd_co_discard,
+    .bdrv_refresh_limits        = nbd_refresh_limits,
     .bdrv_getlength             = nbd_getlength,
     .bdrv_detach_aio_context    = nbd_detach_aio_context,
     .bdrv_attach_aio_context    = nbd_attach_aio_context,
@@ -430,6 +437,7 @@ static BlockDriver bdrv_nbd_unix = {
     .bdrv_close                 = nbd_close,
     .bdrv_co_flush_to_os        = nbd_co_flush,
     .bdrv_co_discard            = nbd_co_discard,
+    .bdrv_refresh_limits        = nbd_refresh_limits,
     .bdrv_getlength             = nbd_getlength,
     .bdrv_detach_aio_context    = nbd_detach_aio_context,
     .bdrv_attach_aio_context    = nbd_attach_aio_context,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-02-02 20:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-02 14:48 [Qemu-devel] [PATCH] block: change default for discard and write zeroes to INT_MAX Peter Lieven
2015-02-02 16:13 ` Kevin Wolf
2015-02-02 16:25   ` Denis V. Lunev
2015-02-02 16:45     ` Kevin Wolf
2015-02-02 17:00       ` Denis V. Lunev
2015-02-02 17:34         ` Paolo Bonzini
2015-02-02 17:40           ` Denis V. Lunev
2015-02-02 16:23 ` [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard Denis V. Lunev
2015-02-02 16:23   ` [Qemu-devel] [PATCH 2/2] nbd: " Denis V. Lunev
2015-02-02 18:29 [Qemu-devel] [PATCH v2 0/2] fix max_discard for NBD/gluster block drivers Denis V. Lunev
2015-02-02 18:29 ` [Qemu-devel] [PATCH 2/2] nbd: fix max_discard Denis V. Lunev
2015-02-02 19:55   ` Peter Lieven
2015-02-02 20:09 [Qemu-devel] [PATCH v3 0/2] fix max_discard for NBD/gluster block drivers Denis V. Lunev
2015-02-02 20:09 ` [Qemu-devel] [PATCH 2/2] nbd: fix max_discard Denis V. Lunev

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.