On Mon, 14 Mar 2011 15:26:45 +0200
Or Gerlitz <ogerlitz@xxxxxxxxxxxx> wrote:
> From: Roland Friedwagner <roland.friedwagner@xxxxxxxx>
>
> commit e2f95b537 "semaphore key clash with Open-iSCSI initiator" didn't
> address a further clash introduced when multiple tgt instances are
> running. Address that by creating the IPC semaphore keyid based on
> the managment socket file using ftok(3).
>
> Signed-off-by: Roland Friedwagner <roland.friedwagner@xxxxxxxx>
> Signed-off-by: Or Gerlitz <ogerlitz@xxxxxxxxxxxx>
>
> ---
> usr/log.c | 15 +++++++++++++--
> usr/mgmt.c | 9 +++++----
> usr/tgtadm.c | 6 +++---
> 3 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/usr/log.c b/usr/log.c
> index 06cd8bf..2f0c4d4 100644
> --- a/usr/log.c
> +++ b/usr/log.c
> @@ -33,7 +33,6 @@
>
> #include "log.h"
>
> -#define SEMKEY 0x54475444L /* TGTD */
> #define LOGDBG 0
>
> #if LOGDBG
> @@ -50,6 +49,8 @@ static pid_t pid;
> static int logarea_init (int size)
> {
> int shmid;
> + extern char mgmt_path[];
> + key_t semkey;
>
> logdbg(stderr,"enter logarea_init\n");
>
> @@ -109,13 +110,23 @@ static int logarea_init (int size)
>
> shmctl(shmid, IPC_RMID, NULL);
>
> - if ((la->semid = semget(SEMKEY, 1, 0666 | IPC_CREAT)) < 0) {
> + if ((semkey = ftok(mgmt_path, 'a')) < 0) {
> + syslog(LOG_ERR, "semkey failed %d", errno);
> + return 1;
> + }
> +
> + if ((semget(semkey, 0, IPC_EXCL)) > 0) {
> + /* Semkey may exists after SIGKILL tgtd */
> + syslog(LOG_WARNING, "semkey 0x%x already exists", semkey);
> + }
> + if ((la->semid = semget(semkey, 1, 0666 | IPC_CREAT)) < 0) {
> syslog(LOG_ERR, "semget failed %d", errno);
> shmdt(la->buff);
> shmdt(la->start);
> shmdt(la);
> return 1;
> }
> + syslog(LOG_INFO, "semkey 0x%x", semkey);
>
> la->semarg.val=1;
> if (semctl(la->semid, 0, SETVAL, la->semarg) < 0) {
> diff --git a/usr/mgmt.c b/usr/mgmt.c
> index 5a6a03b..fb703a6 100644
> --- a/usr/mgmt.c
> +++ b/usr/mgmt.c
> @@ -59,6 +59,7 @@ struct mgmt_task {
> };
>
> static int ipc_fd;
> +char mgmt_path[256];
>
> static void set_show_results(struct tgtadm_rsp *rsp, int *err)
> {
> @@ -567,7 +568,7 @@ int ipc_init(void)
> extern short control_port;
> int fd, err;
> struct sockaddr_un addr;
> - char path[256];
> + extern char mgmt_path[];
No need?
> fd = socket(AF_LOCAL, SOCK_STREAM, 0);
> if (fd < 0) {
> @@ -575,11 +576,11 @@ int ipc_init(void)
> return -1;
> }
>
> - sprintf(path, "%s.%d", TGT_IPC_NAMESPACE, control_port);
> - unlink(path);
> + sprintf(mgmt_path, "%s.%d", TGT_IPC_NAMESPACE, control_port);
> + unlink(mgmt_path);
> memset(&addr, 0, sizeof(addr));
> addr.sun_family = AF_LOCAL;
> - strncpy(addr.sun_path, path, sizeof(addr.sun_path));
> + strncpy(addr.sun_path, mgmt_path, sizeof(addr.sun_path));
>
> err = bind(fd, (struct sockaddr *) &addr, sizeof(addr));
> if (err) {
> diff --git a/usr/tgtadm.c b/usr/tgtadm.c
> index e81a419..9821917 100644
> --- a/usr/tgtadm.c
> +++ b/usr/tgtadm.c
> @@ -190,7 +190,7 @@ static int ipc_mgmt_connect(int *fd)
> {
> int err;
> struct sockaddr_un addr;
> - char path[256];
> + char mgmt_path[256];
>
> *fd = socket(AF_LOCAL, SOCK_STREAM, 0);
> if (*fd < 0) {
> @@ -200,8 +200,8 @@ static int ipc_mgmt_connect(int *fd)
>
> memset(&addr, 0, sizeof(addr));
> addr.sun_family = AF_LOCAL;
> - sprintf(path, "%s.%d", TGT_IPC_NAMESPACE, control_port);
> - strncpy(addr.sun_path, path, sizeof(addr.sun_path));
> + sprintf(mgmt_path, "%s.%d", TGT_IPC_NAMESPACE, control_port);
> + strncpy(addr.sun_path, mgmt_path, sizeof(addr.sun_path));
Why do we need to rename path to mgmt_path in tgtadm.c?
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
