1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] virfile: added GPFS as shared fs

Diego Michelotto posted 1 patch 5 years, 1 month ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190225181903.24442-1-diego.michelotto@cnaf.infn.it
src/util/virfile.c            | 9 ++++++++-
src/util/virfile.h            | 1 +
tests/virfiledata/mounts3.txt | 1 +
tests/virfilemock.c           | 5 +++++
tests/virfiletest.c           | 1 +
5 files changed, 16 insertions(+), 1 deletion(-)
[libvirt] [PATCH] virfile: added GPFS as shared fs
Posted by Diego Michelotto 5 years, 1 month ago 
From: dmichelotto <diego.michelotto@cnaf.infn.it>

Added GPFS as shared file system recognized during live migration
security checks.

GPFS is 'IBM General Parallel File System' also called
'IBM Spectrum Scale'

BUG: https://bugzilla.redhat.com/show_bug.cgi?id=1679528

Signed-off-by: Diego Michelotto <diego.michelotto@cnaf.infn.it>
---
 src/util/virfile.c            | 9 ++++++++-
 src/util/virfile.h            | 1 +
 tests/virfiledata/mounts3.txt | 1 +
 tests/virfilemock.c           | 5 +++++
 tests/virfiletest.c           | 1 +
 5 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 271bf5e796..615c287104 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3468,6 +3468,9 @@ int virFilePrintf(FILE *fp, const char *msg, ...)
 # ifndef CEPH_SUPER_MAGIC
 #  define CEPH_SUPER_MAGIC 0x00C36400
 # endif
+# ifndef GPFS_SUPER_MAGIC
+#  define GPFS_SUPER_MAGIC 0x47504653
+# endif
 
 # define PROC_MOUNTS "/proc/mounts"
 
@@ -3613,6 +3616,9 @@ virFileIsSharedFSType(const char *path,
     if ((fstypes & VIR_FILE_SHFS_CEPH) &&
         (f_type == CEPH_SUPER_MAGIC))
         return 1;
+    if ((fstypes & VIR_FILE_SHFS_GPFS) &&
+        (f_type == GPFS_SUPER_MAGIC))
+        return 1;
 
     return 0;
 }
@@ -3776,7 +3782,8 @@ int virFileIsSharedFS(const char *path)
                                  VIR_FILE_SHFS_AFS |
                                  VIR_FILE_SHFS_SMB |
                                  VIR_FILE_SHFS_CIFS |
-                                 VIR_FILE_SHFS_CEPH);
+                                 VIR_FILE_SHFS_CEPH |
+                                 VIR_FILE_SHFS_GPFS);
 }
 
 
diff --git a/src/util/virfile.h b/src/util/virfile.h
index be612af770..0b946fe1ca 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -221,6 +221,7 @@ enum {
     VIR_FILE_SHFS_SMB = (1 << 4),
     VIR_FILE_SHFS_CIFS = (1 << 5),
     VIR_FILE_SHFS_CEPH = (1 << 6),
+    VIR_FILE_SHFS_GPFS = (1 << 7),
 };
 
 int virFileIsSharedFSType(const char *path, int fstypes) ATTRIBUTE_NONNULL(1);
diff --git a/tests/virfiledata/mounts3.txt b/tests/virfiledata/mounts3.txt
index 68eded048c..4377e5d471 100644
--- a/tests/virfiledata/mounts3.txt
+++ b/tests/virfiledata/mounts3.txt
@@ -35,3 +35,4 @@ host:/gv0 /gluster fuse.glusterfs rw 0 0
 root@host:/tmp/mkdir /gluster/sshfs fuse.sshfs rw 0 0
 192.168.0.1:/ceph/data /ceph ceph rw,noatime,name=cephfs,secret=<hidden>,acl,wsize=16777216 0 0
 192.168.0.1,192.168.0.2,192.168.0.3:/ceph/data2 /ceph/multi ceph rw,noatime,name=cephfs,secret=<hidden>,acl,wsize=16777216 0 0
