[Qemu-devel] [PATCH v2] block/gluster: defend on legacy ftruncate api use

Niels de Vos posted 1 patch 5 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180726091907.26839-1-ndevos@redhat.com
Test docker-mingw@fedora passed
Test checkpatch passed
Test docker-quick@centos7 passed
Test docker-clang@ubuntu passed
block/gluster.c | 15 +++++++++++++--
configure       | 18 ++++++++++++++++++
2 files changed, 31 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH v2] block/gluster: defend on legacy ftruncate api use
Posted by Niels de Vos 5 years, 9 months ago
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. Never the less, 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>

--
v2: do a compile check as suggested by Eric Blake
---
 block/gluster.c | 15 +++++++++++++--
 configure       | 18 ++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index 4fd55a9cc5..d1c6f81f5c 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -997,6 +997,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) {
@@ -1024,7 +1025,12 @@ 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)) {
+#ifdef CONFIG_GLUSTERFS_LEGACY_FTRUNCATE
+        ret = glfs_ftruncate(fd, offset);
+#else
+        ret = glfs_ftruncate(fd, offset, NULL, NULL);
+#endif
+        if (ret) {
             error_setg_errno(errp, errno, "Could not resize file");
             return -errno;
         }
@@ -1035,7 +1041,12 @@ 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)) {
+#ifdef CONFIG_GLUSTERFS_LEGACY_FTRUNCATE
+        ret = glfs_ftruncate(fd, offset);
+#else
+        ret = glfs_ftruncate(fd, offset, NULL, NULL);
+#endif
+        if (ret) {
             error_setg_errno(errp, errno, "Could not resize file");
             return -errno;
         }
diff --git a/configure b/configure
index 2a7796ea80..f3c0918d6b 100755
--- a/configure
+++ b/configure
@@ -451,6 +451,7 @@ glusterfs_xlator_opt="no"
 glusterfs_discard="no"
 glusterfs_fallocate="no"
 glusterfs_zerofill="no"
+glusterfs_legacy_ftruncate="no"
 gtk=""
 gtkabi=""
 gtk_gl="no"
@@ -3947,6 +3948,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" \
@@ -6644,6 +6658,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.17.1


Re: [Qemu-devel] [PATCH v2] block/gluster: defend on legacy ftruncate api use
Posted by Eric Blake 5 years, 9 months ago
On 07/26/2018 04:19 AM, Niels de Vos wrote:
> From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

When sending a v2, it's best to start a new thread instead of 
in-reply-to v1, so that the automated tooling spots it easier.

Subject line is awkward, may I suggest:

block/gluster: Handle changed glfs_ftruncate signature

> 
> 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. Never the less, the API has changed and needs to be

s/Never the less/Nevertheless/

> adopted.
> 
> Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
> Signed-off-by: Niels de Vos <ndevos@redhat.com>
> 
> --
> v2: do a compile check as suggested by Eric Blake
> ---
>   block/gluster.c | 15 +++++++++++++--
>   configure       | 18 ++++++++++++++++++
>   2 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/block/gluster.c b/block/gluster.c
> index 4fd55a9cc5..d1c6f81f5c 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -997,6 +997,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) {
> @@ -1024,7 +1025,12 @@ 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)) {
> +#ifdef CONFIG_GLUSTERFS_LEGACY_FTRUNCATE
> +        ret = glfs_ftruncate(fd, offset);
> +#else
> +        ret = glfs_ftruncate(fd, offset, NULL, NULL);
> +#endif

I'm personally a fan of minimizing in-function #ifdef, where it is easy. 
This could be done by a top-level:

#ifdef CONFIG_GLUSTERFS_LEGACY_FTRUNCATE
# define glfs_ftruncate(fd, offs, _1, _2) glfs_ftruncate(fd, offs)
#endif

then just using glfs_ftruncate(fd, offs, NULL, NULL) unconditionally in 
the rest of the file.

At any rate, the configure test looks a lot better than the v1 patch.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org