Re: [SSSD] [PATCH] Save all data to sysdb in one transaction
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/23/2010 07:51 AM, Stephen Gallagher wrote: > On 09/09/2010 08:29 AM, Sumit Bose wrote: >>> rebased versions are attached. It was necessary to add some >>> modifications during the rebase. I have done a couple of online and >>> offline test but it would be nice if someone else can run additional >>> tests. > > > Ack to both. > Pushed to master. - -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkybjM8ACgkQeiVVYja6o6P4EACfZ8p5MbjjJ2rtblZsxhnDy+cU FooAoIa+uK1p7thWBMdUeZ9fzIP9DXZO =84s/ -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Save all data to sysdb in one transaction
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/09/2010 08:29 AM, Sumit Bose wrote: > On Wed, Sep 08, 2010 at 09:52:31AM -0400, Stephen Gallagher wrote: > On 09/08/2010 09:50 AM, Sumit Bose wrote: On Wed, Sep 08, 2010 at 09:33:04AM -0400, Stephen Gallagher wrote: On 07/08/2010 12:57 PM, Stephen Gallagher wrote: >>> Just a reminder that it's been about a month since there was any >>> activity on this patch. >>> *bump* Any word on this one, Sumit? Would you prefer that I have someone else look into this? > ah, sorry, I have a patch which addresses the comments in my tree, but I > need to rebase it. I will send it tomorrow or Friday. > > Okay, no problem. If there are any further review comments at that > point, I'm going to turn it over to Jakub to finish up. > >> rebased versions are attached. It was necessary to add some >> modifications during the rebase. I have done a couple of online and >> offline test but it would be nice if someone else can run additional >> tests. Ack to both. - -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkybPzwACgkQeiVVYja6o6N65QCfZUpqW9yqwOM3YaFiSvi4j/ww W6sAnRrnJVucB51MhRfUmsgTiEs0VYBj =lWVt -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Save all data to sysdb in one transaction
On Wed, Sep 08, 2010 at 09:52:31AM -0400, Stephen Gallagher wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> On 09/08/2010 09:50 AM, Sumit Bose wrote:
> > On Wed, Sep 08, 2010 at 09:33:04AM -0400, Stephen Gallagher wrote:
> > On 07/08/2010 12:57 PM, Stephen Gallagher wrote:
> Just a reminder that it's been about a month since there was any
> activity on this patch.
>
> >
> > *bump* Any word on this one, Sumit? Would you prefer that I have someone
> > else look into this?
> >
> >> ah, sorry, I have a patch which addresses the comments in my tree, but I
> >> need to rebase it. I will send it tomorrow or Friday.
> >
>
> Okay, no problem. If there are any further review comments at that
> point, I'm going to turn it over to Jakub to finish up.
rebased versions are attached. It was necessary to add some
modifications during the rebase. I have done a couple of online and
offline test but it would be nice if someone else can run additional
tests.
bye,
Sumit
>
> - --
> Stephen Gallagher
> RHCE 804006346421761
>
> Delivering value year after year.
> Red Hat ranks #1 in value among software vendors.
> http://www.redhat.com/promo/vendor/
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v1.4.10 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkyHlR8ACgkQeiVVYja6o6MKkgCeNCEYKVfiOvZLl09I+zZJ7ZQ2
> YAkAn2hu0LXBr3I7AIjJN9nOFWP8CTXT
> =yHsK
> -END PGP SIGNATURE-
From 60db9e05332137d13ca437029419630e8c52b99f Mon Sep 17 00:00:00 2001
From: Sumit Bose
Date: Wed, 2 Jun 2010 11:45:24 +0200
Subject: [PATCH 1/2] Handle host objects like other objects
---
src/providers/ipa/ipa_access.c | 309 +++-
src/providers/ipa/ipa_access.h |3 +-
2 files changed, 183 insertions(+), 129 deletions(-)
diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c
index 3fb8191..d1ba6c7 100644
--- a/src/providers/ipa/ipa_access.c
+++ b/src/providers/ipa/ipa_access.c
@@ -164,6 +164,144 @@ static errno_t hbac_sysdb_data_recv(TALLOC_CTX *mem_ctx,
return EOK;
}
+static errno_t set_local_and_remote_host_info(TALLOC_CTX *mem_ctx,
+ size_t host_count,
+ struct sysdb_attrs **host_list,
+ const char *local_hostname,
+ const char *remote_hostname,
+ struct hbac_host_info
**local_hhi,
+ struct hbac_host_info
**remote_hhi)
+
+{
+size_t c;
+int ret;
+struct hbac_host_info *hhi;
+struct ldb_message_element *el;
+TALLOC_CTX *tmp_ctx;
+
+if (local_hostname == NULL || *local_hostname == '\0') {
+DEBUG(1, ("Missing local hostname.\n"));
+ret = EINVAL;
+goto done;
+}
+
+if (host_count == 0) {
+DEBUG(1, ("No host data available.\n"));
+ret = EINVAL;
+goto done;
+}
+
+tmp_ctx = talloc_new(mem_ctx);
+if (tmp_ctx == NULL) {
+ret = ENOMEM;
+goto done;
+}
+
+for (c = 0; c < host_count; c++) {
+hhi = talloc_zero(tmp_ctx, struct hbac_host_info);
+if (hhi == NULL) {
+DEBUG(1, ("talloc_zero failed.\n"));
+ret = ENOMEM;
+goto done;
+}
+
+ret = sysdb_attrs_get_el(host_list[c], SYSDB_ORIG_DN, &el);
+if (ret != EOK) {
+DEBUG(1, ("sysdb_attrs_get_el failed.\n"));
+goto done;
+}
+if (el->num_values == 0) {
+DEBUG(1, ("Missing OriginalDN.\n"));
+ret = EINVAL;
+goto done;
+}
+DEBUG(9, ("OriginalDN: [%.*s].\n", el->values[0].length,
+ (char *)el->values[0].data));
+hhi->dn = talloc_strndup(hhi, (char *)el->values[0].data,
+ el->values[0].length);
+if (hhi->dn == NULL) {
+DEBUG(1, ("talloc_strndup failed.\n"));
+ret = ENOMEM;
+goto done;
+}
+
+ret = sysdb_attrs_get_el(host_list[c], IPA_HOST_SERVERHOSTNAME, &el);
+if (ret != EOK) {
+DEBUG(1, ("sysdb_attrs_get_el failed.\n"));
+goto done;
+}
+if (el->num_values == 0) {
+DEBUG(1, ("Missing ServerHostName.\n"));
+ret = EINVAL;
+goto done;
+}
+DEBUG(9, ("ServerHostName: [%.*s].\n", el->values[0].length,
+ (char *)el->values[0].data));
+hhi->serverhostname = talloc_strndup(hhi, (char *)el->values[0].data,
+ el->values[0].length);
+if (hhi->serverhostname == NULL) {
+DEBUG(1, ("talloc_strndup failed.\n"));
+ret = ENOMEM;
+goto done;
+}
+
+ret = sys
Re: [SSSD] [PATCH] Save all data to sysdb in one transaction
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/08/2010 09:50 AM, Sumit Bose wrote: > On Wed, Sep 08, 2010 at 09:33:04AM -0400, Stephen Gallagher wrote: > On 07/08/2010 12:57 PM, Stephen Gallagher wrote: Just a reminder that it's been about a month since there was any activity on this patch. > > *bump* Any word on this one, Sumit? Would you prefer that I have someone > else look into this? > >> ah, sorry, I have a patch which addresses the comments in my tree, but I >> need to rebase it. I will send it tomorrow or Friday. > Okay, no problem. If there are any further review comments at that point, I'm going to turn it over to Jakub to finish up. - -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkyHlR8ACgkQeiVVYja6o6MKkgCeNCEYKVfiOvZLl09I+zZJ7ZQ2 YAkAn2hu0LXBr3I7AIjJN9nOFWP8CTXT =yHsK -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Save all data to sysdb in one transaction
On Wed, Sep 08, 2010 at 09:33:04AM -0400, Stephen Gallagher wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > On 07/08/2010 12:57 PM, Stephen Gallagher wrote: > > Just a reminder that it's been about a month since there was any > > activity on this patch. > > > > *bump* Any word on this one, Sumit? Would you prefer that I have someone > else look into this? ah, sorry, I have a patch which addresses the comments in my tree, but I need to rebase it. I will send it tomorrow or Friday. bye, Sumit > > > > > On 06/10/2010 07:53 AM, Stephen Gallagher wrote: > >> On 06/10/2010 05:56 AM, Sumit Bose wrote: > >>> Hi, > >>> > >>> this patch for master saves all IPA HBAC data in one transaction and > >>> proceeds if the sysdb transaction fails. It addresses a review comment > >>> from the "Use new schema for HBAC service checks" patch. > >>> > >>> The first patch changes the handling of the host data to be compatible > >>> with the other data. > > > >> Nack. > > > >> I think it would be better form in set_local_and_remote_host_info() to > >> maintain a private memory context onto which to allocate the > >> hbac_host_info objects until we're ready to return, at which time they > >> should be stolen back onto mem_ctx. > > > >> This way if we end up in the failure condition, we don't exit the > >> function with potentially host_count chunks of memory of sizeof(struct > >> hbac_host_info) sitting around uselessly until the mem_ctx is eventually > >> freed. > > > >> For most functions, I wouldn't recommend this, but this particular > >> function has the potential of allocating a LOT of memory (if there are a > >> lot of HBAC rules on the server) and I'd prefer that we not hang onto it > >> if something goes wrong. > > > >>> The second removes the individual transactions and > >>> introduces a common sysdb transaction. > >>> > > > >> Nack. > > > >> In hbac_save_data_to_sysdb() you start a transaction, then create a > >> tmp_ctx, but if talloc_new() fails, you exit the function without > >> cancelling the transaction. > > > >> You have a memory leak on the success case of hbac_save_data_to_sysdb(). > >> You're not freeing tmp_ctx here. (Yeah, it's a short leak, as it will be > >> cleared when mem_ctx is, but it's bad form). > > > >> Similar to my comment about set_local_and_remote_host_info() in the > >> first patch, since it's possible for us to allocate a lot of memory (if > >> count is large) in hbac_save_list(), I think it makes sense to do > >> allocations in this function in a tmp_ctx. Furthermore, unless I'm > >> reading this wrong, the allocation made here does not get returned to > >> the calling function, so proper memory management policy would be to > >> clean it up here. > > > >> We probably shouldn't be passing in a mem_ctx to hbac_save_list() at > >> all. The function does not return anything up the stack, so we should > >> contain its memory usage entirely within the function. > > > >> As a note for the future, I don't see a purpose for passing mem_ctx to > >> sysdb_delete_recursive() either. It returns no values that we are > >> supposed to be responsible for, so having it attach to our memory > >> context is a bad idea. It should attach to a new toplevel context > >> instead, so valgrind would notice if we ever forget to free it properly. > > > >>> Do we want/need a similar patch for master, too? > >> I'm assuming you mean sssd-1-2 here. I don't think we're likely to hit > >> situations where the sysdb can't be saved very often, so I'd suggest > >> leaving it out of sssd-1-2 unless we get bugs reported on it. > > > > > > > > > ___ > sssd-devel mailing list > sssd-devel@lists.fedorahosted.org > https://fedorahosted.org/mailman/listinfo/sssd-devel > > - -- > Stephen Gallagher > RHCE 804006346421761 > > Delivering value year after year. > Red Hat ranks #1 in value among software vendors. > http://www.redhat.com/promo/vendor/ > -BEGIN PGP SIGNATURE- > Version: GnuPG v1.4.10 (GNU/Linux) > Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ > > iEYEARECAAYFAkyHkJAACgkQeiVVYja6o6PURACgpC0W+4z+CG87NTIXndv6AFtj > iasAn2/gdOdxzNhjzefPDXLDEQgsFFd3 > =0qZe > -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Save all data to sysdb in one transaction
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/08/2010 03:33 PM, Stephen Gallagher wrote: > *bump* Any word on this one, Sumit? Would you prefer that I have someone > else look into this? I think Sumit stated yesterday that he's currently swamped, as my task list for the upcoming release is shrinking, I can volunteer to finish the patch. Jakub -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkyHkv0ACgkQHsardTLnvCXbfgCgz8GAcnO1J6ZIRUhoJuQrFpco Hd8An2sONHPpFv+WrRwOntkv/Em9kM4D =Jcqr -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Save all data to sysdb in one transaction
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 07/08/2010 12:57 PM, Stephen Gallagher wrote: > Just a reminder that it's been about a month since there was any > activity on this patch. > *bump* Any word on this one, Sumit? Would you prefer that I have someone else look into this? > > On 06/10/2010 07:53 AM, Stephen Gallagher wrote: >> On 06/10/2010 05:56 AM, Sumit Bose wrote: >>> Hi, >>> >>> this patch for master saves all IPA HBAC data in one transaction and >>> proceeds if the sysdb transaction fails. It addresses a review comment >>> from the "Use new schema for HBAC service checks" patch. >>> >>> The first patch changes the handling of the host data to be compatible >>> with the other data. > >> Nack. > >> I think it would be better form in set_local_and_remote_host_info() to >> maintain a private memory context onto which to allocate the >> hbac_host_info objects until we're ready to return, at which time they >> should be stolen back onto mem_ctx. > >> This way if we end up in the failure condition, we don't exit the >> function with potentially host_count chunks of memory of sizeof(struct >> hbac_host_info) sitting around uselessly until the mem_ctx is eventually >> freed. > >> For most functions, I wouldn't recommend this, but this particular >> function has the potential of allocating a LOT of memory (if there are a >> lot of HBAC rules on the server) and I'd prefer that we not hang onto it >> if something goes wrong. > >>> The second removes the individual transactions and >>> introduces a common sysdb transaction. >>> > >> Nack. > >> In hbac_save_data_to_sysdb() you start a transaction, then create a >> tmp_ctx, but if talloc_new() fails, you exit the function without >> cancelling the transaction. > >> You have a memory leak on the success case of hbac_save_data_to_sysdb(). >> You're not freeing tmp_ctx here. (Yeah, it's a short leak, as it will be >> cleared when mem_ctx is, but it's bad form). > >> Similar to my comment about set_local_and_remote_host_info() in the >> first patch, since it's possible for us to allocate a lot of memory (if >> count is large) in hbac_save_list(), I think it makes sense to do >> allocations in this function in a tmp_ctx. Furthermore, unless I'm >> reading this wrong, the allocation made here does not get returned to >> the calling function, so proper memory management policy would be to >> clean it up here. > >> We probably shouldn't be passing in a mem_ctx to hbac_save_list() at >> all. The function does not return anything up the stack, so we should >> contain its memory usage entirely within the function. > >> As a note for the future, I don't see a purpose for passing mem_ctx to >> sysdb_delete_recursive() either. It returns no values that we are >> supposed to be responsible for, so having it attach to our memory >> context is a bad idea. It should attach to a new toplevel context >> instead, so valgrind would notice if we ever forget to free it properly. > >>> Do we want/need a similar patch for master, too? >> I'm assuming you mean sssd-1-2 here. I don't think we're likely to hit >> situations where the sysdb can't be saved very often, so I'd suggest >> leaving it out of sssd-1-2 unless we get bugs reported on it. > > > > ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel - -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkyHkJAACgkQeiVVYja6o6PURACgpC0W+4z+CG87NTIXndv6AFtj iasAn2/gdOdxzNhjzefPDXLDEQgsFFd3 =0qZe -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Save all data to sysdb in one transaction
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Just a reminder that it's been about a month since there was any activity on this patch. On 06/10/2010 07:53 AM, Stephen Gallagher wrote: > On 06/10/2010 05:56 AM, Sumit Bose wrote: >> Hi, >> >> this patch for master saves all IPA HBAC data in one transaction and >> proceeds if the sysdb transaction fails. It addresses a review comment >> from the "Use new schema for HBAC service checks" patch. >> >> The first patch changes the handling of the host data to be compatible >> with the other data. > > Nack. > > I think it would be better form in set_local_and_remote_host_info() to > maintain a private memory context onto which to allocate the > hbac_host_info objects until we're ready to return, at which time they > should be stolen back onto mem_ctx. > > This way if we end up in the failure condition, we don't exit the > function with potentially host_count chunks of memory of sizeof(struct > hbac_host_info) sitting around uselessly until the mem_ctx is eventually > freed. > > For most functions, I wouldn't recommend this, but this particular > function has the potential of allocating a LOT of memory (if there are a > lot of HBAC rules on the server) and I'd prefer that we not hang onto it > if something goes wrong. > >> The second removes the individual transactions and >> introduces a common sysdb transaction. >> > > Nack. > > In hbac_save_data_to_sysdb() you start a transaction, then create a > tmp_ctx, but if talloc_new() fails, you exit the function without > cancelling the transaction. > > You have a memory leak on the success case of hbac_save_data_to_sysdb(). > You're not freeing tmp_ctx here. (Yeah, it's a short leak, as it will be > cleared when mem_ctx is, but it's bad form). > > Similar to my comment about set_local_and_remote_host_info() in the > first patch, since it's possible for us to allocate a lot of memory (if > count is large) in hbac_save_list(), I think it makes sense to do > allocations in this function in a tmp_ctx. Furthermore, unless I'm > reading this wrong, the allocation made here does not get returned to > the calling function, so proper memory management policy would be to > clean it up here. > > We probably shouldn't be passing in a mem_ctx to hbac_save_list() at > all. The function does not return anything up the stack, so we should > contain its memory usage entirely within the function. > > As a note for the future, I don't see a purpose for passing mem_ctx to > sysdb_delete_recursive() either. It returns no values that we are > supposed to be responsible for, so having it attach to our memory > context is a bad idea. It should attach to a new toplevel context > instead, so valgrind would notice if we ever forget to free it properly. > >> Do we want/need a similar patch for master, too? > I'm assuming you mean sssd-1-2 here. I don't think we're likely to hit > situations where the sysdb can't be saved very often, so I'd suggest > leaving it out of sssd-1-2 unless we get bugs reported on it. > > - -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.14 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkw2A14ACgkQeiVVYja6o6M7BwCdGhWNQuD2U0E1ejHywzUhCwG0 XT0An1xZAn1H1Gf7vz8qjXpxvl0rlxq2 =LM77 -END PGP SIGNATURE- ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] Save all data to sysdb in one transaction
On 06/10/2010 05:56 AM, Sumit Bose wrote: > Hi, > > this patch for master saves all IPA HBAC data in one transaction and > proceeds if the sysdb transaction fails. It addresses a review comment > from the "Use new schema for HBAC service checks" patch. > > The first patch changes the handling of the host data to be compatible > with the other data. Nack. I think it would be better form in set_local_and_remote_host_info() to maintain a private memory context onto which to allocate the hbac_host_info objects until we're ready to return, at which time they should be stolen back onto mem_ctx. This way if we end up in the failure condition, we don't exit the function with potentially host_count chunks of memory of sizeof(struct hbac_host_info) sitting around uselessly until the mem_ctx is eventually freed. For most functions, I wouldn't recommend this, but this particular function has the potential of allocating a LOT of memory (if there are a lot of HBAC rules on the server) and I'd prefer that we not hang onto it if something goes wrong. > The second removes the individual transactions and > introduces a common sysdb transaction. > Nack. In hbac_save_data_to_sysdb() you start a transaction, then create a tmp_ctx, but if talloc_new() fails, you exit the function without cancelling the transaction. You have a memory leak on the success case of hbac_save_data_to_sysdb(). You're not freeing tmp_ctx here. (Yeah, it's a short leak, as it will be cleared when mem_ctx is, but it's bad form). Similar to my comment about set_local_and_remote_host_info() in the first patch, since it's possible for us to allocate a lot of memory (if count is large) in hbac_save_list(), I think it makes sense to do allocations in this function in a tmp_ctx. Furthermore, unless I'm reading this wrong, the allocation made here does not get returned to the calling function, so proper memory management policy would be to clean it up here. We probably shouldn't be passing in a mem_ctx to hbac_save_list() at all. The function does not return anything up the stack, so we should contain its memory usage entirely within the function. As a note for the future, I don't see a purpose for passing mem_ctx to sysdb_delete_recursive() either. It returns no values that we are supposed to be responsible for, so having it attach to our memory context is a bad idea. It should attach to a new toplevel context instead, so valgrind would notice if we ever forget to free it properly. > Do we want/need a similar patch for master, too? I'm assuming you mean sssd-1-2 here. I don't think we're likely to hit situations where the sysdb can't be saved very often, so I'd suggest leaving it out of sssd-1-2 unless we get bugs reported on it. -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
[SSSD] [PATCH] Save all data to sysdb in one transaction
Hi,
this patch for master saves all IPA HBAC data in one transaction and
proceeds if the sysdb transaction fails. It addresses a review comment
from the "Use new schema for HBAC service checks" patch.
The first patch changes the handling of the host data to be compatible
with the other data. The second removes the individual transactions and
introduces a common sysdb transaction.
Do we want/need a similar patch for master, too?
bye,
Sumit
From 95a2ae812c5d2fd85a17ec7c98eb5ae55ee67aa6 Mon Sep 17 00:00:00 2001
From: Sumit Bose
Date: Wed, 2 Jun 2010 11:45:24 +0200
Subject: [PATCH 1/2] Handle host objects like other objects
---
src/providers/ipa/ipa_access.c | 271 +++-
src/providers/ipa/ipa_access.h |3 +-
2 files changed, 156 insertions(+), 118 deletions(-)
diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c
index 3d66c16..25302f8 100644
--- a/src/providers/ipa/ipa_access.c
+++ b/src/providers/ipa/ipa_access.c
@@ -159,6 +159,136 @@ static errno_t hbac_sysdb_data_recv(TALLOC_CTX *mem_ctx,
return EOK;
}
+static errno_t set_local_and_remote_host_info(TALLOC_CTX *mem_ctx,
+ size_t host_count,
+ struct sysdb_attrs **host_list,
+ const char *local_hostname,
+ const char *remote_hostname,
+ struct hbac_host_info
**local_hhi,
+ struct hbac_host_info
**remote_hhi)
+
+{
+size_t c;
+int ret;
+struct hbac_host_info *hhi;
+struct ldb_message_element *el;
+
+if (local_hostname == NULL || *local_hostname == '\0') {
+DEBUG(1, ("Missing local hostname.\n"));
+ret = EINVAL;
+goto fail;
+}
+
+if (host_count == 0) {
+DEBUG(1, ("No host data available.\n"));
+ret = EINVAL;
+goto fail;
+}
+
+for (c = 0; c < host_count; c++) {
+hhi = talloc_zero(mem_ctx, struct hbac_host_info);
+if (hhi == NULL) {
+DEBUG(1, ("talloc_zero failed.\n"));
+ret = ENOMEM;
+goto fail;
+}
+
+ret = sysdb_attrs_get_el(host_list[c], SYSDB_ORIG_DN, &el);
+if (ret != EOK) {
+DEBUG(1, ("sysdb_attrs_get_el failed.\n"));
+goto fail;
+}
+if (el->num_values == 0) {
+DEBUG(1, ("Missing OriginalDN.\n"));
+ret = EINVAL;
+goto fail;
+}
+DEBUG(9, ("OriginalDN: [%.*s].\n", el->values[0].length,
+ (char *)el->values[0].data));
+hhi->dn = talloc_strndup(hhi, (char *)el->values[0].data,
+ el->values[0].length);
+if (hhi->dn == NULL) {
+DEBUG(1, ("talloc_strndup failed.\n"));
+ret = ENOMEM;
+goto fail;
+}
+
+ret = sysdb_attrs_get_el(host_list[c], IPA_HOST_SERVERHOSTNAME, &el);
+if (ret != EOK) {
+DEBUG(1, ("sysdb_attrs_get_el failed.\n"));
+goto fail;
+}
+if (el->num_values == 0) {
+DEBUG(1, ("Missing ServerHostName.\n"));
+ret = EINVAL;
+goto fail;
+}
+DEBUG(9, ("ServerHostName: [%.*s].\n", el->values[0].length,
+ (char *)el->values[0].data));
+hhi->serverhostname = talloc_strndup(hhi, (char *)el->values[0].data,
+ el->values[0].length);
+if (hhi->serverhostname == NULL) {
+DEBUG(1, ("talloc_strndup failed.\n"));
+ret = ENOMEM;
+goto fail;
+}
+
+ret = sysdb_attrs_get_el(host_list[c], IPA_HOST_FQDN, &el);
+if (ret != EOK) {
+DEBUG(1, ("sysdb_attrs_get_el failed.\n"));
+goto fail;
+}
+if (el->num_values == 0) {
+DEBUG(1, ("Missing FQDN.\n"));
+ret = EINVAL;
+goto fail;
+}
+DEBUG(9, ("FQDN: [%.*s].\n", el->values[0].length,
+ (char *)el->values[0].data));
+hhi->fqdn = talloc_strndup(hhi, (char *)el->values[0].data,
+ el->values[0].length);
+if (hhi->fqdn == NULL) {
+DEBUG(1, ("talloc_strndup failed.\n"));
+ret = ENOMEM;
+goto fail;
+}
+
+ret = sysdb_attrs_get_string_array(host_list[c], SYSDB_ORIG_MEMBEROF,
+ hhi, &hhi->memberof);
+if (ret != EOK) {
+if (ret != ENOENT) {
+DEBUG(1, ("sysdb_attrs_get_string_array failed.\n"));
+goto fail;
+}
+
+hhi->memberof = talloc_array(hhi, const char *, 1);
+if (hhi->memberof == NULL) {
+