+gpfs_data /gpfs/data gpfs rw,relatime 0 0
diff --git a/tests/virfilemock.c b/tests/virfilemock.c
index 499135d773..89e14c5b67 100644
--- a/tests/virfilemock.c
+++ b/tests/virfilemock.c
@@ -89,6 +89,9 @@ setmntent(const char *filename, const char *type)
 #ifndef CEPH_SUPER_MAGIC
 # define CEPH_SUPER_MAGIC 0x00c36400
 #endif
+#ifndef GPFS_SUPER_MAGIC
+# define GPFS_SUPER_MAGIC 0x47504653
+#endif
 
 
 static int
@@ -137,6 +140,8 @@ statfs_mock(const char *mtab,
             ftype = FUSE_SUPER_MAGIC;
         } else if (STRPREFIX(mb.mnt_type, "ceph")) {
             ftype = CEPH_SUPER_MAGIC;
+        } else if (STRPREFIX(mb.mnt_type, "gpfs")) {
+            ftype = GPFS_SUPER_MAGIC;
         } else {
             /* Everything else is EXT4. We don't care really for other paths. */
             ftype = EXT4_SUPER_MAGIC;
diff --git a/tests/virfiletest.c b/tests/virfiletest.c
index f90c611ac4..e2bd4953ed 100644
--- a/tests/virfiletest.c
+++ b/tests/virfiletest.c
@@ -457,6 +457,7 @@ mymain(void)
     DO_TEST_FILE_IS_SHARED_FS_TYPE("mounts3.txt", "/some/symlink/file", true);
     DO_TEST_FILE_IS_SHARED_FS_TYPE("mounts3.txt", "/ceph/file", true);
     DO_TEST_FILE_IS_SHARED_FS_TYPE("mounts3.txt", "/ceph/multi/file", true);
+    DO_TEST_FILE_IS_SHARED_FS_TYPE("mounts3.txt", "/gpfs/data", true);
 
     return ret != 0 ? EXIT_FAILURE : EXIT_SUCCESS;
 }
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virfile: added GPFS as shared fs
Posted by Peter Krempa 5 years, 1 month ago 
On Mon, Feb 25, 2019 at 19:19:03 +0100, Diego Michelotto wrote:
> From: dmichelotto <diego.michelotto@cnaf.infn.it>

With your permission I'll remove the above line when applying the patch
so that your authorship of the patch shows up as:

Author: Diego Michelotto <diego.michelotto@cnaf.infn.it>

instead of

Author: dmichelotto <diego.michelotto@cnaf.infn.it>

> 
> Added GPFS as shared file system recognized during live migration
> security checks.
> 
> GPFS is 'IBM General Parallel File System' also called
> 'IBM Spectrum Scale'
> 
> BUG: https://bugzilla.redhat.com/show_bug.cgi?id=1679528
> 
> Signed-off-by: Diego Michelotto <diego.michelotto@cnaf.infn.it>
> ---
>  src/util/virfile.c            | 9 ++++++++-
>  src/util/virfile.h            | 1 +
>  tests/virfiledata/mounts3.txt | 1 +
>  tests/virfilemock.c           | 5 +++++
>  tests/virfiletest.c           | 1 +
>  5 files changed, 16 insertions(+), 1 deletion(-)

One thing which might be worth exploring is whether GPFS is also
migratable with cache != none and thus should be also added to
virStorageFileIsClusterFS.

ACK.

