[libvirt] [PATCH v2] selinux: Only create the selabel_handle once.
From: "Richard W.M. Jones" <rjones@redhat.com> According to Eric Paris this is slightly more efficient because it only loads the regular expressions in libselinux once. --- src/security/security_selinux.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a3ef728..d4f0595 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -935,20 +935,30 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon) return 0; } +#if HAVE_SELINUX_LABEL_H + +static struct selabel_handle *seLabelHandle = NULL; + +static int +seLabelHandleOnceInit (void) +{ + seLabelHandle = selabel_open(SELABEL_CTX_FILE, NULL, 0); + return seLabelHandle ? 0 : -1; +} + +VIR_ONCE_GLOBAL_INIT(seLabelHandle) + +#endif + /* Set fcon to the appropriate label for path and mode, or return -1. */ static int getContext(const char *newpath, mode_t mode, security_context_t *fcon) { #if HAVE_SELINUX_LABEL_H - struct selabel_handle *handle = selabel_open(SELABEL_CTX_FILE, NULL, 0); - int ret; - - if (handle == NULL) + if (seLabelHandleInitialize() < 0) return -1; - ret = selabel_lookup_raw(handle, fcon, newpath, mode); - selabel_close(handle); - return ret; + return selabel_lookup_raw(seLabelHandle, fcon, newpath, mode); #else return matchpathcon(newpath, mode, fcon); #endif -- 1.8.1
On Wed, Jan 23, 2013 at 08:33:55PM +0000, Richard W.M. Jones wrote:
From: "Richard W.M. Jones" <rjones@redhat.com>
According to Eric Paris this is slightly more efficient because it only loads the regular expressions in libselinux once. --- src/security/security_selinux.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a3ef728..d4f0595 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -935,20 +935,30 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon) return 0; }
+#if HAVE_SELINUX_LABEL_H + +static struct selabel_handle *seLabelHandle = NULL;
Same comment as previous version - this should go in the virSecuritySELinuxData struct, not a global variable. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Wed, Jan 23, 2013 at 08:35:13PM +0000, Daniel P. Berrange wrote:
On Wed, Jan 23, 2013 at 08:33:55PM +0000, Richard W.M. Jones wrote:
From: "Richard W.M. Jones" <rjones@redhat.com>
According to Eric Paris this is slightly more efficient because it only loads the regular expressions in libselinux once. --- src/security/security_selinux.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a3ef728..d4f0595 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -935,20 +935,30 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon) return 0; }
+#if HAVE_SELINUX_LABEL_H + +static struct selabel_handle *seLabelHandle = NULL;
Same comment as previous version - this should go in the virSecuritySELinuxData struct, not a global variable.
Will do .. Is there some problem with global variables? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
On Wed, Jan 23, 2013 at 08:39:05PM +0000, Richard W.M. Jones wrote:
On Wed, Jan 23, 2013 at 08:35:13PM +0000, Daniel P. Berrange wrote:
On Wed, Jan 23, 2013 at 08:33:55PM +0000, Richard W.M. Jones wrote:
From: "Richard W.M. Jones" <rjones@redhat.com>
According to Eric Paris this is slightly more efficient because it only loads the regular expressions in libselinux once. --- src/security/security_selinux.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a3ef728..d4f0595 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -935,20 +935,30 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon) return 0; }
+#if HAVE_SELINUX_LABEL_H + +static struct selabel_handle *seLabelHandle = NULL;
Same comment as previous version - this should go in the virSecuritySELinuxData struct, not a global variable.
Will do .. Is there some problem with global variables?
As we move to finer grained locking in libvirt, we're increasingly having one mutex per functional object. In this case the virSecurityManagerPtr objects. If there are multiple security manager objects then there is no clarity on locking rules for the global variables which are disassociated from the rest of the state that is being maintained. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Richard W.M. Jones