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
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 :|
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
© 2016 - 2025 Red Hat, Inc.