[SEMANAGE] Commit numbers for ro database calls

[SEMANAGE] Commit numbers for ro database calls

[Ivan Gyurdiev]

[-- Attachment #1: Type: text/plain, Size: 518 bytes --]

This should make Joshua happy... return commit numbers on all ro 
operations. This does not return commit numbers on rw operations, which 
is consistent with what the modules code does (I think...).

Next we should take advantage of commit numbers to only re-read the 
cache on ro calls when the commit number has changed.

----
I don't like how dependency on semanage_store.c is creeping into database.c.
(but it was there to begin with - active_lock, and so on...) - might 
need to reorganize some of this later..



[-- Attachment #2: libsemanage6.commit_numbers.diff --]
[-- Type: text/x-patch, Size: 1531 bytes --]

diff -Naurp --exclude-from excludes old/libsemanage/src/database.c new/libsemanage/src/database.c
--- old/libsemanage/src/database.c	2005-11-04 15:37:49.000000000 -0500
+++ new/libsemanage/src/database.c	2005-12-23 06:14:15.000000000 -0500
@@ -45,14 +45,18 @@ static int enter_ro(
 	return STATUS_ERR;
 }
 
-static inline void exit_ro(
+static inline int exit_ro(
 	semanage_handle_t* handle,
 	dbase_config_t* dconfig) {
 
+	int commit_num = semanage_get_commit_number(handle);
+
 	if (!handle->is_in_transaction) {
 		semanage_release_active_lock(handle);
 		dconfig->dtable->drop_cache(dconfig->dbase);
 	}
+
+	return commit_num;
 }
 
 static int enter_rw(
@@ -150,8 +154,7 @@ int dbase_query (
 		return STATUS_ERR;
 	}
 
-	exit_ro(handle, dconfig);
-	return STATUS_SUCCESS;
+	return exit_ro(handle, dconfig);
 }
 
 int dbase_exists (
@@ -168,8 +171,7 @@ int dbase_exists (
 		return STATUS_ERR;
 	}
 
-	exit_ro(handle, dconfig);
-	return STATUS_SUCCESS;
+	return exit_ro(handle, dconfig);
 }
 
 int dbase_count (
@@ -185,8 +187,7 @@ int dbase_count (
 		return STATUS_ERR;
 	}
 
-	exit_ro(handle, dconfig);
-	return STATUS_SUCCESS;
+	return exit_ro(handle, dconfig);
 }
 
 int dbase_iterate(
@@ -203,8 +204,7 @@ int dbase_iterate(
 		return STATUS_ERR;
 	}
 
-	exit_ro(handle, dconfig);
-	return STATUS_SUCCESS;
+	return exit_ro(handle, dconfig);
 }
 
 int dbase_list (
@@ -221,6 +221,5 @@ int dbase_list (
 		return STATUS_ERR;
 	}
 
-	exit_ro(handle, dconfig);
-	return STATUS_SUCCESS;
+	return exit_ro(handle, dconfig);
 }

Re: [SEMANAGE] Commit numbers for ro database calls

[Ivan Gyurdiev]

>> Next we should take advantage of commit numbers to only re-read the 
>> cache on ro calls when the commit number has changed.
>
> It only matters when you read more than 1 thing at a time. AFAIK this 
> only happens in semanage for selinux users. 
I think we're talking about different things - my comment was about 
library performance - not re-caching unless we have to (particularly in 
the policydb case, which is slower). You seem to be referring to using 
commit numbers in the client to prevent races (which, I agree, is 
another thing that needs to be done).
> However, there are queries done outside a transaction and then a 
> transaction started (mostly error handling, eg: Seuser already 
> defined), technically a race but inconsequential since there is error 
> handling in libsemanage. My example (pywrap-test) implementation makes 
> the transaction window as small as possible since it is a 
> discretionary lock but any libsemanage user needs to be careful with 
> this, perhaps something we should document.
Yes....
In general, is there any documentation about libsemanage (API?)
Or should we start writing that, and how? I don't know how to use any 
doc. software yet
(not even writing manpages), so I'd have to learn. Someone complained 
about lack of
documentation of the seusers format earlier.

>>
>> ----
>> I don't like how dependency on semanage_store.c is creeping into 
>> database.c.
>> (but it was there to begin with - active_lock, and so on...) - might 
>> need to reorganize some of this later..
>
> This is because database.c is handling locking when it should really 
> only be handled by semanage_store, since locks are a property of the 
> store. 
The database only signals when we're entering and exiting. The store is 
responsible for managing the lock files, and the details of locking. Not 
sure how else you'd have it done...

> The database (file backend) implementation reads files directly out of 
> the store which is probably broken. Not sure if it is even worth 
> fixing though, since there is already support for non-file backends 
> that seems to work.
How is it broken? How should it read the data?



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [SEMANAGE] Commit numbers for ro database calls

[Ivan Gyurdiev]

>>
>> How is it broken? How should it read the data?
>>
> It should request the data from semanage_store if it is being stored 
> in the store, just like you'd request the data from an LDAP server, 
> etc. Right now the database is sneaking into the store without going 
> through the store API, which is broken but like I said, probably not 
> worth the time to fix.
I didn't realize there was an API for reading things from the store.
I guess what you're asking me to do is to move code for read/write into 
the semanage_store,
which I could do, but I'm not sure I see the benefit of that - it will 
just complicate things and make
the store dependent on polymorphism, which I'm guessing you wouldn't like.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [SEMANAGE] Commit numbers for ro database calls

[Joshua Brindle]

Ivan Gyurdiev wrote:
> This should make Joshua happy... return commit numbers on all ro 
> operations. This does not return commit numbers on rw operations, which 
> is consistent with what the modules code does (I think...).
> 
great. correct, rw operations are 0 for success <0 for error. This 
should probably be documented (other than in comments)

> Next we should take advantage of commit numbers to only re-read the 
> cache on ro calls when the commit number has changed.

It only matters when you read more than 1 thing at a time. AFAIK this 
only happens in semanage for selinux users. However, there are queries 
done outside a transaction and then a transaction started (mostly error 
handling, eg: Seuser already defined), technically a race but 
inconsequential since there is error handling in libsemanage. My example 
(pywrap-test) implementation makes the transaction window as small as 
possible since it is a discretionary lock but any libsemanage user needs 
to be careful with this, perhaps something we should document.
> 
> ----
> I don't like how dependency on semanage_store.c is creeping into 
> database.c.
> (but it was there to begin with - active_lock, and so on...) - might 
> need to reorganize some of this later..

This is because database.c is handling locking when it should really 
only be handled by semanage_store, since locks are a property of the 
store. The database (file backend) implementation reads files directly 
out of the store which is probably broken. Not sure if it is even worth 
fixing though, since there is already support for non-file backends that 
seems to work.
> 
> 

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [SEMANAGE] Commit numbers for ro database calls

[Joshua Brindle]

Ivan Gyurdiev wrote:
> 
>>> Next we should take advantage of commit numbers to only re-read the 
>>> cache on ro calls when the commit number has changed.
>>
>>
>> It only matters when you read more than 1 thing at a time. AFAIK this 
>> only happens in semanage for selinux users. 
> 
> I think we're talking about different things - my comment was about 
> library performance - not re-caching unless we have to (particularly in 
> the policydb case, which is slower). You seem to be referring to using 
> commit numbers in the client to prevent races (which, I agree, is 
> another thing that needs to be done).
> 
Right, but if you do exactly 1 query and then exit (semanage login -l) 
then it's irrelavent for both cases (race and cache optimization), which 
was my point.

>> However, there are queries done outside a transaction and then a 
>> transaction started (mostly error handling, eg: Seuser already 
>> defined), technically a race but inconsequential since there is error 
>> handling in libsemanage. My example (pywrap-test) implementation makes 
>> the transaction window as small as possible since it is a 
>> discretionary lock but any libsemanage user needs to be careful with 
>> this, perhaps something we should document.
> 
> Yes....
> In general, is there any documentation about libsemanage (API?)
> Or should we start writing that, and how? I don't know how to use any 
> doc. software yet
> (not even writing manpages), so I'd have to learn. Someone complained 
> about lack of
> documentation of the seusers format earlier.
> 
There is no API doc outside the headers. Man pages are probably prefered 
but I don't really know how to make them either. Perhaps Steve has a 
suggestion for this?

>>>
>>> ----
>>> I don't like how dependency on semanage_store.c is creeping into 
>>> database.c.
>>> (but it was there to begin with - active_lock, and so on...) - might 
>>> need to reorganize some of this later..
>>
>>
>> This is because database.c is handling locking when it should really 
>> only be handled by semanage_store, since locks are a property of the 
>> store. 
> 
> The database only signals when we're entering and exiting. The store is 
> responsible for managing the lock files, and the details of locking. Not 
> sure how else you'd have it done...
>
>> The database (file backend) implementation reads files directly out of 
>> the store which is probably broken. Not sure if it is even worth 
>> fixing though, since there is already support for non-file backends 
>> that seems to work.
> 
> How is it broken? How should it read the data?
> 
It should request the data from semanage_store if it is being stored in 
the store, just like you'd request the data from an LDAP server, etc. 
Right now the database is sneaking into the store without going through 
the store API, which is broken but like I said, probably not worth the 
time to fix.


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [SEMANAGE] Commit numbers for ro database calls

[Joshua Brindle]

Ivan Gyurdiev wrote:
> 
>>>
>>> How is it broken? How should it read the data?
>>>
>> It should request the data from semanage_store if it is being stored 
>> in the store, just like you'd request the data from an LDAP server, 
>> etc. Right now the database is sneaking into the store without going 
>> through the store API, which is broken but like I said, probably not 
>> worth the time to fix.
> 
> I didn't realize there was an API for reading things from the store.\
There isn't exactly, but the dependance on semanage_store that you 
commented on is a symptom of this. You wouldn't put ldap locking in a 
database, for example, likewise with semanage_store.

> I guess what you're asking me to do is to move code for read/write into 
> the semanage_store,
> which I could do, but I'm not sure I see the benefit of that - it will 
> just complicate things and make
> the store dependent on polymorphism, which I'm guessing you wouldn't like.
> 

Right, these are within the same library so it isn't an issue at all 
(except maybe stylism) and the current stuff work so I'd rather not 
touch it. One could always view the _file backends as an extension of 
semanage_store, which makes them unconditionally depenadant on it, which 
  I'm fine with.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [SEMANAGE] Commit numbers for ro database calls

[Stephen Smalley]

On Mon, 2006-01-02 at 14:11 -0500, Joshua Brindle wrote:
> Ivan Gyurdiev wrote:
> > Yes....
> > In general, is there any documentation about libsemanage (API?)
> > Or should we start writing that, and how? I don't know how to use any 
> > doc. software yet
> > (not even writing manpages), so I'd have to learn. Someone complained 
> > about lack of
> > documentation of the seusers format earlier.
> > 
> There is no API doc outside the headers. Man pages are probably prefered 
> but I don't really know how to make them either. Perhaps Steve has a 
> suggestion for this?

man pages are preferred (at least as a starting point; supplemental
documentation is fine as well, but man pages should always be
available).  Just copy an existing one from libselinux or libsepol and
use it as a template.  Prioritize them according to likelihood of use
outside of core SELinux code and actual use.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.