Re: the evils of the CLD api
On Fri, 15 Jan 2010 18:40:44 -0800
Colin McCabe <cmccabe@xxxxxxxxxxxxxx> wrote:
> 1) when starting the cld session, the user code could register a
> "vtable" containing a bunch of different callbacks.
> That way all GET responses for the session could flow through
> my_get_response(), and so on.
I'm not sure if that would be much better than what we have now,
so I don't see a big pay-off from switching to it.
> I think a non-blocking API, in some form, should probably be
> available, though, in case people need it. We may not be able to
> anticipate all the uses that CLD is put to.
The easiest way to do this is to leave cldc intact.
> 2) provide a blocking API that users could take advantage of.
> I think for a lot of simple programs, programmers would be happy to
> spawn a thread per CLD session. In fact, I'm not aware of any programs
> that want to spawn a lot of cld sessions (maybe someone can point out
> one.)
Here's what I have for tonight:
commit e011b6c073d1dafd506dc36c5bb2d7ef122f65d7
Author: Master <zaitcev@xxxxxxxxxxxxxxxxxx>
Date: Fri Jan 15 23:26:08 2010 -0700
Checkpoint WIP for ncld.h.
diff --git a/include/ncld.h b/include/ncld.h
new file mode 100644
index 0000000..ed52f1d
--- /dev/null
+++ b/include/ncld.h
@@ -0,0 +1,58 @@
+#ifndef __NCLD_H__
+#define __NCLD_H__
+
+/*
+ * Copyright 2009 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; see the file COPYING. If not, write to
+ * the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+/*
+ * The ncld.h API is a replacement for cldc.h. Do not include both.
+ *
+ * We do not believe into making "internal" structures "opaque"
+ * with pointers to void. Therefore, this header might want include
+ * some legacy definitions or whatnot, but users do not need to.
+ */
+#include <glib.h>
+#include <cldc.h>
+
+struct ncld_fh {
+};
+
+struct ncld_sess {
+ char *host;
+ unsigned short port;
+ // GMutex *mutex; mutex = g_mutex_new(); g_mutex_lock(mutex);
+ GThread *thread;
+ struct cldc_udp *udp;
+ void (*event)(struct ncld_sess *, void *, unsigned int);
+ void *event_arg;
+};
+
+extern struct ncld_sess *ncld_sess_open(const char *host, const char *port,
+ int *error,
+ void (*event)(struct ncld_sess *, void *, unsigned int), void *ev_arg);
+extern struct ncld_fh *ncld_open(struct ncld_sess *s, const char *fname,
+ unsigned int mode, int *error, unsigned int events,
+ void (*event)(struct ncld_fh *, void *, unsigned int), void *ev_arg);
+extern long ncld_read(struct ncld_fh *, void *data, long len);
+extern long ncld_write(struct ncld_fh *, const void *data, long len);
+extern int ncld_lock(struct ncld_fh *);
+extern void ncld_unlock(struct ncld_fh *);
+extern void ncld_close(struct ncld_fh *);
+extern void ncld_sess_close(struct ncld_sess *s);
+extern void ncld_init(void);
+
+#endif /* __NCLD_H__ */
diff --git a/lib/cldc.c b/lib/cldc.c
index bc4b48c..514b0f7 100644
--- a/lib/cldc.c
+++ b/lib/cldc.c
@@ -1378,6 +1378,366 @@ char *cldc_dirent_name(struct cld_dirent_cur *dc)
return s;
}
+#if 0 /* temporarily, while checking cld_timer_foo in libtimer */
+/*
+ * On error, return the code (not negated code like a kernel function would).
+ */
+static int ncld_getsrv(char *hostp, unsigned short *portp,
+ struct hail_log *hlog)
+{
+ enum { hostsz = 64 };
+ char hostb[hostsz];
+ GList *host_list = NULL;
+ GList *tmp;
+ struct cldc_host *hp;
+
+ if (gethostname(hostb, hostsz-1) < 0)
+ return errno;
+ hostb[hostsz-1] = 0;
+
+ if (cldc_getaddr(&host_list, hostb, hlog))
+ return 1001;
+
+ /*
+ * This is a good place to compare weights and priorities, maybe later.
+ * For now, just grab first host in the list.
+ */
+ hp = host_list->data; /* cannot be NULL if success above */
+ *hostp = hp->host;
+ *portp = hp->port;
+
+ for (tmp = host_list; tmp; tmp = tmp->next) {
+ hp = tmp->data;
+ free(hp->host);
+ free(hp);
+ }
+ g_list_free(host_list);
+ return 0;
+}
+
+static int ncld_gethost(char *hostp, unsigned short *portp,
+ const char *hostname, const char *portstr)
+{
+ long n;
+
+ n = strtol(portstr, NULL, 10);
+ if (n <= 0 || n >= 65536)
+ return EINVAL;
+ *portp = n;
+
+ if (!(*hostp = strdup(hostname)))
+ return ENOMEM;
+
+ return 0;
+}
+
+static void ncld_udp_timer_event(int fd, short events, void *userdata)
+ <------- timer
+{
+ struct cld_session *sp = userdata;
+ struct cldc_udp *udp = sp->lib;
+
+ if (udp->cb)
+ udp->cb(udp->sess, udp->cb_private);
+}
+
+static gpointer ncld_sess_thr(gpointer data)
+{
+ struct ncld_sess *nsp = data;
+ int ufd;
+ fd_set rset;
+ struct timeval tm;
+ time_t tmo;
+ int rc;
+
+ for (;;) {
+ if (!nsp->udp)
+ ......
+
+ ufd = nsp->udp->fd;
+ FD_ZERO(&rset);
+ FD_SET(ufd, &rset);
+
+ tmo = cld_timers_run(&nsp->tlist);
+ if (tmo) {
+ tm.tv_sec = tmo;
+ tm.tv_usec = 0;
+ rc = select(ufd + 1, &rset, NULL, NULL, &tm);
+ if (rc < 0) {
+ fprintf(stderr, "select: error\n");
+ exit(1);
+ }
+ if (rc == 0)
+ continue;
+ } else {
+ rc = select(ufd + 1, &rset, NULL, NULL, NULL);
+ if (rc <= 0) {
+ fprintf(stderr, "select: nfd %d\n", rc);
+ exit(1);
+ }
+ }
+
+ if (FD_ISSET(ufd, &rset)) {
+ rc = cldc_udp_receive_pkt(udp);
+ if (rc) {
+ fprintf(stderr,
+ "cldc_udp_receive_pkt: error %d\n", rc);
+ exit(1);
+ }
+ } else {
+ fprintf(stderr, "noevent\n");
+ exit(1);
+ }
+ }
+
+ return NULL;
+}
+
+/*
+ * Safe the thread so that we can tear down the session.
+ */
+static void ncld_thr_poison(struct ncld_sess *nsp)
+{
+}
+
+/*
+ * Ask the thread to exit and wait until it does.
+ */
+static void ncld_thr_end(struct ncld_sess *nsp)
+{
+ .......
+}
+
+static int ncld_new_sess(struct cldc_call_opts *copts, enum cle_err_codes errc)
+{
+ ......
+}
+
+static bool ncld_p_timer_ctl(void *priv, bool add,
+ int (*cb)(struct cldc_session *, void *),
+ void *cb_priv, time_t secs)
+{
+ struct ncld_sess *nsp = priv;
+ struct cldc_udp *udp = nsp->udp;
+ struct timeval tv = { secs, 0 };
+
+ if (add) {
+ udp->cb = cb;
+ udp->cb_private = cb_priv;
+ return cld_timer_add(&nsp->lib_timer, &tv) == 0;
+ } else {
+ return cld_timer_del(&nsp->lib_timer) == 0;
+ }
+}
+
+static int ncld_p_pkt_send(void *priv, const void *addr, size_t addrlen,
+ const void *buf, size_t buflen)
+{
+ struct ncld_sess *nsp = priv;
+ return cldc_udp_pkt_send(nsp->udp, addr, addrlen, buf, buflen);
+}
+
+static void ncld_p_event(void *priv, struct cldc_session *csp,
+ struct cldc_fh *fh, uint32_t what)
+{
+ struct ncld_sess *nsp = priv;
+ int newactive;
+
+ if (what == CE_SESS_FAILED) {
+ sp->sess_open = false;
+ if (sp->lib->sess != csp)
+ applog(LOG_ERR, "Stray session failed, sid " SIDFMT,
+ SIDARG(csp->sid));
+ else
+ applog(LOG_ERR, "Session failed, sid " SIDFMT,
+ SIDARG(csp->sid));
+ // cld_timer_del(&sp->tm);
+ sp->lib->sess = NULL;
+ newactive = cldu_nextactive(sp);
+ if (cldu_set_cldc(sp, newactive))
+ return;
+ // cld_timer_add(&sp->tm, &cldc_to_delay);
+ } else {
+ if (csp)
+ applog(LOG_INFO, "cldc event 0x%x sid " SIDFMT,
+ what, SIDARG(csp->sid));
+ else
+ applog(LOG_INFO, "cldc event 0x%x no sid", what);
+ }
+}
+
+static struct cldc_ops ncld_ops = {
+ .timer_ctl = ncld_p_timer_ctl,
+ .pkt_send = ncld_p_pkt_send,
+ .event = ncld_p_event,
+ .errlog = NULL;
+};
+
+/*
+ * Initiating session may take a while, since we retry failures.
+ * We promise that eventually we return from this function.
+ *
+ * On error, returns NULL and sets the error code (system errno if below 1000,
+ * otherwise our own mysterious codes that you should just print in decimal).
+ *
+ * Keep in mind that the (*event)() is likely to be called on a context
+ * of the extra thread that we spawn. At least we won't steal some random
+ * context and monkey with signals. Also, (*event)() may be called before
+ * this function returns, with the session going down, for example.
+ * This is kind of dirty, but oh well. Maybe we'll fix this later.
+ *
+ * The lifetime of the hlog must be longer than that of the session.
+ *
+ * @param host Host name (NULL if resolving SRV records)
+ * @param port Port string (NULL if resolving SRV records)
+ * @param error Buffer for the error code
+ * @param event Session event function
+ * @param ev_arg User-supplied argument to the session event function
+ * @param cld_user The user identifier to be used to authentication
+ * @param cld_key The user key to be used to authentication
+ * @param hlog The log to which we (sadly) write, because we use cldc.
+ */
+struct ncld_sess *ncld_sess_open(const char *host, const char *port, int *error,
+ void (*event)(struct ncld_sess *, void *, unsigned int), void *ev_arg,
+ const char *cld_user, const char *cld_key,
+ struct hail_log *hlog)
+{
+ struct ncld_sess *nsp;
+ static GThread *thread;
+ struct cldc_call_opts copts;
+ GError *gerr;
+ int err;
+
+ err = ENOMEM;
+ nsp = malloc(sizeof(struct ncld_sess));
+ if (!nsp)
+ goto out_sesalloc;
+ memset(nsp, 0, sizeof(ncld_sess));
+
+ if (!host) {
+ err = EINVAL;
+ if (port)
+ goto out_portarg;
+ err = ncld_getsrv(&nsp->host, &nsp->port, hlog);
+ if (err)
+ goto out_srv;
+ } else {
+ err = EINVAL;
+ if (!port)
+ goto out_portarg;
+ err = ncld_gethost(&nsp->host, &nsp->port, host, port);
+ if (err)
+ goto out_srv;
+ }
+
+ nsp->event = event;
+ nsp->event_arg = ev_arg;
+
+............
+ nsp->thread = g_thread_create(ncld_sess_thr, nsp, FALSE, &gerr);
+ if (nsp->thread == NULL) {
+ HAIL_ERR(hlog, "Failed to start replication thread: %s",
+ gerr->message);
+ err = 1002;
+ goto out_thread;
+ }
+
+ if (cldc_udp_new(nsp->host, nsp->port, &nsp->udp)) {
+ err = 1003;
+ goto out_udp;
+ }
+
+ /*
+ * FIXME If anyone ever gets 2 sessions in one application,
+ * their logs' lifetimes will overlap. We need to move the errlog
+ * from ops to the session itself.
+ */
+ ncld_ops.errlog = hlog.func;
+
+ memset(&copts, 0, sizeof(copts));
+ copts.cb = ncld_new_sess;
+ if (cldc_new_sess(&ncld_ops, &copts, nsp->udp->addr, nsp->udp->addr_len,
+ cld_user, cld_key, nsp, &nsp->udp->sess)) {
+ err = 1004;
+ goto out_session;
+ }
+
+ /* udp->sess->log.verbose = hlog.verbose; */
+
+ wait for new session to establish
+
+ return nsp;
+
+out_session:
+ ncld_thr_poison(nsp);
+ cldc_udp_free(nsp->udp);
+out_udp:
+ ncld_thr_end(nsp);
+out_thread:
+out_portdup:
+ free(nsp->host);
+out_srv:
+out_portarg:
+ free(nsp);
+out_sesalloc:
+ *error = err;
+ return NULL;
+}
+
+/*
+ * We do not provide anything like ncld_set_callback because it inevitably
+ * leads to lost events. Unused arguments are not too onerous, so just
+ * put zero into the events if you don't want notifications.
+ *
+ * On error, return NULL and set the error code. XXX to what value?
+ */
+struct ncld_fh *ncld_open(struct ncld_sess *s, const char *fname,
+ unsigned int mode, int *error, unsigned int events,
+ void (*event)(struct ncld_fh *, void *, unsigned int), void *ev_arg)
+{
+}
+
+/*
+ * Using a read/write-type interface inevitably produces an extra data copy,
+ * but CLD is not intended as a high performance service. So, there is
+ * no get/put API, only read/write.
+ */
+long ncld_read(struct ncld_fh *, void *data, long len)
+{
+}
+
+long ncld_write(struct ncld_fh *, const void *data, long len)
+{
+}
+
+int ncld_lock(struct ncld_fh *)
+{
+}
+
+void ncld_unlock(struct ncld_fh *)
+{
+}
+
+void ncld_close(struct ncld_fh *)
+{
+}
+
+void ncld_sess_close(struct ncld_sess *nsp)
+{
+ cldc_kill_sess(nsp->udp->sess);
+ ncld_thr_poison(nsp);
+ cldc_udp_free(nsp->udp);
+ ncld_thr_end(nsp);
+ free(nsp->host);
+ free(nsp);
+}
+
+void ncld_init(void)
+{
+ cldc_init();
+}
+#endif 0 /* temporarily */
+
/*
* For extra safety, call cldc_init after g_thread_init, if present.
* Currently we just call srand(), but since we use GLib, we may need
-- Pete
--
To unsubscribe from this list: send the line "unsubscribe hail-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
[Linux USB Devel]
[Video for Linux]
[Linux Audio Users]
[Photo]
[Yosemite News]
[Yosemite Photos]
[Free Online Dating]
[Linux Kernel]
[Linux SCSI]
[XFree86]
