[libvirt] [PATCH] util: recognize SMB filesystems as shared

This should resolve: https://bugzilla.redhat.com/show_bug.cgi?id=1012085 libvirt previously recognized NFS, GFS2, OCFS2, and AFS filesystems as "shared", and thus eligible for exceptions to certain rules/actions about chowning image files before handing them off to a guest. This patch widens the definition of "shared filesystem" to include the SMB filesystem (sometimes called CIFS, or "Windows file sharing"). --- src/util/virstoragefile.c | 9 ++++++++- src/util/virstoragefile.h | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 0b9cec3..ed43b2b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1241,6 +1241,9 @@ cleanup: # ifndef AFS_FS_MAGIC # define AFS_FS_MAGIC 0x6B414653 # endif +# ifndef SMB_SUPER_MAGIC +# define SMB_SUPER_MAGIC 0x517B +# endif int virStorageFileIsSharedFSType(const char *path, @@ -1304,6 +1307,9 @@ int virStorageFileIsSharedFSType(const char *path, if ((fstypes & VIR_STORAGE_FILE_SHFS_AFS) && (sb.f_type == AFS_FS_MAGIC)) return 1; + if ((fstypes & VIR_STORAGE_FILE_SHFS_SMB) && + (sb.f_type == SMB_SUPER_MAGIC)) + return 1; return 0; } @@ -1322,7 +1328,8 @@ int virStorageFileIsSharedFS(const char *path) VIR_STORAGE_FILE_SHFS_NFS | VIR_STORAGE_FILE_SHFS_GFS2 | VIR_STORAGE_FILE_SHFS_OCFS | - VIR_STORAGE_FILE_SHFS_AFS); + VIR_STORAGE_FILE_SHFS_AFS | + VIR_STORAGE_FILE_SHFS_SMB); } int virStorageFileIsClusterFS(const char *path) diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 1f89839..9c07212 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -112,6 +112,7 @@ enum { VIR_STORAGE_FILE_SHFS_GFS2 = (1 << 1), VIR_STORAGE_FILE_SHFS_OCFS = (1 << 2), VIR_STORAGE_FILE_SHFS_AFS = (1 << 3), + VIR_STORAGE_FILE_SHFS_SMB = (1 << 4), }; int virStorageFileIsSharedFS(const char *path); -- 1.7.11.7

On 09/26/2013 03:43 AM, Laine Stump wrote:
This should resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1012085
libvirt previously recognized NFS, GFS2, OCFS2, and AFS filesystems as "shared", and thus eligible for exceptions to certain rules/actions about chowning image files before handing them off to a guest. This patch widens the definition of "shared filesystem" to include the SMB filesystem (sometimes called CIFS, or "Windows file sharing"). --- src/util/virstoragefile.c | 9 ++++++++- src/util/virstoragefile.h | 1 + 2 files changed, 9 insertions(+), 1 deletion(-)
Coreutils includes a rather extensive list of file systems (alas, it's GPLv3+ code, so we can't use it verbatim without asking Jim Meyering and other coreutils folks to relax the license): http://git.sv.gnu.org/cgit/coreutils.git/tree/src/stat.c#n243
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 0b9cec3..ed43b2b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1241,6 +1241,9 @@ cleanup: # ifndef AFS_FS_MAGIC # define AFS_FS_MAGIC 0x6B414653 # endif +# ifndef SMB_SUPER_MAGIC +# define SMB_SUPER_MAGIC 0x517B +# endif
This is correct; but since samba and cifs are highly inter-related (same protocol, just which driver is implementing it), you also need a define for CIFS_SUPER_MAGIC 0xff534d42. Looking forward to v2. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

