[Ocfs2-tools-devel] [PATCH] o2hbmonitor: add semaphore

[Sunil Mushran]

On 01/03/2011 04:35 PM, Srinivas Eeda wrote:
> add semaphore to limit to one o2hbmonitor per node

Allow only one active instance of the monitor utility.

>
> +static int islocked(void)
> +{
> +	int semid;
> +	struct sembuf trylock[1] = {
> +		{.sem_num = 0, .sem_op = 0, .sem_flg = SEM_UNDO|IPC_NOWAIT},
> +	};
> +
> +	semid = semget(O2HB_SEM_MAGIC_KEY, 1, 0);
> +	if (semid<  0)
> +		return 0;
> +	if (semop(semid, trylock, 1)<  0)
> +		return 1;
> +	return 0;
> +}
> +
> +static int getlock(void)
> +{
> +	int ret, semid, vals[1] = { 0 };
> +	struct sembuf trylock[2] = {
> +		{.sem_num = 0, .sem_op = 0, .sem_flg = SEM_UNDO|IPC_NOWAIT},
> +		{.sem_num = 0, .sem_op = 1, .sem_flg = SEM_UNDO|IPC_NOWAIT},
> +	};
> +
> +	semid = semget(O2HB_SEM_MAGIC_KEY, 1, 0);
> +	if (semid<  0) {
> +		semid = semget(O2HB_SEM_MAGIC_KEY, 1,
> +			       IPC_CREAT|IPC_EXCL|S_IRUSR);
> +		if (semid<  0)
> +			goto out;
> +		semctl(semid, 0, SETALL, vals);
> +		if (semop(semid, trylock, 2)<  0)
> +			goto out;
> +		else
> +			return 0;
> +	}
> +	if (semop(semid, trylock, 2)<  0)
> +		goto out;
> +	return 0;
> +out:
> +	ret = errno;
> +	if (ret == EAGAIN) {
> +		syslog(LOG_WARNING, "Another instance of %s is already running."
> +		       " Aborting.\n", progname);
> +	}
> +	return errno;

+	if (errno == EAGAIN) {
+		syslog(LOG_WARNING, "Another instance of %s is already running."
+		       " Aborting.\n", progname);
+	}
+	return errno == EAGAIN;


> +}
> +
>   static void usage(void)
>   {
>   	fprintf(stderr, "usage: %s [-w percent] -[ivV]\n", progname);
> @@ -352,6 +403,12 @@ int main(int argc, char **argv)
>   	if (version)
>   		show_version();
>
> +	if (islocked()) {
> +		fprintf(stderr, "Another instance of %s is already running. "
> +		       "Aborting.\n", progname);
> +		return 1;
> +	}
> +
>   	if (!interactive) {
>   		ret = daemon(0, verbose);
>   		if (ret)
> @@ -360,6 +417,13 @@ int main(int argc, char **argv)
>   	}
>
>   	openlog(progname, LOG_CONS|LOG_NDELAY, LOG_DAEMON);
> +	ret = getlock();
> +	if (ret == EAGAIN) {

+	if (ret) {


A cleaner interface would be if getlock() returns 0 if successful
and 1 if unsuccessful. EAGAIN is just an implementation detail
that should not be allowed to bubble out.

> +		closelog();
> +		return ret;
> +	}
> +
> +	syslog(LOG_INFO, "Starting\n");
>   	monitor();
>   	closelog();
>