src/resource.c | 34 +++++++++++++++++++++++++++++----- src/sanlock_resource.h | 31 ++++++++++++++++++++++++------- 2 files changed, 53 insertions(+), 12 deletions(-)
New commits: commit 34402f8647015862448ee2f0b030b2fbff982d56 Author: David Teigland teigland@redhat.com Date: Mon Dec 22 11:13:29 2014 -0600
sanlock: fix adopting shared orphan lock
Adopting a shared orphan lock was missing some initialization (which was already being done for the case of adopting ex orphan.)
Signed-off-by: David Teigland teigland@redhat.com
diff --git a/src/resource.c b/src/resource.c index 79dd415..15d88a9 100644 --- a/src/resource.c +++ b/src/resource.c @@ -1408,6 +1408,16 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags, list_add(&token->list, &r->tokens); list_move(&r->list, &resources_held); pthread_mutex_unlock(&resource_mutex); + + /* do this to initialize some token fields */ + rv = open_disks(token->disks, token->r.num_disks); + if (rv < 0) { + /* TODO: what parts above need to be undone? */ + log_errot(token, "acquire_token sh orphan open error %d", rv); + release_token_nodisk(task, token); + return rv; + } + close_disks(token->disks, token->r.num_disks); return SANLK_OK; }
commit dc10ec4fb86fb256fd5e5096823f9c64cb22c697 Author: David Teigland teigland@redhat.com Date: Thu Dec 18 13:36:14 2014 -0600
sanlock: add ORPHAN_ONLY flag for acquire
The existing ORPHAN flag will acquire an orphan lock if it exists, otherwise will acquire a new lock.
The new ORPHAN_ONLY flag will acquire an orphan lock if it exists, but will not acquire a new lock.
Signed-off-by: David Teigland teigland@redhat.com
diff --git a/src/resource.c b/src/resource.c index 8e94a63..79dd415 100644 --- a/src/resource.c +++ b/src/resource.c @@ -1318,6 +1318,8 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags, uint32_t new_num_hosts = 0; int sh_retries = 0; int live_count = 0; + int allow_orphan = 0; + int only_orphan = 0; int rv;
if (token->acquire_flags & SANLK_RES_LVER) @@ -1325,6 +1327,11 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags, if (token->acquire_flags & SANLK_RES_NUM_HOSTS) new_num_hosts = token->acquire_data32;
+ if (cmd_flags & (SANLK_ACQUIRE_ORPHAN | SANLK_ACQUIRE_ORPHAN_ONLY)) + allow_orphan = 1; + if (cmd_flags & SANLK_ACQUIRE_ORPHAN_ONLY) + only_orphan = 1; + pthread_mutex_lock(&resource_mutex);
/* @@ -1368,7 +1375,7 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags, /* caller did not ask for orphan, but an orphan exists */
r = find_resource(token, &resources_orphan); - if (r && !(cmd_flags & SANLK_ACQUIRE_ORPHAN)) { + if (r && !allow_orphan) { log_errot(token, "acquire_token found orphan"); pthread_mutex_unlock(&resource_mutex); return -EUCLEAN; @@ -1376,7 +1383,7 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags,
/* caller asked for exclusive orphan, but a shared orphan exists */
- if (r && (cmd_flags & SANLK_ACQUIRE_ORPHAN) && + if (r && allow_orphan && (r->flags & R_SHARED) && !(token->acquire_flags & SANLK_RES_SHARED)) { log_errot(token, "acquire_token orphan is shared"); pthread_mutex_unlock(&resource_mutex); @@ -1385,7 +1392,7 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags,
/* caller asked for a shared orphan, but an exclusive orphan exists */
- if (r && (cmd_flags & SANLK_ACQUIRE_ORPHAN) && + if (r && allow_orphan && !(r->flags & R_SHARED) && (token->acquire_flags & SANLK_RES_SHARED)) { log_errot(token, "acquire_token orphan is exclusive"); pthread_mutex_unlock(&resource_mutex); @@ -1394,7 +1401,7 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags,
/* caller asked for shared orphan, and a shared orphan exists */
- if (r && (cmd_flags & SANLK_ACQUIRE_ORPHAN) && + if (r && allow_orphan && (r->flags & R_SHARED) && (token->acquire_flags & SANLK_RES_SHARED)) { log_token(token, "acquire_token adopt shared orphan"); token->resource = r; @@ -1406,7 +1413,7 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags,
/* caller asked for exclusive orphan, and an exclusive orphan exists */
- if (r && (cmd_flags & SANLK_ACQUIRE_ORPHAN) && + if (r && allow_orphan && !(r->flags & R_SHARED) && !(token->acquire_flags & SANLK_RES_SHARED)) { log_token(token, "acquire_token adopt orphan"); token->r.lver = r->leader.lver; @@ -1428,6 +1435,13 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags, return SANLK_OK; }
+ /* caller only wants to acquire an orphan */ + + if (cmd_flags & only_orphan) { + pthread_mutex_unlock(&resource_mutex); + return -ENOENT; + } + /* * The resource does not exist, so create it. */ diff --git a/src/sanlock_resource.h b/src/sanlock_resource.h index 635eec9..624a7d0 100644 --- a/src/sanlock_resource.h +++ b/src/sanlock_resource.h @@ -21,16 +21,33 @@ */
/* restrict flags */ -#define SANLK_RESTRICT_ALL 0x00000001 -#define SANLK_RESTRICT_SIGKILL 0x00000002 -#define SANLK_RESTRICT_SIGTERM 0x00000004 +#define SANLK_RESTRICT_ALL 0x00000001 +#define SANLK_RESTRICT_SIGKILL 0x00000002 +#define SANLK_RESTRICT_SIGTERM 0x00000004
/* killpath flags */ -#define SANLK_KILLPATH_PID 0x00000001 +#define SANLK_KILLPATH_PID 0x00000001
-/* acquire flags */ -#define SANLK_ACQUIRE_LVB 0x00000001 -#define SANLK_ACQUIRE_ORPHAN 0x00000002 +/* + * acquire flags + * + * SANLK_ACQUIRE_LVB + * Enable the use of an LVB with the lock. + * + * SANLK_ACQUIRE_ORPHAN + * If the lock already exists as an orphan, + * then acquire it. Otherwise, acquire a + * new lock as usual. + * + * SANLK_ACQUIRE_ORPHAN_ONLY + * If the lock already exists as an orphan, + * then acquire it. Otherwise, do not acquire + * a lock at all and return -ENOENT. + */ + +#define SANLK_ACQUIRE_LVB 0x00000001 +#define SANLK_ACQUIRE_ORPHAN 0x00000002 +#define SANLK_ACQUIRE_ORPHAN_ONLY 0x00000004
/* * release flags
----- Original Message -----
From: "David Teigland" teigland@fedoraproject.org To: sanlock-devel@lists.fedorahosted.org Sent: Monday, December 22, 2014 7:15:05 PM Subject: 2 commits - src/resource.c src/sanlock_resource.h
src/resource.c | 34 +++++++++++++++++++++++++++++----- src/sanlock_resource.h | 31 ++++++++++++++++++++++++------- 2 files changed, 53 insertions(+), 12 deletions(-)
New commits: commit 34402f8647015862448ee2f0b030b2fbff982d56 Author: David Teigland teigland@redhat.com Date: Mon Dec 22 11:13:29 2014 -0600
sanlock: fix adopting shared orphan lock Adopting a shared orphan lock was missing some initialization (which was already being done for the case of adopting ex orphan.)
Can you explain what is the consequence of this issue?
Signed-off-by: David Teigland <teigland@redhat.com>
diff --git a/src/resource.c b/src/resource.c index 79dd415..15d88a9 100644 --- a/src/resource.c +++ b/src/resource.c @@ -1408,6 +1408,16 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags, list_add(&token->list, &r->tokens); list_move(&r->list, &resources_held); pthread_mutex_unlock(&resource_mutex);
/* do this to initialize some token fields */
rv = open_disks(token->disks, token->r.num_disks);
if (rv < 0) {
/* TODO: what parts above need to be undone? */
log_errot(token, "acquire_token sh orphan open error %d", rv);
log_errot?
release_token_nodisk(task, token);
return rv;
}
return SANLK_OK; }close_disks(token->disks, token->r.num_disks);
commit dc10ec4fb86fb256fd5e5096823f9c64cb22c697 Author: David Teigland teigland@redhat.com Date: Thu Dec 18 13:36:14 2014 -0600
sanlock: add ORPHAN_ONLY flag for acquire The existing ORPHAN flag will acquire an orphan lock if it exists, otherwise will acquire a new lock. The new ORPHAN_ONLY flag will acquire an orphan lock if it exists, but will not acquire a new lock.
Can you explain what is the consequence of this issue/fix?
Signed-off-by: David Teigland <teigland@redhat.com>
diff --git a/src/resource.c b/src/resource.c index 8e94a63..79dd415 100644 --- a/src/resource.c +++ b/src/resource.c @@ -1318,6 +1318,8 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags, uint32_t new_num_hosts = 0; int sh_retries = 0; int live_count = 0;
int allow_orphan = 0;
int only_orphan = 0; int rv;
if (token->acquire_flags & SANLK_RES_LVER)
@@ -1325,6 +1327,11 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags, if (token->acquire_flags & SANLK_RES_NUM_HOSTS) new_num_hosts = token->acquire_data32;
if (cmd_flags & (SANLK_ACQUIRE_ORPHAN | SANLK_ACQUIRE_ORPHAN_ONLY))
allow_orphan = 1;
if (cmd_flags & SANLK_ACQUIRE_ORPHAN_ONLY)
only_orphan = 1;
pthread_mutex_lock(&resource_mutex);
/*
@@ -1368,7 +1375,7 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags, /* caller did not ask for orphan, but an orphan exists */
r = find_resource(token, &resources_orphan);
- if (r && !(cmd_flags & SANLK_ACQUIRE_ORPHAN)) {
- if (r && !allow_orphan) { log_errot(token, "acquire_token found orphan");
log_errot? (there are more instances bellow)
pthread_mutex_unlock(&resource_mutex); return -EUCLEAN;
@@ -1376,7 +1383,7 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags,
/* caller asked for exclusive orphan, but a shared orphan exists */
- if (r && (cmd_flags & SANLK_ACQUIRE_ORPHAN) &&
- if (r && allow_orphan && (r->flags & R_SHARED) && !(token->acquire_flags & SANLK_RES_SHARED)) { log_errot(token, "acquire_token orphan is shared"); pthread_mutex_unlock(&resource_mutex);
@@ -1385,7 +1392,7 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags,
/* caller asked for a shared orphan, but an exclusive orphan exists */
- if (r && (cmd_flags & SANLK_ACQUIRE_ORPHAN) &&
- if (r && allow_orphan && !(r->flags & R_SHARED) && (token->acquire_flags & SANLK_RES_SHARED)) { log_errot(token, "acquire_token orphan is exclusive"); pthread_mutex_unlock(&resource_mutex);
@@ -1394,7 +1401,7 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags,
/* caller asked for shared orphan, and a shared orphan exists */
- if (r && (cmd_flags & SANLK_ACQUIRE_ORPHAN) &&
- if (r && allow_orphan && (r->flags & R_SHARED) && (token->acquire_flags & SANLK_RES_SHARED)) { log_token(token, "acquire_token adopt shared orphan"); token->resource = r;
@@ -1406,7 +1413,7 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags,
/* caller asked for exclusive orphan, and an exclusive orphan exists */
- if (r && (cmd_flags & SANLK_ACQUIRE_ORPHAN) &&
- if (r && allow_orphan && !(r->flags & R_SHARED) && !(token->acquire_flags & SANLK_RES_SHARED)) { log_token(token, "acquire_token adopt orphan"); token->r.lver = r->leader.lver;
@@ -1428,6 +1435,13 @@ int acquire_token(struct task *task, struct token *token, uint32_t cmd_flags, return SANLK_OK; }
- /* caller only wants to acquire an orphan */
- if (cmd_flags & only_orphan) {
pthread_mutex_unlock(&resource_mutex);
return -ENOENT;
- }
- /*
*/
- The resource does not exist, so create it.
diff --git a/src/sanlock_resource.h b/src/sanlock_resource.h index 635eec9..624a7d0 100644 --- a/src/sanlock_resource.h +++ b/src/sanlock_resource.h @@ -21,16 +21,33 @@ */
/* restrict flags */ -#define SANLK_RESTRICT_ALL 0x00000001 -#define SANLK_RESTRICT_SIGKILL 0x00000002 -#define SANLK_RESTRICT_SIGTERM 0x00000004 +#define SANLK_RESTRICT_ALL 0x00000001 +#define SANLK_RESTRICT_SIGKILL 0x00000002 +#define SANLK_RESTRICT_SIGTERM 0x00000004
Why change the indentation?
/* killpath flags */ -#define SANLK_KILLPATH_PID 0x00000001 +#define SANLK_KILLPATH_PID 0x00000001
Why change the indentation?
-/* acquire flags */ -#define SANLK_ACQUIRE_LVB 0x00000001 -#define SANLK_ACQUIRE_ORPHAN 0x00000002 +/*
- acquire flags
- SANLK_ACQUIRE_LVB
- Enable the use of an LVB with the lock.
- SANLK_ACQUIRE_ORPHAN
- If the lock already exists as an orphan,
- then acquire it. Otherwise, acquire a
- new lock as usual.
- SANLK_ACQUIRE_ORPHAN_ONLY
- If the lock already exists as an orphan,
- then acquire it. Otherwise, do not acquire
- a lock at all and return -ENOENT.
- */
+#define SANLK_ACQUIRE_LVB 0x00000001 +#define SANLK_ACQUIRE_ORPHAN 0x00000002 +#define SANLK_ACQUIRE_ORPHAN_ONLY 0x00000004
/*
- release flags
sanlock-devel mailing list sanlock-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sanlock-devel
sanlock: fix adopting shared orphan lock Adopting a shared orphan lock was missing some initialization (which was already being done for the case of adopting ex orphan.)
Can you explain what is the consequence of this issue?
Releasing the adopted lock returns an error. Do you have any plans to use orphan locks?
log_errot(token, "acquire_token sh orphan open error %d", rv);
log_errot?
defined in log.h
sanlock: add ORPHAN_ONLY flag for acquire The existing ORPHAN flag will acquire an orphan lock if it exists, otherwise will acquire a new lock. The new ORPHAN_ONLY flag will acquire an orphan lock if it exists, but will not acquire a new lock.
Can you explain what is the consequence of this issue/fix?
You can't make a request to only adopt a lock without it.
log_errot? (there are more instances bellow)
What's the question?
-#define SANLK_RESTRICT_ALL 0x00000001 -#define SANLK_RESTRICT_SIGKILL 0x00000002 -#define SANLK_RESTRICT_SIGTERM 0x00000004 +#define SANLK_RESTRICT_ALL 0x00000001 +#define SANLK_RESTRICT_SIGKILL 0x00000002 +#define SANLK_RESTRICT_SIGTERM 0x00000004
Why change the indentation?
for appearance
-#define SANLK_KILLPATH_PID 0x00000001 +#define SANLK_KILLPATH_PID 0x00000001
Why change the indentation?
for appearance
----- Original Message -----
From: "David Teigland" teigland@redhat.com To: "Nir Soffer" nsoffer@redhat.com Cc: sanlock-devel@lists.fedorahosted.org Sent: Monday, December 22, 2014 7:56:04 PM Subject: Re: 2 commits - src/resource.c src/sanlock_resource.h
sanlock: fix adopting shared orphan lock Adopting a shared orphan lock was missing some initialization (which was already being done for the case of adopting ex orphan.)
Can you explain what is the consequence of this issue?
Releasing the adopted lock returns an error. Do you have any plans to use orphan locks?
I don't know what is orphan lock :-)
I wonder if this effect vdsm in any way - do we have a related bug?
log_errot(token, "acquire_token sh orphan open error %d", rv);
log_errot?
defined in log.h
Ok, it looks like a typing error. log_error_tok or log_err_t would be more clear.
sanlock: add ORPHAN_ONLY flag for acquire The existing ORPHAN flag will acquire an orphan lock if it exists, otherwise will acquire a new lock. The new ORPHAN_ONLY flag will acquire an orphan lock if it exists, but will not acquire a new lock.
Can you explain what is the consequence of this issue/fix?
You can't make a request to only adopt a lock without it.
log_errot? (there are more instances bellow)
What's the question?
-#define SANLK_RESTRICT_ALL 0x00000001 -#define SANLK_RESTRICT_SIGKILL 0x00000002 -#define SANLK_RESTRICT_SIGTERM 0x00000004 +#define SANLK_RESTRICT_ALL 0x00000001 +#define SANLK_RESTRICT_SIGKILL 0x00000002 +#define SANLK_RESTRICT_SIGTERM 0x00000004
Why change the indentation?
for appearance
Sure, but the indentation is not consistent now - here in the mail it looks ok, but in the patch RESTRICT_ALL value is indented less than the other values.
Nir
I don't know what is orphan lock :-)
I wonder if this effect vdsm in any way - do we have a related bug?
vdsm doesn't use persistent/orphan locks. I introduced them recently for sanlk_reset and lvmlockd (commit e66d5d11). Acquiring/adopting orphan locks hasn't been used until the past week when I added that to lvmlockd (since this is the first time it's been used, it's not surprising to find some issues.)
Sure, but the indentation is not consistent now - here in the mail it looks ok, but in the patch RESTRICT_ALL value is indented less than the other values.
Alignment sometimes appears off in patch output because of the +, but if you look in the file it's correct.
sanlock-devel@lists.fedorahosted.org