[actually adding coreutils] On 09/26/2013 08:28 AM, Eric Blake wrote:
On 09/26/2013 03:43 AM, Laine Stump wrote:
This should resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1012085
libvirt previously recognized NFS, GFS2, OCFS2, and AFS filesystems as "shared", and thus eligible for exceptions to certain rules/actions about chowning image files before handing them off to a guest. This patch widens the definition of "shared filesystem" to include the SMB filesystem (sometimes called CIFS, or "Windows file sharing"). --- src/util/virstoragefile.c | 9 ++++++++- src/util/virstoragefile.h | 1 + 2 files changed, 9 insertions(+), 1 deletion(-)
Coreutils includes a rather extensive list of file systems (alas, it's GPLv3+ code, so we can't use it verbatim without asking Jim Meyering and other coreutils folks to relax the license): http://git.sv.gnu.org/cgit/coreutils.git/tree/src/stat.c#n243
Would it be worth moving the list of known file systems, and knowledge of whether they are remote (shared) or local, out of coreutils and into a gnulib module, for reuse by other projects? Libvirt (LGPLv2+) has to make decisions based on what file system is hosting a guest image (guest migration behaves differently depending on whether the guest disk image resides on a local or a shared file system). Licensing may be a sticking point - coreutils' list is currently GPLv3+, but is derived from the kernel (GPLv2), and libvirt cannot reuse it unless it is further relaxed to LGPLv2+. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 09/26/2013 03:33 PM, Eric Blake wrote:
[actually adding coreutils]
On 09/26/2013 08:28 AM, Eric Blake wrote:
On 09/26/2013 03:43 AM, Laine Stump wrote:
This should resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1012085
libvirt previously recognized NFS, GFS2, OCFS2, and AFS filesystems as "shared", and thus eligible for exceptions to certain rules/actions about chowning image files before handing them off to a guest. This patch widens the definition of "shared filesystem" to include the SMB filesystem (sometimes called CIFS, or "Windows file sharing"). --- src/util/virstoragefile.c | 9 ++++++++- src/util/virstoragefile.h | 1 + 2 files changed, 9 insertions(+), 1 deletion(-)
Coreutils includes a rather extensive list of file systems (alas, it's GPLv3+ code, so we can't use it verbatim without asking Jim Meyering and other coreutils folks to relax the license): http://git.sv.gnu.org/cgit/coreutils.git/tree/src/stat.c#n243
Would it be worth moving the list of known file systems, and knowledge of whether they are remote (shared) or local, out of coreutils and into a gnulib module, for reuse by other projects? Libvirt (LGPLv2+) has to make decisions based on what file system is hosting a guest image (guest migration behaves differently depending on whether the guest disk image resides on a local or a shared file system). Licensing may be a sticking point - coreutils' list is currently GPLv3+, but is derived from the kernel (GPLv2), and libvirt cannot reuse it unless it is further relaxed to LGPLv2+.
I'd be happy moving that list to an LGPLv2+ gnulib module For reference, from my coreutils build here: $ grep "return 0" src/fs-is-local.h case S_MAGIC_AFS: return 0; case S_MAGIC_AUFS: return 0; case S_MAGIC_CEPH: return 0; case S_MAGIC_CIFS: return 0; case S_MAGIC_CODA: return 0; case S_MAGIC_FHGFS: return 0; case S_MAGIC_FUSEBLK: return 0; case S_MAGIC_FUSECTL: return 0; case S_MAGIC_GFS: return 0; case S_MAGIC_GPFS: return 0; case S_MAGIC_KAFS: return 0; case S_MAGIC_LUSTRE: return 0; case S_MAGIC_NCP: return 0; case S_MAGIC_NFS: return 0; case S_MAGIC_NFSD: return 0; case S_MAGIC_OCFS2: return 0; case S_MAGIC_PANFS: return 0; case S_MAGIC_PIPEFS: return 0; case S_MAGIC_SMB: return 0; case S_MAGIC_SNFS: return 0; case S_MAGIC_VMHGFS: return 0; I would be better of course if there was some introspection provided by the kernel. Also we use the above to determine if "remote" or not. Whether those remote file ssytems are used in a shared context is another question I suppose. cheers, Pádraig.

On Thu, Sep 26, 2013 at 7:33 AM, Eric Blake <eblake@redhat.com> wrote:
[actually adding coreutils]
On 09/26/2013 08:28 AM, Eric Blake wrote:
On 09/26/2013 03:43 AM, Laine Stump wrote:
This should resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1012085
libvirt previously recognized NFS, GFS2, OCFS2, and AFS filesystems as "shared", and thus eligible for exceptions to certain rules/actions about chowning image files before handing them off to a guest. This patch widens the definition of "shared filesystem" to include the SMB filesystem (sometimes called CIFS, or "Windows file sharing"). --- src/util/virstoragefile.c | 9 ++++++++- src/util/virstoragefile.h | 1 + 2 files changed, 9 insertions(+), 1 deletion(-)
Coreutils includes a rather extensive list of file systems (alas, it's GPLv3+ code, so we can't use it verbatim without asking Jim Meyering and other coreutils folks to relax the license): http://git.sv.gnu.org/cgit/coreutils.git/tree/src/stat.c#n243
Would it be worth moving the list of known file systems, and knowledge of whether they are remote (shared) or local, out of coreutils and into a gnulib module, for reuse by other projects? Libvirt (LGPLv2+) has to make decisions based on what file system is hosting a guest image (guest migration behaves differently depending on whether the guest disk image resides on a local or a shared file system). Licensing may be a sticking point - coreutils' list is currently GPLv3+, but is derived from the kernel (GPLv2), and libvirt cannot reuse it unless it is further relaxed to LGPLv2+.
Hi Eric, Moving it to gnulib sounds sensible, and I'm ok with the switch to LGPLv2+. It is currently v3 merely by virtue of residing in a .c file that calls exit. If you do this, please consider addressing this comment: case S_MAGIC_AUFS: /* 0x61756673 remote */ /* FIXME: change syntax or add an optional attribute like "inotify:no". The above is labeled as "remote" so that tail always uses polling, but this isn't really a remote file system type. */ return "aufs"; It is referring to the fact that while initially "local" and "remote" were sufficient, for some file system types, the lines have blurred, making those terms misnomers. The intended boolean is something like "has_inotify_support". Also, coreutils has makefile rules that try to add new types to the list -- typically we'd run those manually, just prior to a release. I'd like to preserve that functionality one way or another. Thanks, Jim
participants (4)
-
Eric Blake
-
Jim Meyering
-
Laine Stump
-
Pádraig Brady