Messages in this thread | | | Date | Mon, 25 Apr 2005 20:24:09 +0200 (CEST) | From | Jesper Juhl <> | Subject | Re: [PATCH 3/7] dlm: recovery |
| |
On Mon, 25 Apr 2005, David Teigland wrote:
> > When a node is removed from a lockspace, recovery is required for that > lockspace on all the remaining lockspace members. Recovery involves: a > full rebuild of the distributed resource directory, selecting a new master > node for locks/resources previously mastered on the removed node, and > rebuilding master-copy locks on newly selected masters. > > Signed-Off-By: Dave Teigland <teigland@redhat.com> > Signed-Off-By: Patrick Caulfield <pcaulfie@redhat.com> > > ---
[...]
> +static void receive_rcom_status(struct dlm_ls *ls, struct dlm_rcom *rc_in) > +{ > + struct dlm_rcom *rc; > + struct dlm_mhandle *mh; > + int error, nodeid = rc_in->rc_header.h_nodeid; > + > + error = create_rcom(ls, nodeid, DLM_RCOM_STATUS_REPLY, 0, &rc, &mh); > + rc->rc_result = make_status(ls); > + > + error = send_rcom(ls, mh, rc); > +}
This last assignment seems a bit pointless since you never use the value stored in `error' for anything. Shouldn't you be testing `error' at this point and take appropriate action? if not, then why bother assigning the value in the first place. The same comment goes for the assignment a few lines above. Either use the return value or just kill off the local variable `error' alltogether and just call create_rcom() and send_rcom() and throw away the result... I don't know what's appropriate here, but in any case the current code is a bit silly.
> +static void receive_rcom_names(struct dlm_ls *ls, struct dlm_rcom *rc_in) > +{ > + struct dlm_rcom *rc; > + struct dlm_mhandle *mh; > + int error, inlen, outlen; [...] > + error = create_rcom(ls, nodeid, DLM_RCOM_NAMES_REPLY, outlen, &rc, &mh); > + > + error = dlm_copy_master_names(ls, rc_in->rc_buf, inlen, rc->rc_buf, > + outlen, nodeid); > + > + error = send_rcom(ls, mh, rc); > +} Some more seemingly pointless assignments of values to `error' that are never used.
> +int dlm_send_rcom_lookup(struct dlm_rsb *r, int dir_nodeid) > +{ > + struct dlm_rcom *rc; > + struct dlm_mhandle *mh; > + struct dlm_ls *ls = r->res_ls; > + int error; > + > + error = create_rcom(ls, dir_nodeid, DLM_RCOM_LOOKUP, r->res_length, > + &rc, &mh); > + memcpy(rc->rc_buf, r->res_name, r->res_length); > + rc->rc_id = (unsigned long) r; > + > + error = send_rcom(ls, mh, rc); > + return 0; > +} Again these assignments to a local `error' variable that's never used.
> +static void receive_rcom_lookup(struct dlm_ls *ls, struct dlm_rcom *rc_in) > +{ > + struct dlm_rcom *rc; > + struct dlm_mhandle *mh; > + int error, ret_nodeid, nodeid = rc_in->rc_header.h_nodeid; > + int len = rc_in->rc_header.h_length - sizeof(struct dlm_rcom); > + > + error = dlm_dir_lookup(ls, nodeid, rc_in->rc_buf, len, &ret_nodeid); > + > + error = create_rcom(ls, nodeid, DLM_RCOM_LOOKUP_REPLY, 0, &rc, &mh); > + rc->rc_result = ret_nodeid; > + rc->rc_id = rc_in->rc_id; > + > + error = send_rcom(ls, mh, rc); > +} Yet more error assignments that I don't see the point of. There are more of these, but you can find the rest yourself :)
Don't have time right now to look at more code, so I'll stop here for now...
-- Jesper Juhl
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |