| Message ID | 20190328105227.74725-1-sgarzare@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] block/gluster: limit the transfer size to 512 MiB | expand |
On Thu, Mar 28, 2019 at 11:52:27AM +0100, Stefano Garzarella wrote: > Several versions of GlusterFS (3.12? -> 6.0.1) fail when the > transfer size is greater or equal to 1024 MiB, so we are > limiting the transfer size to 512 MiB to avoid this rare issue. > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1691320 > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > > RFC: > Should I add a parameter to allow the user to modify the max_transfer > variable? No, that is not needed IMHO. In most environments where large files are used, sharding is enabled on the Gluster volumes. For VM images shards of 256MB are recommeneded (I think). When sharding is enabled, the writes will be limited to that size as well, hence the problem is not noticed on (most) production deployments. Reviewed-by: Niels de Vos <ndevos@redhat.com> > > > block/gluster.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/block/gluster.c b/block/gluster.c > index af64330211..e1e4eaa525 100644 > --- a/block/gluster.c > +++ b/block/gluster.c > @@ -9,6 +9,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/units.h" > #include <glusterfs/api/glfs.h> > #include "block/block_int.h" > #include "block/qdict.h" > @@ -37,6 +38,12 @@ > #define GLUSTER_DEBUG_MAX 9 > #define GLUSTER_OPT_LOGFILE "logfile" > #define GLUSTER_LOGFILE_DEFAULT "-" /* handled in libgfapi as /dev/stderr */ > +/* > + * Several versions of GlusterFS (3.12? -> 6.0.1) fail when the transfer size > + * is greater or equal to 1024 MiB, so we are limiting the transfer size to 512 > + * MiB to avoid this rare issue. > + */ > +#define GLUSTER_MAX_TRANSFER (512 * MiB) > > #define GERR_INDEX_HINT "hint: check in 'server' array index '%d'\n" > > @@ -879,6 +886,11 @@ out: > return ret; > } > > +static void qemu_gluster_refresh_limits(BlockDriverState *bs, Error **errp) > +{ > + bs->bl.max_transfer = GLUSTER_MAX_TRANSFER; > +} > + > static int qemu_gluster_reopen_prepare(BDRVReopenState *state, > BlockReopenQueue *queue, Error **errp) > { > @@ -1536,6 +1548,7 @@ static BlockDriver bdrv_gluster = { > .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes, > #endif > .bdrv_co_block_status = qemu_gluster_co_block_status, > + .bdrv_refresh_limits = qemu_gluster_refresh_limits, > .create_opts = &qemu_gluster_create_opts, > .strong_runtime_opts = gluster_strong_open_opts, > }; > @@ -1566,6 +1579,7 @@ static BlockDriver bdrv_gluster_tcp = { > .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes, > #endif > .bdrv_co_block_status = qemu_gluster_co_block_status, > + .bdrv_refresh_limits = qemu_gluster_refresh_limits, > .create_opts = &qemu_gluster_create_opts, > .strong_runtime_opts = gluster_strong_open_opts, > }; > @@ -1596,6 +1610,7 @@ static BlockDriver bdrv_gluster_unix = { > .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes, > #endif > .bdrv_co_block_status = qemu_gluster_co_block_status, > + .bdrv_refresh_limits = qemu_gluster_refresh_limits, > .create_opts = &qemu_gluster_create_opts, > .strong_runtime_opts = gluster_strong_open_opts, > }; > @@ -1632,6 +1647,7 @@ static BlockDriver bdrv_gluster_rdma = { > .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes, > #endif > .bdrv_co_block_status = qemu_gluster_co_block_status, > + .bdrv_refresh_limits = qemu_gluster_refresh_limits, > .create_opts = &qemu_gluster_create_opts, > .strong_runtime_opts = gluster_strong_open_opts, > }; > -- > 2.20.1 > >
Am 28.03.2019 um 11:52 hat Stefano Garzarella geschrieben: > Several versions of GlusterFS (3.12? -> 6.0.1) fail when the > transfer size is greater or equal to 1024 MiB, so we are > limiting the transfer size to 512 MiB to avoid this rare issue. > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1691320 > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > > RFC: > Should I add a parameter to allow the user to modify the max_transfer > variable? Ideally, gluster would have provided a way to detect automatically whether the bug is present. But since you suggest a user option, I'm afraid they haven't? Kevin
On Thu, Mar 28, 2019 at 12:59:55PM +0100, Kevin Wolf wrote: > Am 28.03.2019 um 11:52 hat Stefano Garzarella geschrieben: > > Several versions of GlusterFS (3.12? -> 6.0.1) fail when the > > transfer size is greater or equal to 1024 MiB, so we are > > limiting the transfer size to 512 MiB to avoid this rare issue. > > > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1691320 > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > --- > > > > RFC: > > Should I add a parameter to allow the user to modify the max_transfer > > variable? > > Ideally, gluster would have provided a way to detect automatically > whether the bug is present. But since you suggest a user option, I'm > afraid they haven't? Unfortunately, all version that I tried have this bug, also the latest available, for this reason, I hard coded the limit in the gluster driver. I also think that this limit is not reached very often for standard operations. Maybe I should check the version of the library during the configuration, when they will fix the issue. What do you suggest? Thanks, Stefano
Am 28.03.2019 um 14:35 hat Stefano Garzarella geschrieben: > On Thu, Mar 28, 2019 at 12:59:55PM +0100, Kevin Wolf wrote: > > Am 28.03.2019 um 11:52 hat Stefano Garzarella geschrieben: > > > Several versions of GlusterFS (3.12? -> 6.0.1) fail when the > > > transfer size is greater or equal to 1024 MiB, so we are > > > limiting the transfer size to 512 MiB to avoid this rare issue. > > > > > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1691320 > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > > --- > > > > > > RFC: > > > Should I add a parameter to allow the user to modify the max_transfer > > > variable? > > > > Ideally, gluster would have provided a way to detect automatically > > whether the bug is present. But since you suggest a user option, I'm > > afraid they haven't? > > Unfortunately, all version that I tried have this bug, also the latest > available, for this reason, I hard coded the limit in the gluster driver. > I also think that this limit is not reached very often for standard > operations. > > Maybe I should check the version of the library during the configuration, > when they will fix the issue. > > What do you suggest? Oh, sorry, I should have actually looked at the BZ. I assumed it was already fixed, but it's not. Then I think what you have is fine for now. Maybe you could suggest to the gluster people that they add some kind of a feature flag or something somewhere together with the fix so we can find out at runtime whether the installed gluster libs are fixed or not. Kevin
On Thu, Mar 28, 2019 at 03:42:49PM +0100, Kevin Wolf wrote: > Am 28.03.2019 um 14:35 hat Stefano Garzarella geschrieben: > > On Thu, Mar 28, 2019 at 12:59:55PM +0100, Kevin Wolf wrote: > > > Am 28.03.2019 um 11:52 hat Stefano Garzarella geschrieben: > > > > Several versions of GlusterFS (3.12? -> 6.0.1) fail when the > > > > transfer size is greater or equal to 1024 MiB, so we are > > > > limiting the transfer size to 512 MiB to avoid this rare issue. > > > > > > > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1691320 > > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > > > --- > > > > > > > > RFC: > > > > Should I add a parameter to allow the user to modify the max_transfer > > > > variable? > > > > > > Ideally, gluster would have provided a way to detect automatically > > > whether the bug is present. But since you suggest a user option, I'm > > > afraid they haven't? > > > > Unfortunately, all version that I tried have this bug, also the latest > > available, for this reason, I hard coded the limit in the gluster driver. > > I also think that this limit is not reached very often for standard > > operations. > > > > Maybe I should check the version of the library during the configuration, > > when they will fix the issue. > > > > What do you suggest? > > Oh, sorry, I should have actually looked at the BZ. I assumed it was > already fixed, but it's not. Then I think what you have is fine for now. Good :) Should I resend without RFC or it can go as is through your block tree? > > Maybe you could suggest to the gluster people that they add some kind of > a feature flag or something somewhere together with the fix so we can > find out at runtime whether the installed gluster libs are fixed or not. Thanks! I'll forward this useful suggestion! Stefano
Am 28.03.2019 um 16:27 hat Stefano Garzarella geschrieben: > On Thu, Mar 28, 2019 at 03:42:49PM +0100, Kevin Wolf wrote: > > Am 28.03.2019 um 14:35 hat Stefano Garzarella geschrieben: > > > On Thu, Mar 28, 2019 at 12:59:55PM +0100, Kevin Wolf wrote: > > > > Am 28.03.2019 um 11:52 hat Stefano Garzarella geschrieben: > > > > > Several versions of GlusterFS (3.12? -> 6.0.1) fail when the > > > > > transfer size is greater or equal to 1024 MiB, so we are > > > > > limiting the transfer size to 512 MiB to avoid this rare issue. > > > > > > > > > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1691320 > > > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > > > > --- > > > > > > > > > > RFC: > > > > > Should I add a parameter to allow the user to modify the max_transfer > > > > > variable? > > > > > > > > Ideally, gluster would have provided a way to detect automatically > > > > whether the bug is present. But since you suggest a user option, I'm > > > > afraid they haven't? > > > > > > Unfortunately, all version that I tried have this bug, also the latest > > > available, for this reason, I hard coded the limit in the gluster driver. > > > I also think that this limit is not reached very often for standard > > > operations. > > > > > > Maybe I should check the version of the library during the configuration, > > > when they will fix the issue. > > > > > > What do you suggest? > > > > Oh, sorry, I should have actually looked at the BZ. I assumed it was > > already fixed, but it's not. Then I think what you have is fine for now. > > Good :) > Should I resend without RFC or it can go as is through your block tree? No need to resend, I'll apply it right away. > > Maybe you could suggest to the gluster people that they add some kind of > > a feature flag or something somewhere together with the fix so we can > > find out at runtime whether the installed gluster libs are fixed or not. > > Thanks! I'll forward this useful suggestion! Thanks! Kevin
diff --git a/block/gluster.c b/block/gluster.c index af64330211..e1e4eaa525 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -9,6 +9,7 @@ */ #include "qemu/osdep.h" +#include "qemu/units.h" #include <glusterfs/api/glfs.h> #include "block/block_int.h" #include "block/qdict.h" @@ -37,6 +38,12 @@ #define GLUSTER_DEBUG_MAX 9 #define GLUSTER_OPT_LOGFILE "logfile" #define GLUSTER_LOGFILE_DEFAULT "-" /* handled in libgfapi as /dev/stderr */ +/* + * Several versions of GlusterFS (3.12? -> 6.0.1) fail when the transfer size + * is greater or equal to 1024 MiB, so we are limiting the transfer size to 512 + * MiB to avoid this rare issue. + */ +#define GLUSTER_MAX_TRANSFER (512 * MiB) #define GERR_INDEX_HINT "hint: check in 'server' array index '%d'\n" @@ -879,6 +886,11 @@ out: return ret; } +static void qemu_gluster_refresh_limits(BlockDriverState *bs, Error **errp) +{ + bs->bl.max_transfer = GLUSTER_MAX_TRANSFER; +} + static int qemu_gluster_reopen_prepare(BDRVReopenState *state, BlockReopenQueue *queue, Error **errp) { @@ -1536,6 +1548,7 @@ static BlockDriver bdrv_gluster = { .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes, #endif .bdrv_co_block_status = qemu_gluster_co_block_status, + .bdrv_refresh_limits = qemu_gluster_refresh_limits, .create_opts = &qemu_gluster_create_opts, .strong_runtime_opts = gluster_strong_open_opts, }; @@ -1566,6 +1579,7 @@ static BlockDriver bdrv_gluster_tcp = { .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes, #endif .bdrv_co_block_status = qemu_gluster_co_block_status, + .bdrv_refresh_limits = qemu_gluster_refresh_limits, .create_opts = &qemu_gluster_create_opts, .strong_runtime_opts = gluster_strong_open_opts, }; @@ -1596,6 +1610,7 @@ static BlockDriver bdrv_gluster_unix = { .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes, #endif .bdrv_co_block_status = qemu_gluster_co_block_status, + .bdrv_refresh_limits = qemu_gluster_refresh_limits, .create_opts = &qemu_gluster_create_opts, .strong_runtime_opts = gluster_strong_open_opts, }; @@ -1632,6 +1647,7 @@ static BlockDriver bdrv_gluster_rdma = { .bdrv_co_pwrite_zeroes = qemu_gluster_co_pwrite_zeroes, #endif .bdrv_co_block_status = qemu_gluster_co_block_status, + .bdrv_refresh_limits = qemu_gluster_refresh_limits, .create_opts = &qemu_gluster_create_opts, .strong_runtime_opts = gluster_strong_open_opts, };
Several versions of GlusterFS (3.12? -> 6.0.1) fail when the transfer size is greater or equal to 1024 MiB, so we are limiting the transfer size to 512 MiB to avoid this rare issue. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1691320 Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- RFC: Should I add a parameter to allow the user to modify the max_transfer variable? block/gluster.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)