[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH] nbd/server: Nicer spelling of max
From: |
Stefano Garzarella |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH] nbd/server: Nicer spelling of max BLOCK_STATUS reply length |
Date: |
Mon, 13 May 2019 09:54:47 +0200 |
User-agent: |
NeoMutt/20180716 |
On Fri, May 10, 2019 at 10:17:35AM -0500, Eric Blake wrote:
> Commit 3d068aff (3.0) introduced NBD_MAX_BITMAP_EXTENTS as a limit on
> how large we would allow a reply to NBD_CMD_BLOCK_STATUS to grow when
> it is visiting a qemu:dirty-bitmap: context. Later, commit fb7afc79
> (3.1) reused the constant to limit base:allocation context replies,
> although the name is now less appropriate in that situation.
>
> Rename things, and improve the macro to use units.h for better
> legibility. Then reformat the comment to comply with checkpatch rules
> added in the meantime. No semantic change.
>
> Signed-off-by: Eric Blake <address@hidden>
> ---
> nbd/server.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/nbd/server.c b/nbd/server.c
> index e21bd501dc6..2c49744fc43 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -21,15 +21,18 @@
> #include "qapi/error.h"
> #include "trace.h"
> #include "nbd-internal.h"
> +#include "qemu/units.h"
>
> #define NBD_META_ID_BASE_ALLOCATION 0
> #define NBD_META_ID_DIRTY_BITMAP 1
>
> -/* NBD_MAX_BITMAP_EXTENTS: 1 mb of extents data. An empirical
> +/*
> + * NBD_MAX_BLOCK_STATUS_EXTENTS: 1 mb of extents data. An empirical
Since we are here, what about using MiB also in this comment block?
> * constant. If an increase is needed, note that the NBD protocol
> * recommends no larger than 32 mb, so that the client won't consider
> - * the reply as a denial of service attack. */
> -#define NBD_MAX_BITMAP_EXTENTS (0x100000 / 8)
> + * the reply as a denial of service attack.
> + */
> +#define NBD_MAX_BLOCK_STATUS_EXTENTS (1 * MiB / 8)
>
> static int system_errno_to_nbd_errno(int err)
> {
> @@ -1958,7 +1961,7 @@ static int nbd_co_send_block_status(NBDClient *client,
> uint64_t handle,
> Error **errp)
> {
> int ret;
> - unsigned int nb_extents = dont_fragment ? 1 : NBD_MAX_BITMAP_EXTENTS;
> + unsigned int nb_extents = dont_fragment ? 1 :
> NBD_MAX_BLOCK_STATUS_EXTENTS;
> NBDExtent *extents = g_new(NBDExtent, nb_extents);
> uint64_t final_length = length;
>
> @@ -2043,7 +2046,7 @@ static int nbd_co_send_bitmap(NBDClient *client,
> uint64_t handle,
> uint32_t context_id, Error **errp)
> {
> int ret;
> - unsigned int nb_extents = dont_fragment ? 1 : NBD_MAX_BITMAP_EXTENTS;
> + unsigned int nb_extents = dont_fragment ? 1 :
> NBD_MAX_BLOCK_STATUS_EXTENTS;
> NBDExtent *extents = g_new(NBDExtent, nb_extents);
> uint64_t final_length = length;
With or without changing the comment:
Reviewed-by: Stefano Garzarella <address@hidden>