* [Qemu-devel] [PATCH 0/2] block: Gluster 6 compatibility @ 2019-03-04 16:21 Niels de Vos 2019-03-04 16:21 ` [Qemu-devel] [PATCH 1/2] block/gluster: Handle changed glfs_ftruncate signature Niels de Vos 2019-03-04 16:21 ` [Qemu-devel] [PATCH 2/2] gluster: the glfs_io_cbk callback function pointer adds pre/post stat args Niels de Vos 0 siblings, 2 replies; 7+ messages in thread From: Niels de Vos @ 2019-03-04 16:21 UTC (permalink / raw) To: qemu-block; +Cc: Cole Robinson, integration, qemu-devel, Niels de Vos Gluster 6 is currently available as release candidate. There have been a few changes to libgfapi.so that need to be adapted by consuming projects like QEMU. Fedora Rawhide already contains glusterfs-6.0-RC0, and this prevents rebuilds of QEMU there (https://bugzilla.redhat.com/1684298). The following two changes should be sufficient to consume Gluster 6 once it is released. These have been tested on CentOS-7 with Gluster 5 and Gluster 6 (minimal manual qemu-img tests only). Cheers, Niels Niels de Vos (2): block/gluster: Handle changed glfs_ftruncate signature gluster: the glfs_io_cbk callback function pointer adds pre/post stat args block/gluster.c | 17 ++++++++++++++--- configure | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 1/2] block/gluster: Handle changed glfs_ftruncate signature 2019-03-04 16:21 [Qemu-devel] [PATCH 0/2] block: Gluster 6 compatibility Niels de Vos @ 2019-03-04 16:21 ` Niels de Vos 2019-03-04 16:41 ` Daniel P. Berrangé 2019-03-04 16:21 ` [Qemu-devel] [PATCH 2/2] gluster: the glfs_io_cbk callback function pointer adds pre/post stat args Niels de Vos 1 sibling, 1 reply; 7+ messages in thread From: Niels de Vos @ 2019-03-04 16:21 UTC (permalink / raw) To: qemu-block Cc: Cole Robinson, integration, qemu-devel, Niels de Vos, Prasanna Kumar Kalever From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> New versions of Glusters libgfapi.so have an updated glfs_ftruncate() function that returns additional 'struct stat' structures to enable advanced caching of attributes. This is useful for file servers, not so much for QEMU. Nevertheless, the API has changed and needs to be adopted. Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> Signed-off-by: Niels de Vos <ndevos@redhat.com> --- v4: rebase to current master branch v3: define old backwards compatible glfs_ftruncate() macro, from Eric Blake v2: do a compile check as suggested by Eric Blake --- block/gluster.c | 11 +++++++++-- configure | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/block/gluster.c b/block/gluster.c index af64330211..86e5278524 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -20,6 +20,10 @@ #include "qemu/option.h" #include "qemu/cutils.h" +#ifdef CONFIG_GLUSTERFS_LEGACY_FTRUNCATE +# define glfs_ftruncate(fd, offset, _u1, _u2) glfs_ftruncate(fd, offset) +#endif + #define GLUSTER_OPT_FILENAME "filename" #define GLUSTER_OPT_VOLUME "volume" #define GLUSTER_OPT_PATH "path" @@ -1005,6 +1009,7 @@ static int qemu_gluster_do_truncate(struct glfs_fd *fd, int64_t offset, PreallocMode prealloc, Error **errp) { int64_t current_length; + int ret; current_length = glfs_lseek(fd, 0, SEEK_END); if (current_length < 0) { @@ -1032,7 +1037,8 @@ static int qemu_gluster_do_truncate(struct glfs_fd *fd, int64_t offset, #endif /* CONFIG_GLUSTERFS_FALLOCATE */ #ifdef CONFIG_GLUSTERFS_ZEROFILL case PREALLOC_MODE_FULL: - if (glfs_ftruncate(fd, offset)) { + ret = glfs_ftruncate(fd, offset, NULL, NULL); + if (ret) { error_setg_errno(errp, errno, "Could not resize file"); return -errno; } @@ -1043,7 +1049,8 @@ static int qemu_gluster_do_truncate(struct glfs_fd *fd, int64_t offset, break; #endif /* CONFIG_GLUSTERFS_ZEROFILL */ case PREALLOC_MODE_OFF: - if (glfs_ftruncate(fd, offset)) { + ret = glfs_ftruncate(fd, offset, NULL, NULL); + if (ret) { error_setg_errno(errp, errno, "Could not resize file"); return -errno; } diff --git a/configure b/configure index 540bee19ba..1d09bef1f9 100755 --- a/configure +++ b/configure @@ -456,6 +456,7 @@ glusterfs_xlator_opt="no" glusterfs_discard="no" glusterfs_fallocate="no" glusterfs_zerofill="no" +glusterfs_legacy_ftruncate="no" gtk="" gtk_gl="no" tls_priority="NORMAL" @@ -4057,6 +4058,19 @@ if test "$glusterfs" != "no" ; then glusterfs_fallocate="yes" glusterfs_zerofill="yes" fi + cat > $TMPC << EOF +#include <glusterfs/api/glfs.h> + +int +main(void) +{ + /* new glfs_ftruncate() passes two additional args */ + return glfs_ftruncate(NULL, 0 /*, NULL, NULL */); +} +EOF + if compile_prog "$glusterfs_cflags" "$glusterfs_libs" ; then + glusterfs_legacy_ftruncate="yes" + fi else if test "$glusterfs" = "yes" ; then feature_not_found "GlusterFS backend support" \ @@ -6853,6 +6867,10 @@ if test "$glusterfs_zerofill" = "yes" ; then echo "CONFIG_GLUSTERFS_ZEROFILL=y" >> $config_host_mak fi +if test "$glusterfs_legacy_ftruncate" = "yes" ; then + echo "CONFIG_GLUSTERFS_LEGACY_FTRUNCATE=y" >> $config_host_mak +fi + if test "$libssh2" = "yes" ; then echo "CONFIG_LIBSSH2=m" >> $config_host_mak echo "LIBSSH2_CFLAGS=$libssh2_cflags" >> $config_host_mak -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] block/gluster: Handle changed glfs_ftruncate signature 2019-03-04 16:21 ` [Qemu-devel] [PATCH 1/2] block/gluster: Handle changed glfs_ftruncate signature Niels de Vos @ 2019-03-04 16:41 ` Daniel P. Berrangé 2019-03-05 15:16 ` Niels de Vos 0 siblings, 1 reply; 7+ messages in thread From: Daniel P. Berrangé @ 2019-03-04 16:41 UTC (permalink / raw) To: Niels de Vos Cc: qemu-block, integration, Prasanna Kumar Kalever, qemu-devel, Cole Robinson On Mon, Mar 04, 2019 at 05:21:02PM +0100, Niels de Vos wrote: > From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> > > New versions of Glusters libgfapi.so have an updated glfs_ftruncate() > function that returns additional 'struct stat' structures to enable > advanced caching of attributes. This is useful for file servers, not so > much for QEMU. Nevertheless, the API has changed and needs to be > adopted. > > Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> > Signed-off-by: Niels de Vos <ndevos@redhat.com> > > --- > v4: rebase to current master branch > v3: define old backwards compatible glfs_ftruncate() macro, from Eric Blake > v2: do a compile check as suggested by Eric Blake > --- > block/gluster.c | 11 +++++++++-- > configure | 18 ++++++++++++++++++ > 2 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/block/gluster.c b/block/gluster.c > index af64330211..86e5278524 100644 > --- a/block/gluster.c > +++ b/block/gluster.c > @@ -20,6 +20,10 @@ > #include "qemu/option.h" > #include "qemu/cutils.h" > > +#ifdef CONFIG_GLUSTERFS_LEGACY_FTRUNCATE > +# define glfs_ftruncate(fd, offset, _u1, _u2) glfs_ftruncate(fd, offset) > +#endif I don't much like this approach as it sets up a trapdoor. If future QEMU passes a non-NULL value for the 3rd/4th argument it will be silently ignored depending on glfs version built against which can result in incorrect behaviour at runtime. If we reverse it: #ifndef CONFIG_GLUSTERFS_LEGACY_FTRUNCATE # define glfs_ftruncate(fd, offset) glfs_ftruncate(fd, offset, NULL, NULL) #endif then it ensures we can't silently do the wrong thing in future. Anyone who wants to use the 3rd/4th args will have to make an explicit effort to ensure it works correctly with old glfs APIs. An added benefit is that it avoids the following patch chunks. > + > #define GLUSTER_OPT_FILENAME "filename" > #define GLUSTER_OPT_VOLUME "volume" > #define GLUSTER_OPT_PATH "path" > @@ -1005,6 +1009,7 @@ static int qemu_gluster_do_truncate(struct glfs_fd *fd, int64_t offset, > PreallocMode prealloc, Error **errp) > { > int64_t current_length; > + int ret; > > current_length = glfs_lseek(fd, 0, SEEK_END); > if (current_length < 0) { > @@ -1032,7 +1037,8 @@ static int qemu_gluster_do_truncate(struct glfs_fd *fd, int64_t offset, > #endif /* CONFIG_GLUSTERFS_FALLOCATE */ > #ifdef CONFIG_GLUSTERFS_ZEROFILL > case PREALLOC_MODE_FULL: > - if (glfs_ftruncate(fd, offset)) { > + ret = glfs_ftruncate(fd, offset, NULL, NULL); > + if (ret) { > error_setg_errno(errp, errno, "Could not resize file"); > return -errno; > } > @@ -1043,7 +1049,8 @@ static int qemu_gluster_do_truncate(struct glfs_fd *fd, int64_t offset, > break; > #endif /* CONFIG_GLUSTERFS_ZEROFILL */ > case PREALLOC_MODE_OFF: > - if (glfs_ftruncate(fd, offset)) { > + ret = glfs_ftruncate(fd, offset, NULL, NULL); > + if (ret) { > error_setg_errno(errp, errno, "Could not resize file"); > return -errno; > } > diff --git a/configure b/configure > index 540bee19ba..1d09bef1f9 100755 > --- a/configure > +++ b/configure > @@ -456,6 +456,7 @@ glusterfs_xlator_opt="no" > glusterfs_discard="no" > glusterfs_fallocate="no" > glusterfs_zerofill="no" > +glusterfs_legacy_ftruncate="no" > gtk="" > gtk_gl="no" > tls_priority="NORMAL" > @@ -4057,6 +4058,19 @@ if test "$glusterfs" != "no" ; then > glusterfs_fallocate="yes" > glusterfs_zerofill="yes" > fi > + cat > $TMPC << EOF > +#include <glusterfs/api/glfs.h> > + > +int > +main(void) > +{ > + /* new glfs_ftruncate() passes two additional args */ > + return glfs_ftruncate(NULL, 0 /*, NULL, NULL */); > +} > +EOF > + if compile_prog "$glusterfs_cflags" "$glusterfs_libs" ; then > + glusterfs_legacy_ftruncate="yes" > + fi > else > if test "$glusterfs" = "yes" ; then > feature_not_found "GlusterFS backend support" \ > @@ -6853,6 +6867,10 @@ if test "$glusterfs_zerofill" = "yes" ; then > echo "CONFIG_GLUSTERFS_ZEROFILL=y" >> $config_host_mak > fi > > +if test "$glusterfs_legacy_ftruncate" = "yes" ; then > + echo "CONFIG_GLUSTERFS_LEGACY_FTRUNCATE=y" >> $config_host_mak > +fi > + > if test "$libssh2" = "yes" ; then > echo "CONFIG_LIBSSH2=m" >> $config_host_mak > echo "LIBSSH2_CFLAGS=$libssh2_cflags" >> $config_host_mak > -- > 2.20.1 > > Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] block/gluster: Handle changed glfs_ftruncate signature 2019-03-04 16:41 ` Daniel P. Berrangé @ 2019-03-05 15:16 ` Niels de Vos 0 siblings, 0 replies; 7+ messages in thread From: Niels de Vos @ 2019-03-05 15:16 UTC (permalink / raw) To: Daniel P. Berrangé Cc: qemu-block, integration, Prasanna Kumar Kalever, qemu-devel, Cole Robinson On Mon, Mar 04, 2019 at 04:41:44PM +0000, Daniel P. Berrangé wrote: > On Mon, Mar 04, 2019 at 05:21:02PM +0100, Niels de Vos wrote: > > From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> > > > > New versions of Glusters libgfapi.so have an updated glfs_ftruncate() > > function that returns additional 'struct stat' structures to enable > > advanced caching of attributes. This is useful for file servers, not so > > much for QEMU. Nevertheless, the API has changed and needs to be > > adopted. > > > > Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> > > Signed-off-by: Niels de Vos <ndevos@redhat.com> > > > > --- > > v4: rebase to current master branch > > v3: define old backwards compatible glfs_ftruncate() macro, from Eric Blake > > v2: do a compile check as suggested by Eric Blake > > --- > > block/gluster.c | 11 +++++++++-- > > configure | 18 ++++++++++++++++++ > > 2 files changed, 27 insertions(+), 2 deletions(-) > > > > diff --git a/block/gluster.c b/block/gluster.c > > index af64330211..86e5278524 100644 > > --- a/block/gluster.c > > +++ b/block/gluster.c > > @@ -20,6 +20,10 @@ > > #include "qemu/option.h" > > #include "qemu/cutils.h" > > > > +#ifdef CONFIG_GLUSTERFS_LEGACY_FTRUNCATE > > +# define glfs_ftruncate(fd, offset, _u1, _u2) glfs_ftruncate(fd, offset) > > +#endif > > I don't much like this approach as it sets up a trapdoor. If future > QEMU passes a non-NULL value for the 3rd/4th argument it will be > silently ignored depending on glfs version built against which can > result in incorrect behaviour at runtime. If we reverse it: > > #ifndef CONFIG_GLUSTERFS_LEGACY_FTRUNCATE > # define glfs_ftruncate(fd, offset) glfs_ftruncate(fd, offset, NULL, NULL) > #endif > > then it ensures we can't silently do the wrong thing in future. Anyone > who wants to use the 3rd/4th args will have to make an explicit effort > to ensure it works correctly with old glfs APIs. An added benefit is > that it avoids the following patch chunks. Thanks for the suggestion. All makes sense and I'll update the patch accordingly. Niels ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 2/2] gluster: the glfs_io_cbk callback function pointer adds pre/post stat args 2019-03-04 16:21 [Qemu-devel] [PATCH 0/2] block: Gluster 6 compatibility Niels de Vos 2019-03-04 16:21 ` [Qemu-devel] [PATCH 1/2] block/gluster: Handle changed glfs_ftruncate signature Niels de Vos @ 2019-03-04 16:21 ` Niels de Vos 2019-03-05 10:29 ` [Qemu-devel] [Qemu-block] " Kevin Wolf 1 sibling, 1 reply; 7+ messages in thread From: Niels de Vos @ 2019-03-04 16:21 UTC (permalink / raw) To: qemu-block; +Cc: Cole Robinson, integration, qemu-devel, Niels de Vos The glfs_*_async() functions do a callback once finished. This callback has changed its arguments, pre- and post-stat structures have been added. This makes it possible to improve cashing, which is useful for Samba and NFS-Ganesha, but not so much for QEMU. Gluster 6 is the first release that includes these new arguments. With an additional detection in ./configure, the new arguments can conditionally get included in the glfs_io_cbk handler. Signed-off-by: Niels de Vos <ndevos@redhat.com> --- block/gluster.c | 6 +++++- configure | 24 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/block/gluster.c b/block/gluster.c index 86e5278524..7483c3b2aa 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -729,7 +729,11 @@ static struct glfs *qemu_gluster_init(BlockdevOptionsGluster *gconf, /* * AIO callback routine called from GlusterFS thread. */ -static void gluster_finish_aiocb(struct glfs_fd *fd, ssize_t ret, void *arg) +static void gluster_finish_aiocb(struct glfs_fd *fd, ssize_t ret, +#ifdef CONFIG_GLUSTERFS_IOCB_HAS_STAT + struct glfs_stat *pre, struct glfs_stat *post, +#endif + void *arg) { GlusterAIOCB *acb = (GlusterAIOCB *)arg; diff --git a/configure b/configure index 1d09bef1f9..d8ae8c2f50 100755 --- a/configure +++ b/configure @@ -457,6 +457,7 @@ glusterfs_discard="no" glusterfs_fallocate="no" glusterfs_zerofill="no" glusterfs_legacy_ftruncate="no" +glusterfs_iocb_has_stat="no" gtk="" gtk_gl="no" tls_priority="NORMAL" @@ -4071,6 +4072,25 @@ EOF if compile_prog "$glusterfs_cflags" "$glusterfs_libs" ; then glusterfs_legacy_ftruncate="yes" fi + cat > $TMPC << EOF +#include <glusterfs/api/glfs.h> + +/* new glfs_io_cbk() passes two additional glfs_stat structs */ +static void +glusterfs_iocb(glfs_fd_t *fd, ssize_t ret, struct glfs_stat *prestat, struct glfs_stat *poststat, void *data) +{} + +int +main(void) +{ + glfs_io_cbk iocb = &glusterfs_iocb; + iocb(NULL, 0 , NULL, NULL, NULL); + return 0; +} +EOF + if compile_prog "$glusterfs_cflags" "$glusterfs_libs" ; then + glusterfs_iocb_has_stat="yes" + fi else if test "$glusterfs" = "yes" ; then feature_not_found "GlusterFS backend support" \ @@ -6871,6 +6891,10 @@ if test "$glusterfs_legacy_ftruncate" = "yes" ; then echo "CONFIG_GLUSTERFS_LEGACY_FTRUNCATE=y" >> $config_host_mak fi +if test "$glusterfs_iocb_has_stat" = "yes" ; then + echo "CONFIG_GLUSTERFS_IOCB_HAS_STAT=y" >> $config_host_mak +fi + if test "$libssh2" = "yes" ; then echo "CONFIG_LIBSSH2=m" >> $config_host_mak echo "LIBSSH2_CFLAGS=$libssh2_cflags" >> $config_host_mak -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] gluster: the glfs_io_cbk callback function pointer adds pre/post stat args 2019-03-04 16:21 ` [Qemu-devel] [PATCH 2/2] gluster: the glfs_io_cbk callback function pointer adds pre/post stat args Niels de Vos @ 2019-03-05 10:29 ` Kevin Wolf 2019-03-05 15:18 ` Niels de Vos 0 siblings, 1 reply; 7+ messages in thread From: Kevin Wolf @ 2019-03-05 10:29 UTC (permalink / raw) To: Niels de Vos; +Cc: qemu-block, integration, qemu-devel, Cole Robinson Am 04.03.2019 um 17:21 hat Niels de Vos geschrieben: > The glfs_*_async() functions do a callback once finished. This callback > has changed its arguments, pre- and post-stat structures have been > added. This makes it possible to improve cashing, which is useful for Did you mean "caching"? > Samba and NFS-Ganesha, but not so much for QEMU. Gluster 6 is the first > release that includes these new arguments. > > With an additional detection in ./configure, the new arguments can > conditionally get included in the glfs_io_cbk handler. > > Signed-off-by: Niels de Vos <ndevos@redhat.com> Kevin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] gluster: the glfs_io_cbk callback function pointer adds pre/post stat args 2019-03-05 10:29 ` [Qemu-devel] [Qemu-block] " Kevin Wolf @ 2019-03-05 15:18 ` Niels de Vos 0 siblings, 0 replies; 7+ messages in thread From: Niels de Vos @ 2019-03-05 15:18 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-block, integration, qemu-devel, Cole Robinson On Tue, Mar 05, 2019 at 11:29:34AM +0100, Kevin Wolf wrote: > Am 04.03.2019 um 17:21 hat Niels de Vos geschrieben: > > The glfs_*_async() functions do a callback once finished. This callback > > has changed its arguments, pre- and post-stat structures have been > > added. This makes it possible to improve cashing, which is useful for > > Did you mean "caching"? Yuck, yes, of course. I'll correct that when I send an updated version for patch 1/2. Niels ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-03-05 15:18 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-04 16:21 [Qemu-devel] [PATCH 0/2] block: Gluster 6 compatibility Niels de Vos 2019-03-04 16:21 ` [Qemu-devel] [PATCH 1/2] block/gluster: Handle changed glfs_ftruncate signature Niels de Vos 2019-03-04 16:41 ` Daniel P. Berrangé 2019-03-05 15:16 ` Niels de Vos 2019-03-04 16:21 ` [Qemu-devel] [PATCH 2/2] gluster: the glfs_io_cbk callback function pointer adds pre/post stat args Niels de Vos 2019-03-05 10:29 ` [Qemu-devel] [Qemu-block] " Kevin Wolf 2019-03-05 15:18 ` Niels de Vos
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.