diff options
-rw-r--r-- | daemons/lvmlockd/lvmlockctl.c | 7 | ||||
-rw-r--r-- | daemons/lvmlockd/lvmlockd-core.c | 71 | ||||
-rw-r--r-- | daemons/lvmlockd/lvmlockd-dlm.c | 14 | ||||
-rw-r--r-- | daemons/lvmlockd/lvmlockd-internal.h | 2 | ||||
-rw-r--r-- | daemons/lvmlockd/lvmlockd-sanlock.c | 103 | ||||
-rw-r--r-- | man/lvmlockd.8.in | 2 | ||||
-rw-r--r-- | tools/lvchange.c | 2 | ||||
-rw-r--r-- | tools/polldaemon.c | 2 | ||||
-rw-r--r-- | tools/vgchange.c | 2 |
9 files changed, 46 insertions, 159 deletions
diff --git a/daemons/lvmlockd/lvmlockctl.c b/daemons/lvmlockd/lvmlockctl.c index 17b122f9d..b8ab9ed11 100644 --- a/daemons/lvmlockd/lvmlockctl.c +++ b/daemons/lvmlockd/lvmlockctl.c @@ -311,6 +311,9 @@ static daemon_reply _lvmlockd_send(const char *req_name, ...) return repl; } +/* See the same in lib/locking/lvmlockd.c */ +#define NO_LOCKD_RESULT -1000 + static int _lvmlockd_result(daemon_reply reply, int *result) { int reply_result; @@ -327,9 +330,7 @@ static int _lvmlockd_result(daemon_reply reply, int *result) return 0; } - /* FIXME: using -1000 is dumb */ - - reply_result = daemon_reply_int(reply, "op_result", -1000); + reply_result = daemon_reply_int(reply, "op_result", NO_LOCKD_RESULT); if (reply_result == -1000) { log_error("lvmlockd_result no op_result"); return 0; diff --git a/daemons/lvmlockd/lvmlockd-core.c b/daemons/lvmlockd/lvmlockd-core.c index 7c962c8eb..cd48a90d9 100644 --- a/daemons/lvmlockd/lvmlockd-core.c +++ b/daemons/lvmlockd/lvmlockd-core.c @@ -1620,7 +1620,7 @@ static void res_process(struct lockspace *ls, struct resource *r, add_client_result(act); } else { /* persistent lock is sh, transient request is ex */ - /* TODO: can we remove this case? do a convert here? */ + /* FIXME: can we remove this case? do a convert here? */ log_debug("res_process %s existing persistent lock new transient", r->name); act->result = -EEXIST; list_del(&act->list); @@ -1654,7 +1654,7 @@ static void res_process(struct lockspace *ls, struct resource *r, continue; if (lk->mode != act->mode) { - /* TODO: convert and change to persistent? */ + /* FIXME: convert and change to persistent? */ log_debug("res_process %s existing transient lock new persistent", r->name); act->result = -EEXIST; list_del(&act->list); @@ -2342,7 +2342,7 @@ static int vg_ls_name(const char *vg_name, char *ls_name) return 0; } -/* TODO: add mutex for gl_lsname_ ? */ +/* FIXME: add mutex for gl_lsname_ ? */ static int gl_ls_name(char *ls_name) { @@ -2811,10 +2811,7 @@ static int for_each_lockspace(int do_stop, int do_free, int do_force) pthread_join(ls->thread, NULL); list_del(&ls->list); - /* TODO: remove this if unneeded */ - if (!list_empty(&ls->actions)) - log_error("TODO: free ls actions"); - + /* In future we may need to free ls->actions here */ free_ls_resources(ls); list_add(&ls->list, &lockspaces_inactive); free_count++; @@ -4031,7 +4028,7 @@ static void client_recv_action(struct client *cl) op == LD_OP_DUMP_INFO || op == LD_OP_DUMP_LOG) { /* - * TODO: add the client command name to the hello messages + * FIXME: add the client command name to the hello messages * so it can be saved in cl->name here. */ @@ -4104,7 +4101,7 @@ static void client_recv_action(struct client *cl) if (cl_pid && cl_pid != cl->pid) log_error("client recv bad message pid %d client %d", cl_pid, cl->pid); - /* TODO: do this in hello message instead */ + /* FIXME: do this in hello message instead */ if (!cl->name[0] && cl_name) strncpy(cl->name, cl_name, MAX_NAME); @@ -4343,52 +4340,6 @@ static void close_client_thread(void) pthread_join(client_thread, NULL); } -#if 0 -static void setup_listener(void) -{ - struct sockaddr_un addr; - int rv, fd, ci; - - rv = lvmlockd_socket_address(&addr); - if (rv < 0) - return rv; - - fd = socket(AF_LOCAL, SOCK_STREAM, 0); - if (fd < 0) - return fd; - - unlink(addr.sun_path); - rv = bind(fd, (struct sockaddr *) &addr, sizeof(struct sockaddr_un)); - if (rv < 0) - goto exit_fail; - - rv = chmod(addr.sun_path, DEFAULT_SOCKET_MODE); - if (rv < 0) - goto exit_fail; - - rv = chown(addr.sun_path, com.uid, com.gid); - if (rv < 0) { - log_error("could not set socket %s permissions: %s", - addr.sun_path, strerror(errno)); - goto exit_fail; - } - - rv = listen(fd, 5); - if (rv < 0) - goto exit_fail; - - fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK); - - listen_pi = add_pollfd(fd); - - return 0; - -exit_fail: - close(fd); - return -1; -} -#endif - /* * Get a list of all VGs with a lockd type (sanlock|dlm) from lvmetad. * We'll match this list against a list of existing lockspaces that are @@ -4929,7 +4880,7 @@ static void adopt_locks(void) list_for_each_entry_safe(ls, lsafe, &vg_lockd, list) { if (!list_empty(&ls->resources)) { /* We should have found a lockspace. */ - /* TODO: add this ls and acquire locks for ls->resources? */ + /* add this ls and acquire locks for ls->resources? */ log_error("No lockspace %s %s found for VG %s with active LVs", ls->name, lm_str(ls->lm_type), ls->vg_name); } else { @@ -5249,7 +5200,7 @@ static void adopt_locks(void) if (act->mode == LD_LK_EX) { /* - * TODO: we probably want to check somehow that + * FIXME: we probably want to check somehow that * there's no lvm command still running that's * using this ex lock and changing things. */ @@ -5294,7 +5245,7 @@ static void adopt_locks(void) } - /* TODO: purge any remaining orphan locks in each rejoined ls? */ + /* FIXME: purge any remaining orphan locks in each rejoined ls? */ if (count_start_fail || count_adopt_fail) goto fail; @@ -5546,8 +5497,8 @@ static int main_loop(daemon_state *ds_arg) } pthread_mutex_unlock(&client_mutex); - /* TODO?: after set_dead, scan pollfd for last unused - slot and reduce pollfd_maxi */ + /* After set_dead, should we scan pollfd for + last unused slot and reduce pollfd_maxi? */ } } diff --git a/daemons/lvmlockd/lvmlockd-dlm.c b/daemons/lvmlockd/lvmlockd-dlm.c index 04336fd66..96b5b2be0 100644 --- a/daemons/lvmlockd/lvmlockd-dlm.c +++ b/daemons/lvmlockd/lvmlockd-dlm.c @@ -236,10 +236,11 @@ int lm_rem_lockspace_dlm(struct lockspace *ls, int free_vg) goto out; /* - * TODO: if free_vg is set, it means we are doing vgremove, - * and we may want to tell any other nodes to leave the lockspace. - * This is not really necessary since there should be no harm in - * having an unused lockspace sitting around. + * If free_vg is set, it means we are doing vgremove, and we may want + * to tell any other nodes to leave the lockspace. This is not really + * necessary since there should be no harm in having an unused + * lockspace sitting around. A new "notification lock" would need to + * be added with a callback to signal this. */ rv = dlm_release_lockspace(ls->name, lmd->dh, 1); @@ -403,7 +404,7 @@ static int lm_adopt_dlm(struct lockspace *ls, struct resource *r, int ld_mode, } /* - * TODO: for GL/VG locks we probably want to read the lvb, + * FIXME: For GL/VG locks we probably want to read the lvb, * especially if adopting an ex lock, because when we * release this adopted ex lock we may want to write new * lvb values based on the current lvb values (at lease @@ -478,7 +479,6 @@ int lm_lock_dlm(struct lockspace *ls, struct resource *r, int ld_mode, r->name, strlen(r->name), 0, NULL, NULL, NULL); if (rv == -EAGAIN) { - /* TODO: what case is this? what should be done? */ log_error("S %s R %s lock_dlm mode %d rv EAGAIN", ls->name, r->name, mode); return -EAGAIN; } @@ -557,7 +557,7 @@ int lm_convert_dlm(struct lockspace *ls, struct resource *r, r->name, strlen(r->name), 0, NULL, NULL, NULL); if (rv == -EAGAIN) { - /* TODO: what case is this? what should be done? */ + /* FIXME: When does this happen? Should something different be done? */ log_error("S %s R %s convert_dlm mode %d rv EAGAIN", ls->name, r->name, mode); return -EAGAIN; } diff --git a/daemons/lvmlockd/lvmlockd-internal.h b/daemons/lvmlockd/lvmlockd-internal.h index f5f7a399c..39085a6fc 100644 --- a/daemons/lvmlockd/lvmlockd-internal.h +++ b/daemons/lvmlockd/lvmlockd-internal.h @@ -11,7 +11,6 @@ #ifndef _LVM_LVMLOCKD_INTERNAL_H #define _LVM_LVMLOCKD_INTERNAL_H -/* TODO: figure out real restraints/requirements for these */ #define MAX_NAME 64 #define MAX_ARGS 64 @@ -184,7 +183,6 @@ struct lockspace { struct list_head actions; /* new client actions */ struct list_head resources; /* resource/lock state for gl/vg/lv */ - /* TODO: should probably be tree */ }; #define VAL_BLK_VERSION 0x0101 diff --git a/daemons/lvmlockd/lvmlockd-sanlock.c b/daemons/lvmlockd/lvmlockd-sanlock.c index fa7ad8ab2..85a52c880 100644 --- a/daemons/lvmlockd/lvmlockd-sanlock.c +++ b/daemons/lvmlockd/lvmlockd-sanlock.c @@ -82,8 +82,8 @@ * the error conditions are often not as simple as storage being lost and then * later connecting, will result in this option being too unreliable. * - * TODO: add a config option that we could use to select a different behavior - * than the default. Then implement one of the simpler options as a proof of + * Add a config option that we could use to select a different behavior than + * the default. Then implement one of the simpler options as a proof of * concept, which could be extended if needed. */ @@ -563,7 +563,7 @@ int lm_rename_vg_sanlock(char *ls_name, char *vg_name, uint32_t flags, char *vg_ if (daemon_test) return 0; - /* FIXME: remove this, device is not always ready for us here */ + /* FIXME: device is not always ready for us here */ sleep(1); align_size = sanlock_align(&disk); @@ -997,13 +997,13 @@ int lm_prepare_lockspace_sanlock(struct lockspace *ls) * activated before lvmlockd is asked to add the lockspace. * (sanlock needs to use the lv.) * - * TODO: can we ask something on the system to activate the - * sanlock lv or should we just require that vgchange be used - * to start sanlock vgs? - * Should sanlock lvs be "auto-activated"? + * In the future we might be able to ask something on the system + * to activate the sanlock lv from here, and with that we might be + * able to start sanlock VGs without requiring a + * vgchange --lock-start command. */ - /* FIXME: remove this, device is not always ready for us here */ + /* FIXME: device is not always ready for us here */ sleep(1); rv = stat(disk_path, &st); @@ -1186,7 +1186,7 @@ out: free(lms); ls->lm_data = NULL; - /* TODO: should we only clear gl_lsname when doing free_vg? */ + /* FIXME: should we only clear gl_lsname when doing free_vg? */ if (!strcmp(ls->name, gl_lsname_sanlock)) memset(gl_lsname_sanlock, 0, sizeof(gl_lsname_sanlock)); @@ -1194,62 +1194,6 @@ out: return 0; } -#if 0 -static int find_lv_offset(struct lockspace *ls, struct resource *r, - uint64_t *lv_args_offset) -{ - struct lm_sanlock *lms = (struct lm_sanlock *)ls->lm_data; - struct sanlk_resourced rd; - uint64_t offset; - int align_size; - int lv_count = 0; - int rv; - - memset(&rd, 0, sizeof(rd)); - - strncpy(rd.rs.lockspace_name, ls->name, SANLK_NAME_LEN); - rd.rs.num_disks = 1; - memcpy(rd.rs.disks[0].path, lms->ss.host_id_disk.path, SANLK_PATH_LEN); - - align_size = sanlock_align(&rd.rs.disks[0]); - if (align_size <= 0) { - log_error("find_lv_offset align error %d", align_size); - return -EINVAL; - } - - offset = align_size * LV_LOCK_BEGIN; - - while (1) { - rd.rs.disks[0].offset = offset; - - memset(rd.rs.name, 0, SANLK_NAME_LEN); - - rv = sanlock_read_resource(&rd.rs, 0); - if (rv == -EMSGSIZE || rv == -ENOSPCE) { - /* This indicates the end of the device is reached. */ - rv = -EMSGSIZE; - break; - } - - if (rv) { - log_error("S %s find_lv_offset read error %d offset %llu", - ls->name, rv, (unsigned long long)offset); - break; - } - - if (!strncmp(rd.rs.name, r->name, SANLK_NAME_LEN)) { - /* found it */ - *lv_args_offset = offset; - rv = 0; - break; - } - - offset += align_size; - } - return rv; -} -#endif - static int lm_add_resource_sanlock(struct lockspace *ls, struct resource *r) { struct lm_sanlock *lms = (struct lm_sanlock *)ls->lm_data; @@ -1281,7 +1225,7 @@ int lm_rem_resource_sanlock(struct lockspace *ls, struct resource *r) { struct rd_sanlock *rds = (struct rd_sanlock *)r->lm_data; - /* TODO: assert r->mode == UN or unlock if it's not? */ + /* FIXME: assert r->mode == UN or unlock if it's not? */ if (rds->vb) free(rds->vb); @@ -1384,8 +1328,6 @@ int lm_lock_sanlock(struct lockspace *ls, struct resource *r, int ld_mode, * It appears that sanlock_acquire returns EAGAIN when we request * a shared lock but the lock is held ex by another host. * There's no point in retrying this case, just return an error. - * - * TODO: verify the sanlock behavior here. */ log_debug("S %s R %s lock_san acquire mode %d rv EAGAIN", ls->name, r->name, ld_mode); *retry = 0; @@ -1443,8 +1385,6 @@ int lm_lock_sanlock(struct lockspace *ls, struct resource *r, int ld_mode, * We can't distinguish between the two cases above, * so if requesting a sh lock, retry a couple times, * otherwise don't. - * - * TODO: verify sanlock behavior here. */ log_debug("S %s R %s lock_san acquire mode %d rv %d", ls->name, r->name, ld_mode, rv); *retry = (ld_mode == LD_LK_SH) ? 1 : 0; @@ -1548,7 +1488,7 @@ int lm_convert_sanlock(struct lockspace *ls, struct resource *r, rv = sanlock_convert(lms->sock, -1, flags, rs); if (rv == -EAGAIN) { - /* TODO: what case is this? what should be done? */ + /* FIXME: When could this happen? Should something different be done? */ log_error("S %s R %s convert_san EAGAIN", ls->name, r->name); return -EAGAIN; } @@ -1703,19 +1643,16 @@ int lm_hosts_sanlock(struct lockspace *ls, int notify) free(hss); if (found_others && notify) { -#if 0 - struct sanlk_host_event he; - memset(&he, 0, sizeof(he)); - hm.host_id = 1; - hm.generation = 0; - hm.event = EVENT_VGSTOP; - sanlock_set_event(ls->name, &he, SANLK_SETEV_ALL_HOSTS); -#endif /* - * We'll need to retry for a while before all the hosts see - * this event and stop the vg. - * We'll need to register for events from the lockspace - * and add the registered fd to our poll set. + * We could use the sanlock event mechanism to notify lvmlockd + * on other hosts to stop this VG. lvmlockd would need to + * register for and listen for sanlock events in the main loop. + * The events are slow to propagate. We'd need to retry for a + * while before all the hosts see the event and stop the VG. + * sanlock_set_event(ls->name, &he, SANLK_SETEV_ALL_HOSTS); + * + * Wait to try this until there appears to be real value/interest + * in doing it. */ } diff --git a/man/lvmlockd.8.in b/man/lvmlockd.8.in index 6e80243a0..79a3218fe 100644 --- a/man/lvmlockd.8.in +++ b/man/lvmlockd.8.in @@ -545,7 +545,7 @@ locks if the PV holding the locks is lost. Contact the LVM group for help with this process. -.\" TODO: This is not clean or safe enough to suggest using without help. +.\" This is not clean or safe enough to suggest using without help. .\" .\" .SS recover from lost PV holding sanlock locks .\" diff --git a/tools/lvchange.c b/tools/lvchange.c index 05805b7be..994336934 100644 --- a/tools/lvchange.c +++ b/tools/lvchange.c @@ -994,7 +994,7 @@ static int _lvchange_single(struct cmd_context *cmd, struct logical_volume *lv, * Otherwise, the lv lock will be taken as non-persistent * and released when this command exits. * - * TODO: use "sh" if the options imply that the lvchange + * FIXME: use "sh" if the options imply that the lvchange * operation does not modify the LV. */ if (!lockd_lv(cmd, lv, "ex", 0)) { diff --git a/tools/polldaemon.c b/tools/polldaemon.c index 2ae4331b2..0544542b6 100644 --- a/tools/polldaemon.c +++ b/tools/polldaemon.c @@ -372,7 +372,7 @@ static int report_progress(struct cmd_context *cmd, struct poll_operation_id *id int ret; /* - * TODO: we don't really need to take the vg lock here, + * FIXME: we don't really need to take the vg lock here, * because we only report the progress on the same host * where the pvmove/lvconvert is happening. This means * that the local pvmove/lvconvert/lvpoll commands are diff --git a/tools/vgchange.c b/tools/vgchange.c index 4e2be3452..87954cd29 100644 --- a/tools/vgchange.c +++ b/tools/vgchange.c @@ -645,7 +645,7 @@ static int _vgchange_locktype(struct cmd_context *cmd, * lockd_free_vg_after(); */ if (is_lockd_type(vg->lock_type)) { - /* TODO */ + /* FIXME: implement full undoing of the lock_type */ log_error("Changing VG %s from lock type %s not yet allowed.", vg->name, vg->lock_type); return 0; |