On 08/18/2016 07:47 AM, Erik Skultety wrote:
Now that we're in the critical section, syslog connection can be
re-opened
by issuing openlog, which is something that cannot be done beforehand, since
syslog keeps its file descriptor private and changing the tag earlier might
introduce a log inconsistency if something went wrong with preparing a new set
of logging outputs in order to replace the existing one.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/util/virlog.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index c0b8b9a..61e71a3 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1794,16 +1794,35 @@ virLogFindOutput(virLogOutputPtr *outputs, size_t noutputs,
int
virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs)
{
+ int ret = -1;
+ size_t i;
+ char *tmp = NULL;
+
if (virLogInitialize() < 0)
return -1;
virLogLock();
virLogResetOutputs();
+
+ /* nothing can go wrong now (except for malloc) and we're also holding the
+ * lock so it's safe to call openlog and change the message tag */
+ for (i = 0; i < noutputs; i++) {
+ if (outputs[i]->dest == VIR_LOG_TO_SYSLOG) {
Is this essentially open coded virLogFindOutput? Or is it that the
->name and current_ident check that would trip that up based on the
weirdness from the previous patch?
If you modify the Find to do that STREQ_NULLABLE check - I think then
that function could be used...
+ if (VIR_STRDUP_QUIET(tmp, outputs[i]->name) < 0)
+ goto cleanup;
+ VIR_FREE(current_ident);
+ current_ident = tmp;
+ openlog(current_ident, 0, 0);
+ }
+ }
+
virLogOutputs = outputs;
virLogNbOutputs = noutputs;
- virLogUnlock();
- return virLogNbOutputs;
+ ret = virLogNbOutputs;
BTW: Still see no reason to not have ret be 0... From the earlier comment...
Conditional ACK... curious about usage of virFindLogOutput...
John
+ cleanup:
+ virLogUnlock();
+ return ret;
}