[Cluster-devel] Re: [PATCH 1/3] NLM add resume procfs file
Wendy Cheng <[email protected]>
| Newsgroups | com.redhat.cluster-devel,org.kernel.vger.linux-nfs |
|---|---|
| Message-ID | <[email protected]> |
J. Bruce Fields wrote: >> + int rc; >> + >> + rc = failover_parse_ip(file, buf, size, &server_ip); >> + if (rc < 0) >> + return rc; >> >> return nlmsvc_failover_ip(server_ip); >> } >> > > Looks great, but it would fit more logically with the previous patch. > (If you know you're going to end up using this code in two places, may > as well write it that way from the start.) > The original unlock patch did have a shared routine for this purpose. After review, its code structure got changed a little bit. Since the revised version has non-trivial amount of testing efforts behind it, I think it is better to do the change here, instead of the well-tested unlock patch. On the other hand, I cut the resume patch into three pieces mostly for review purpose. Do you think it would be easier (for your git tree works) that I combine these three small patches into a big resume patch after all the review comments are incorporated into the code ? -- Wendy