Depending on the angle of view this can be viewed both as a bug and as a
feature. Since we are feature freeze, is anybody objecting in pushing
this right away? (obviously I'm not going to push it right now)


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virfile: added GPFS as shared fs
Posted by Diego Michelotto 5 years, 1 month ago 
HI Peter

> Il giorno 26 feb 2019, alle ore 09:08, Peter Krempa <pkrempa@redhat.com> ha scritto:
> 
> On Mon, Feb 25, 2019 at 19:19:03 +0100, Diego Michelotto wrote:
>> From: dmichelotto <diego.michelotto@cnaf.infn.it <mailto:diego.michelotto@cnaf.infn.it>>
> 
> With your permission I'll remove the above line when applying the patch
> so that your authorship of the patch shows up as:
> 
> Author: Diego Michelotto <diego.michelotto@cnaf.infn.it <mailto:diego.michelotto@cnaf.infn.it>>
> 
> instead of
> 
> Author: dmichelotto <diego.michelotto@cnaf.infn.it <mailto:diego.michelotto@cnaf.infn.it>>

Sure, you can change it.

> 
>> 
>> Added GPFS as shared file system recognized during live migration
>> security checks.
>> 
>> GPFS is 'IBM General Parallel File System' also called
>> 'IBM Spectrum Scale'
>> 
>> BUG: https://bugzilla.redhat.com/show_bug.cgi?id=1679528
>> 
>> Signed-off-by: Diego Michelotto <diego.michelotto@cnaf.infn.it>
>> ---
>> src/util/virfile.c            | 9 ++++++++-
>> src/util/virfile.h            | 1 +
>> tests/virfiledata/mounts3.txt | 1 +
>> tests/virfilemock.c           | 5 +++++
>> tests/virfiletest.c           | 1 +
>> 5 files changed, 16 insertions(+), 1 deletion(-)
> 
> One thing which might be worth exploring is whether GPFS is also
> migratable with cache != none and thus should be also added to
> virStorageFileIsClusterFS.

GPFS stalls badly when a process issues a lot of fsyncs in a short timeframe.
It is better if the VM I/O is not synchronous, with previous version of libvirt
we are using writeback caching.

If you prefer I’ll try to make a new patch, but let me know if it’s easier for you to add yourself GPFS to 
the virStorageFileIsClusterFS function.

> 
> ACK.
> 
> Depending on the angle of view this can be viewed both as a bug and as a
> feature. Since we are feature freeze, is anybody objecting in pushing
> this right away? (obviously I'm not going to push it right now)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virfile: added GPFS as shared fs
Posted by Peter Krempa 5 years, 1 month ago 
On Tue, Feb 26, 2019 at 09:34:21 +0100, Diego Michelotto wrote:
> HI Peter
> 
> > Il giorno 26 feb 2019, alle ore 09:08, Peter Krempa <pkrempa@redhat.com> ha scritto:
> > 
> > On Mon, Feb 25, 2019 at 19:19:03 +0100, Diego Michelotto wrote:
> >> From: dmichelotto <diego.michelotto@cnaf.infn.it <mailto:diego.michelotto@cnaf.infn.it>>
> > 
> > With your permission I'll remove the above line when applying the patch
> > so that your authorship of the patch shows up as:
> > 
> > Author: Diego Michelotto <diego.michelotto@cnaf.infn.it <mailto:diego.michelotto@cnaf.infn.it>>
> > 
> > instead of
> > 
> > Author: dmichelotto <diego.michelotto@cnaf.infn.it <mailto:diego.michelotto@cnaf.infn.it>>
> 
> Sure, you can change it.
> 
> > 
> >> 
> >> Added GPFS as shared file system recognized during live migration
> >> security checks.
> >> 
> >> GPFS is 'IBM General Parallel File System' also called
> >> 'IBM Spectrum Scale'
> >> 
> >> BUG: https://bugzilla.redhat.com/show_bug.cgi?id=1679528
> >> 
> >> Signed-off-by: Diego Michelotto <diego.michelotto@cnaf.infn.it>
> >> ---
> >> src/util/virfile.c            | 9 ++++++++-
> >> src/util/virfile.h            | 1 +
> >> tests/virfiledata/mounts3.txt | 1 +
> >> tests/virfilemock.c           | 5 +++++
> >> tests/virfiletest.c           | 1 +
> >> 5 files changed, 16 insertions(+), 1 deletion(-)
> > 
> > One thing which might be worth exploring is whether GPFS is also
> > migratable with cache != none and thus should be also added to
> > virStorageFileIsClusterFS.
> 
> GPFS stalls badly when a process issues a lot of fsyncs in a short timeframe.
> It is better if the VM I/O is not synchronous, with previous version of libvirt
> we are using writeback caching.
> 
> If you prefer I’ll try to make a new patch, but let me know if it’s easier for you to add yourself GPFS to 
> the virStorageFileIsClusterFS function.

It will be better to send this as a separate patch. The safety of
migration with caching is a complex problem which is harder to prove
that it's correct. That one will require a justification of it's own.

I'll push this meanwhile.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virfile: added GPFS as shared fs
Posted by Ján Tomko 5 years, 1 month ago 
On Tue, Feb 26, 2019 at 09:08:27AM +0100, Peter Krempa wrote:
>On Mon, Feb 25, 2019 at 19:19:03 +0100, Diego Michelotto wrote:
>> From: dmichelotto <diego.michelotto@cnaf.infn.it>
>
>With your permission I'll remove the above line when applying the patch
>so that your authorship of the patch shows up as:
>
>Author: Diego Michelotto <diego.michelotto@cnaf.infn.it>
>
>instead of
>
>Author: dmichelotto <diego.michelotto@cnaf.infn.it>
>
>>
>> Added GPFS as shared file system recognized during live migration
>> security checks.
>>
>> GPFS is 'IBM General Parallel File System' also called
>> 'IBM Spectrum Scale'
>>
>> BUG: https://bugzilla.redhat.com/show_bug.cgi?id=1679528
>>
>> Signed-off-by: Diego Michelotto <diego.michelotto@cnaf.infn.it>
>> ---
>>  src/util/virfile.c            | 9 ++++++++-
>>  src/util/virfile.h            | 1 +
>>  tests/virfiledata/mounts3.txt | 1 +
>>  tests/virfilemock.c           | 5 +++++
>>  tests/virfiletest.c           | 1 +
>>  5 files changed, 16 insertions(+), 1 deletion(-)
>
>One thing which might be worth exploring is whether GPFS is also
>migratable with cache != none and thus should be also added to
>virStorageFileIsClusterFS.
>
>ACK.
>
>Depending on the angle of view this can be viewed both as a bug and as a
>feature.

Bug, since we consider this a blocker for migration.

Also, probably a regression introduced by commit ed11e9cd95bd9cae6cdfab14ae0936930bbb63e6
    qemuMigrationSrcIsSafe: Check local storage more thoroughly

>Since we are feature freeze, is anybody objecting in pushing
>this right away? (obviously I'm not going to push it right now)
>

I object to calling this a feature. Feel free to postpone if you deem it
too risky or invasive.

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virfile: added GPFS as shared fs
Posted by Daniel P. Berrangé 5 years, 1 month ago 
On Tue, Feb 26, 2019 at 09:08:27AM +0100, Peter Krempa wrote:
> On Mon, Feb 25, 2019 at 19:19:03 +0100, Diego Michelotto wrote:
> > From: dmichelotto <diego.michelotto@cnaf.infn.it>
> 
> With your permission I'll remove the above line when applying the patch
> so that your authorship of the patch shows up as:
> 
> Author: Diego Michelotto <diego.michelotto@cnaf.infn.it>
> 
> instead of
> 
> Author: dmichelotto <diego.michelotto@cnaf.infn.it>
> 
> > 
> > Added GPFS as shared file system recognized during live migration
> > security checks.
> > 
> > GPFS is 'IBM General Parallel File System' also called
> > 'IBM Spectrum Scale'
> > 
> > BUG: https://bugzilla.redhat.com/show_bug.cgi?id=1679528
> > 
> > Signed-off-by: Diego Michelotto <diego.michelotto@cnaf.infn.it>
> > ---
> >  src/util/virfile.c            | 9 ++++++++-
> >  src/util/virfile.h            | 1 +
> >  tests/virfiledata/mounts3.txt | 1 +
> >  tests/virfilemock.c           | 5 +++++
> >  tests/virfiletest.c           | 1 +
> >  5 files changed, 16 insertions(+), 1 deletion(-)
> 
> One thing which might be worth exploring is whether GPFS is also
> migratable with cache != none and thus should be also added to
> virStorageFileIsClusterFS.
> 
> ACK.
> 
> Depending on the angle of view this can be viewed both as a bug and as a
> feature. Since we are feature freeze, is anybody objecting in pushing
> this right away? (obviously I'm not going to push it right now)

I can see it both ways, but regardless I think this is safe enough to
push during freeze.


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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.