[libvirt] [PATCH] Don't reset user/group/security label on shared filesystems during migrate

[Daniel Veillard]

On Thu, May 13, 2010 at 11:52:47AM -0400, Daniel P. Berrange wrote:
> When QEMU runs with its disk on NFS, and as a non-root user, the
> disk is chownd to that non-root user. When migration completes
> the last step is shutting down the QEMU on the source host. THis
> normally resets user/group/security label. This is bad when the
> VM was just migrated because the file is still in use on the dest
> host. It is thus neccessary to skip the reset step for any files
> found to be on a shared filesystem
> 
> * src/libvirt_private.syms: Export virStorageFileIsSharedFS
> * src/util/storage_file.c, src/util/storage_file.h: Add a new
>   method virStorageFileIsSharedFS() to determine if a file is
>   on a shared filesystem (NFS, GFS, OCFS2, etc)
> * src/qemu/qemu_driver.c: Tell security driver not to reset
>   disk labels on migration completion
> * src/qemu/qemu_security_dac.c, src/qemu/qemu_security_stacked.c,
>   src/security/security_selinux.c, src/security/security_driver.h,
>   src/security/security_apparmor.c: Add ability to skip disk
>   restore step for files on shared filesystems.

 Patch looks fine to me overall

> +
> +
> +#ifdef __linux__
> +
> +#ifndef OCFS2_SUPER_MAGIC
> +#define OCFS2_SUPER_MAGIC 0x7461636f
> +#endif
> +#ifndef GFS2_MAGIC
> +#define GFS2_MAGIC 0x01161970
> +#endif
> +#ifndef AFS_FS_MAGIC
> +#define AFS_FS_MAGIC 0x6B414653
> +#endif

  hum, cppi is gonna complain on make syntax-check there

> +
> +int virStorageFileIsSharedFS(const char *path)
> +{
> +    struct statfs sb;
> +
> +    if (statfs(path, &sb) < 0) {
> +        virReportSystemError(errno,
> +                             _("cannot determine filesystem for '%s'"),
> +                             path);
> +        return -1;
> +    }
> +
> +    VIR_DEBUG("Check if path %s with FS magic %lld is shared",
> +              path, (long long int)sb.f_type);
> +
> +    if (sb.f_type == NFS_SUPER_MAGIC ||
> +        sb.f_type == GFS2_MAGIC ||
> +        sb.f_type == OCFS2_SUPER_MAGIC ||
> +        sb.f_type == AFS_FS_MAGIC) {
> +        return 1;
> +    }
> +
> +    return 0;
> +}
> +#else
> +int virStorageFileIsSharedFS(const char *path ATTRIBUTE_UNUSED)
> +{
> +    /* XXX implement me :-) */
> +    return 0;
> +}
> +#endif

I wonder if we shouldn't try to unify with the existing NFS
lookup done in qemu_driver.c where we have this kind of NFS_SUPER_MAGIC
It would be good to have all those filesystem specific checks cleanly
exported from util
Like also isolating the routine to find the fstype of a file/directory
currently in the middle of qemudDomainSaveFlag()

  But the cleanup is not urgent, ACK once the cppi is fixed,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/