[1925] in Moira
Re: dcm patch
daemon@ATHENA.MIT.EDU (Garry Zacheiss)
Mon Jan 21 02:02:28 2002
Message-Id: <200201210659.BAA27256@riff-raff.mit.edu>
To: Jonathon Weiss <jweiss@mit.edu>
cc: Garry Zacheiss <zacheiss@mit.edu>, moiradev@mit.edu
In-Reply-To: Your message of "Sun, 20 Jan 2002 17:58:07 EST."
<200201202258.RAA26160@bearing-an-hourglass.mit.edu>
Date: Mon, 21 Jan 2002 01:59:43 -0500
From: Garry Zacheiss <zacheiss@MIT.EDU>
This should clean this up a bit.
I was aware of the race condition; fixing it completely is
annoying and didn't seem particularly worthwhile. I've commented
appropriately.
We end up needing to parse our options before we call
generate_service() on them, since we want things like:
dcm spam spam spam spam spam spam baked_beans spam -f
to work. (i.e., we can't always assume the -f is argv[1].) So now we
parse all our options in a while loop, generating an array of services,
and then iterate over that.
The pointer arithmetic in the while control statement exists in
multiple places in moira, isn't my fault, and does what I want, so I've
left it there.
Index: dcm.pc
===================================================================
RCS file: /afs/athena.mit.edu/astaff/project/moiradev/repository/moira/dcm/dcm.pc,v
retrieving revision 1.15
diff -u -r1.15 dcm.pc
--- dcm.pc 2002/01/18 00:00:35 1.15
+++ dcm.pc 2002/01/21 06:52:29
@@ -53,8 +53,8 @@
int enable;
EXEC SQL END DECLARE SECTION;
struct save_queue *sq;
- int status;
- char **arg = argv;
+ int status, srvcnt = 0;
+ char **arg = argv, *services[BUFSIZ];
if (strchr(argv[0], '/'))
strcpy(whoami, strrchr(argv[0], '/') + 1);
@@ -79,17 +79,32 @@
exit(1);
}
}
+ else
+ /* Doesn't begin with a dash, is a service name.
+ * Build an array of them we can iterate through later.
+ */
+ {
+ services[srvcnt] = malloc(SERVERS_NAME_SIZE);
+ if (!services[srvcnt])
+ {
+ com_err(whoami, 0, "Out of memory!");
+ exit(1);
+ }
+ strncpy(services[srvcnt], *arg, SERVERS_NAME_SIZE);
+ srvcnt++;
+ }
}
- /* if services were specified on the command line, do just those ones */
- if (argc > 1)
+ /* Iterate through services specified on the command line, if any. */
+ if (srvcnt > 0)
{
- for (i = 1; i < argc; i++)
+ for (i = 0; i < srvcnt; i++)
{
- if (argv[i][0] == '-')
- continue;
- if (generate_service(argv[i], force))
- do_hosts(argv[i]);
+ if (generate_service(services[i], force))
+ {
+ do_hosts(services[i]);
+ free(services[i]);
+ }
}
exit(0);
}
@@ -184,6 +199,10 @@
/* Someone might try to run a DCM from the command line while the
* regular one is running, which will bypass the "interval" test.
* Check inprogress to make sure they don't stomp on themselves.
+ *
+ * Note that there is still a race condition here, and this doesn't
+ * absolutely prevent 2 DCMs from stepping on one another, but it
+ * does reduce the window of vulnerability greatly.
*/
if (inprogress == 1)
{
@@ -305,6 +324,9 @@
strtrim(target);
strtrim(script);
+ /* Rudimentary locking. Doesn't eliminate the possibility of 2 DCMs
+ * stepping on one another, but makes it harder.
+ */
if (inprogress == 1)
{
com_err(whoami, 0, "DCM for service `%s' already in progress", name);
@@ -337,6 +359,8 @@
EXEC SQL SELECT inprogress INTO :inprogress FROM serverhosts
WHERE service = UPPER(:service) AND mach_id = :mid;
+ /* Check if someone got here before we did.
+ * There's still a race condition here, but it's a small one. */
if (inprogress == 1)
{
com_err(whoami, 0, "DCM for service `%s' to host `%s' already in progress", service, name);
