[libvirt] [PATCH] Extra filesystem options during backend filesystem mount.

New option index added to support -o options for various netfs. Currently added an option for glusterfs. --- src/storage_backend_fs.c | 32 ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-) diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index c3d66b5..606fb47 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -604,6 +604,7 @@ static int virStorageBackendFileSystemMount(virConnectPtr conn, virStoragePoolObjPtr pool) { char *src; + char *options; const char **mntargv; /* 'mount -t auto' doesn't seem to auto determine nfs (or cifs), @@ -611,6 +612,10 @@ virStorageBackendFileSystemMount(virConnectPtr conn, * accommodate this */ int netauto = (pool->def->type == VIR_STORAGE_POOL_NETFS && pool->def->source.format == VIR_STORAGE_POOL_NETFS_AUTO); + int glusterfs = (pool->def->type == VIR_STORAGE_POOL_NETFS && + pool->def->source.format == VIR_STORAGE_POOL_NETFS_GLUSTERFS); + + int option_index; int source_index; const char *netfs_auto_argv[] = { @@ -632,9 +637,26 @@ virStorageBackendFileSystemMount(virConnectPtr conn, NULL, }; + const char *glusterfs_argv[] = { + MOUNT, + "-t", + pool->def->type == VIR_STORAGE_POOL_FS ? + virStoragePoolFormatFileSystemTypeToString(pool->def->source.format) : + virStoragePoolFormatFileSystemNetTypeToString(pool->def->source.format), + NULL, + "-o", + NULL, + pool->def->target.path, + NULL, + }; + if (netauto) { mntargv = netfs_auto_argv; source_index = 1; + } else if (glusterfs) { + mntargv = glusterfs_argv; + source_index = 3; + option_index = 5; } else { mntargv = fs_argv; source_index = 3; @@ -670,6 +692,12 @@ virStorageBackendFileSystemMount(virConnectPtr conn, } if (pool->def->type == VIR_STORAGE_POOL_NETFS) { + if (pool->def->source.format = VIR_STORAGE_POOL_NETFS_GLUSTERFS) { + if (virAsprintf(&options, "direct-io-mode=1") == -1) { + virReportOOMError(conn); + return -1; + } + } if (virAsprintf(&src, "%s:%s", pool->def->source.host.name, pool->def->source.dir) == -1) { @@ -685,6 +713,10 @@ virStorageBackendFileSystemMount(virConnectPtr conn, } mntargv[source_index] = src; + if (glusterfs) { + mntargv[option_index] = options; + } + if (virRun(conn, mntargv, NULL) < 0) { VIR_FREE(src); return -1; -- 1.6.0.6

On Thu, Jul 16, 2009 at 06:06:25AM -0700, Harshavardhana wrote:
New option index added to support -o options for various netfs. Currently added an option for glusterfs.
What effect does it have ? Or why do we want/need it
@@ -670,6 +692,12 @@ virStorageBackendFileSystemMount(virConnectPtr conn, }
if (pool->def->type == VIR_STORAGE_POOL_NETFS) { + if (pool->def->source.format = VIR_STORAGE_POOL_NETFS_GLUSTERFS) { + if (virAsprintf(&options, "direct-io-mode=1") == -1) { + virReportOOMError(conn); + return -1; + } + }
Normally direct-IO mode is something you'd let apps set on a per file basis as they desire. QEMU / Xen both know todo this, so I'm not convinced we should force it on for the whole mount point. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Hi Daniel, On Thu, Jul 16, 2009 at 7:46 PM, Daniel P. Berrange <berrange@redhat.com>wrote:
On Thu, Jul 16, 2009 at 06:06:25AM -0700, Harshavardhana wrote:
New option index added to support -o options for various netfs. Currently added an option for glusterfs.
What effect does it have ? Or why do we want/need it
Options could be required for filesystem to have few enhaced handling at the site where they will be under use. Correct approach for a configurable will be a new "XML" option in this case. Regarding current patch: This is required for the glusterfs to work properly with VM's. Right now there is a problem/difficulty in using direct-io based mechanism in the fuse kernel module when used with "XEN" in its "tap:aio" framework, we have seen xen vms hang over glusterfs or any fuse based filesystem due to fact that fuse module doesn't yet support "aio" with O_DIRECT internally as a kernel module. To have a work around fix we have to hardcode this value due to its usage in case of VM's. We are currently fixing this problem by fixing directly O_DIRECT problem in fuse. Which will be available in later releases for kernel. So its just a glusterfs specific case.
@@ -670,6 +692,12 @@ virStorageBackendFileSystemMount(virConnectPtr conn, }
if (pool->def->type == VIR_STORAGE_POOL_NETFS) { + if (pool->def->source.format = VIR_STORAGE_POOL_NETFS_GLUSTERFS) { + if (virAsprintf(&options, "direct-io-mode=1") == -1) { + virReportOOMError(conn); + return -1; + } + }
Normally direct-IO mode is something you'd let apps set on a per file basis as they desire. QEMU / Xen both know todo this, so I'm not convinced we should force it on for the whole mount point.
Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/:| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org:| |: http://autobuild.org -o- http://search.cpan.org/~danberr/<http://search.cpan.org/%7Edanberr/>:| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Thu, Jul 16, 2009 at 08:24:09PM +0530, Harshavardhana wrote:
Hi Daniel,
On Thu, Jul 16, 2009 at 7:46 PM, Daniel P. Berrange <berrange@redhat.com>wrote:
On Thu, Jul 16, 2009 at 06:06:25AM -0700, Harshavardhana wrote:
New option index added to support -o options for various netfs. Currently added an option for glusterfs.
What effect does it have ? Or why do we want/need it
Options could be required for filesystem to have few enhaced handling at the site where they will be under use. Correct approach for a configurable will be a new "XML" option in this case.
Regarding current patch: This is required for the glusterfs to work properly with VM's. Right now there is a problem/difficulty in using direct-io based mechanism in the fuse kernel module when used with "XEN" in its "tap:aio" framework, we have seen xen vms hang over glusterfs or any fuse based filesystem due to fact that fuse module doesn't yet support "aio" with O_DIRECT internally as a kernel module. To have a work around fix we have to hardcode this value due to its usage in case of VM's.
We are currently fixing this problem by fixing directly O_DIRECT problem in fuse. Which will be available in later releases for kernel.
ACK, I see why you need this for the current releases of kernel/glusterfs. As & when this problem is fixed we'll need to either remove this, or provide a way to turn it off again. I don't think this is the kind of tunable that should be exposed in the XML, since this is really just a hack to work around a bug in a specific releases. Someone using libvirt has no way to decide whether the hack is needed or not, so making them set it in the XML would not be desirable. One possibility would be to have a config file /etc/libvirt/storage.conf for controlling certain options like this per host. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Daniel, Thanks for the pointers, i will keep this mind as and when the problem is fixed i will send a patch against those specific changes. Regards -- Harshavardhana Z Research Inc http://www.zresearch.com/ On Fri, Jul 17, 2009 at 11:58 PM, Daniel P. Berrange <berrange@redhat.com>wrote:
Hi Daniel,
On Thu, Jul 16, 2009 at 7:46 PM, Daniel P. Berrange <berrange@redhat.com wrote:
On Thu, Jul 16, 2009 at 06:06:25AM -0700, Harshavardhana wrote:
New option index added to support -o options for various netfs. Currently added an option for glusterfs.
What effect does it have ? Or why do we want/need it
Options could be required for filesystem to have few enhaced handling at
On Thu, Jul 16, 2009 at 08:24:09PM +0530, Harshavardhana wrote: the
site where they will be under use. Correct approach for a configurable will be a new "XML" option in this case.
Regarding current patch: This is required for the glusterfs to work properly with VM's. Right now there is a problem/difficulty in using direct-io based mechanism in the fuse kernel module when used with "XEN" in its "tap:aio" framework, we have seen xen vms hang over glusterfs or any fuse based filesystem due to fact that fuse module doesn't yet support "aio" with O_DIRECT internally as a kernel module. To have a work around fix we have to hardcode this value due to its usage in case of VM's.
We are currently fixing this problem by fixing directly O_DIRECT problem in fuse. Which will be available in later releases for kernel.
ACK, I see why you need this for the current releases of kernel/glusterfs.
As & when this problem is fixed we'll need to either remove this, or provide a way to turn it off again. I don't think this is the kind of tunable that should be exposed in the XML, since this is really just a hack to work around a bug in a specific releases. Someone using libvirt has no way to decide whether the hack is needed or not, so making them set it in the XML would not be desirable. One possibility would be to have a config file /etc/libvirt/storage.conf for controlling certain options like this per host.
Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/:| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org:| |: http://autobuild.org -o- http://search.cpan.org/~danberr/<http://search.cpan.org/%7Edanberr/>:| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Hi Daniel, Are you going to apply this to mainline?. Was just wondering if you have any other concerns so that i could work on those if at all there are other better ways you would have. Thanks -- Harshavardhana Gluster - http://www.gluster.com/ On Sat, Jul 18, 2009 at 9:12 AM, Harshavardhana <harsha@gluster.com> wrote:
Daniel,
Thanks for the pointers, i will keep this mind as and when the problem is fixed i will send a patch against those specific changes.
Regards -- Harshavardhana Z Research Inc http://www.zresearch.com/
On Fri, Jul 17, 2009 at 11:58 PM, Daniel P. Berrange <berrange@redhat.com>wrote:
Hi Daniel,
On Thu, Jul 16, 2009 at 7:46 PM, Daniel P. Berrange < berrange@redhat.com>wrote:
On Thu, Jul 16, 2009 at 06:06:25AM -0700, Harshavardhana wrote:
New option index added to support -o options for various netfs. Currently added an option for glusterfs.
What effect does it have ? Or why do we want/need it
Options could be required for filesystem to have few enhaced handling at
On Thu, Jul 16, 2009 at 08:24:09PM +0530, Harshavardhana wrote: the
site where they will be under use. Correct approach for a configurable will be a new "XML" option in this case.
Regarding current patch: This is required for the glusterfs to work properly with VM's. Right now there is a problem/difficulty in using direct-io based mechanism in the fuse kernel module when used with "XEN" in its "tap:aio" framework, we have seen xen vms hang over glusterfs or any fuse based filesystem due to fact that fuse module doesn't yet support "aio" with O_DIRECT internally as a kernel module. To have a work around fix we have to hardcode this value due to its usage in case of VM's.
We are currently fixing this problem by fixing directly O_DIRECT problem in fuse. Which will be available in later releases for kernel.
ACK, I see why you need this for the current releases of kernel/glusterfs.
As & when this problem is fixed we'll need to either remove this, or provide a way to turn it off again. I don't think this is the kind of tunable that should be exposed in the XML, since this is really just a hack to work around a bug in a specific releases. Someone using libvirt has no way to decide whether the hack is needed or not, so making them set it in the XML would not be desirable. One possibility would be to have a config file /etc/libvirt/storage.conf for controlling certain options like this per host.
Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ <http://search.cpan.org/%7Edanberr/> :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Tue, Jul 21, 2009 at 06:43:10PM +0530, Harshavardhana wrote:
Hi Daniel,
Are you going to apply this to mainline?. Was just wondering if you have any other concerns so that i could work on those if at all there are other better ways you would have.
Ah right I nearly forgot about it :-) it's commited, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel P. Berrange
-
Daniel Veillard
-
Harshavardhana