|
|
| 1 2 3 4 |
|
Benny Baumann
|
Hi,
I recently discovered the PHP extension for RRDTool which is available as a contribution to RRDTool. This extension runs successfully with latest PHP5, but misses some functions of current RRDTool releases (no rrd_dump, no rrd_info). I'm currently working on updating the PHP extension to also include these operations. And here comes the issue: The RRDTool library is more or less written to be used in the command line interface thus either returns only single values or dumps output to a file\terminal. For the PHP extension I need output to be written into a buffer though. To solve this I suggest adding a callback interface to e.g. rrd_dump that is called whenever string data (or binary data) has to be returned. An example implementation for rrd_dump_opt_cb_r can be found in the attached patch. Feedback appreciated. IDK if this patch is complete and if the mentioned rrd_dump_opt_cb_r function is properly exported; but at least dumping should work. The basic rrd_dump_opt_r now calls this Callback method internally to give an example of how this callback interface can be used and to avoid duplicate source. Regards, BenBE. [rrd_dump_cb.patch] Index: Z:/data/programming/own/rrdtool/trunk/lib/src/rrd_dump.c =================================================================== --- Z:/data/programming/own/rrdtool/trunk/lib/src/rrd_dump.c (Revision 19) +++ Z:/data/programming/own/rrdtool/trunk/lib/src/rrd_dump.c (Revision 23) @@ -58,107 +58,149 @@ extern char *tzname[2]; #endif +//Local prototypes +size_t rrd_dump_opt_cb_fileout( + const void *data, + size_t len, + void *user); -int rrd_dump_opt_r( + +int rrd_dump_opt_cb_r( const char *filename, - char *outname, - int opt_noheader) + int opt_noheader, + rrd_output_callback_t cb, + void *user) { unsigned int i, ii, ix, iii = 0; time_t now; char somestring[255]; + char somestring_buf[255]; rrd_value_t my_cdp; off_t rra_base, rra_start, rra_next; rrd_file_t *rrd_file; - FILE *out_file; rrd_t rrd; rrd_value_t value; struct tm tm; char *old_locale = ""; + + //Check if we got a (valid) callback method + if (!cb) { + return (-1); + } + rrd_file = rrd_open(filename, &rrd, RRD_READONLY | RRD_READAHEAD); if (rrd_file == NULL) { rrd_free(&rrd); return (-1); } - out_file = NULL; - if (outname) { - if (!(out_file = fopen(outname, "w"))) { - return (-1); - } - } else { - out_file = stdout; - } #ifdef HAVE_SETLOCALE old_locale = setlocale(LC_NUMERIC, "C"); #endif + if (!opt_noheader) { - fputs("<?xml version=\"1.0\" encoding=\"utf-8\"?>\n", out_file); - fputs - ("<!DOCTYPE rrd SYSTEM \"http://oss.oetiker.ch/rrdtool/rrdtool.dtd\">\n", - out_file); + cb("<?xml version=\"1.0\" encoding=\"utf-8\"?>\n", + strlen("<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"), user); + cb("<!DOCTYPE rrd SYSTEM \"http://oss.oetiker.ch/rrdtool/rrdtool.dtd\">\n", + strlen("<!DOCTYPE rrd SYSTEM \"http://oss.oetiker.ch/rrdtool/rrdtool.dtd\">\n"), user); } - fputs("<!-- Round Robin Database Dump -->", out_file); - fputs("<rrd>", out_file); + + cb("<!-- Round Robin Database Dump -->", + strlen("<!-- Round Robin Database Dump -->"), user); + + cb("<rrd>", + strlen("<rrd>"), user); + if (atoi(rrd.stat_head->version) <= 3) { - fprintf(out_file, "\t<version> %s </version>\n", RRD_VERSION3); + snprintf(somestring, 255, "\t<version> %s </version>\n", RRD_VERSION3); } else { - fprintf(out_file, "\t<version> %s </version>\n", RRD_VERSION); + snprintf(somestring, 255, "\t<version> %s </version>\n", RRD_VERSION); } - fprintf(out_file, "\t<step> %lu </step> <!-- Seconds -->\n", - rrd.stat_head->pdp_step); + cb(somestring, strlen(somestring), user); + + snprintf(somestring, 255, "\t<step> %lu </step> <!-- Seconds -->\n", + rrd.stat_head->pdp_step); + cb(somestring, strlen(somestring), user); + #ifdef HAVE_STRFTIME localtime_r(&rrd.live_head->last_up, &tm); - strftime(somestring, 200, "%Y-%m-%d %H:%M:%S %Z", &tm); + strftime(somestring, 255, "%Y-%m-%d %H:%M:%S %Z", &tm); #else # error "Need strftime" #endif - fprintf(out_file, "\t<lastupdate> %lu </lastupdate> <!-- %s -->\n\n", - (unsigned long) rrd.live_head->last_up, somestring); + snprintf(somestring_buf, 255, "\t<lastupdate> %lu </lastupdate> <!-- %s -->\n\n", + (unsigned long) rrd.live_head->last_up, somestring); + cb(somestring_buf, strlen(somestring_buf), user); + for (i = 0; i < rrd.stat_head->ds_cnt; i++) { - fprintf(out_file, "\t<ds>\n"); - fprintf(out_file, "\t\t<name> %s </name>\n", rrd.ds_def[i].ds_nam); - fprintf(out_file, "\t\t<type> %s </type>\n", rrd.ds_def[i].dst); + cb("\t<ds>\n", + strlen("\t<ds>\n"), user); + + snprintf(somestring_buf, 255, "\t\t<name> %s </name>\n", rrd.ds_def[i].ds_nam); + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, "\t\t<type> %s </type>\n", rrd.ds_def[i].dst); + cb(somestring_buf, strlen(somestring_buf), user); + if (dst_conv(rrd.ds_def[i].dst) != DST_CDEF) { - fprintf(out_file, - "\t\t<minimal_heartbeat> %lu </minimal_heartbeat>\n", + snprintf(somestring_buf, 255, "\t\t<minimal_heartbeat> %lu </minimal_heartbeat>\n", rrd.ds_def[i].par[DS_mrhb_cnt].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); + if (isnan(rrd.ds_def[i].par[DS_min_val].u_val)) { - fprintf(out_file, "\t\t<min> NaN </min>\n"); + snprintf(somestring_buf, 255, "\t\t<min> NaN </min>\n"); } else { - fprintf(out_file, "\t\t<min> %0.10e </min>\n", - rrd.ds_def[i].par[DS_min_val].u_val); + snprintf(somestring_buf, 255, "\t\t<min> %0.10e </min>\n", + rrd.ds_def[i].par[DS_min_val].u_val); } + cb(somestring_buf, strlen(somestring_buf), user); + if (isnan(rrd.ds_def[i].par[DS_max_val].u_val)) { - fprintf(out_file, "\t\t<max> NaN </max>\n"); + snprintf(somestring_buf, 255, "\t\t<max> NaN </max>\n"); } else { - fprintf(out_file, "\t\t<max> %0.10e </max>\n", - rrd.ds_def[i].par[DS_max_val].u_val); + snprintf(somestring_buf, 255, "\t\t<max> %0.10e </max>\n", + rrd.ds_def[i].par[DS_max_val].u_val); } + cb(somestring_buf, strlen(somestring_buf), user); } else { /* DST_CDEF */ char *str = NULL; rpn_compact2str((rpn_cdefds_t *) &(rrd.ds_def[i].par[DS_cdef]), - rrd.ds_def, &str); - fprintf(out_file, "\t\t<cdef> %s </cdef>\n", str); + rrd.ds_def, &str); + + //Splitting into 3 writes to avoid allocating memory + //This is better compared to snprintf as str may be of arbitrary size + cb("\t\t<cdef> ", strlen("\t\t<cdef> "), user); + cb(str, strlen(str), user); + cb(" </cdef>\n", strlen(" </cdef>\n"), user); + free(str); } - fprintf(out_file, "\n\t\t<!-- PDP Status -->\n"); - fprintf(out_file, "\t\t<last_ds> %s </last_ds>\n", - rrd.pdp_prep[i].last_ds); + + cb("\n\t\t<!-- PDP Status -->\n", + strlen("\n\t\t<!-- PDP Status -->\n"), user); + snprintf(somestring_buf, 255, "\t\t<last_ds> %s </last_ds>\n", + rrd.pdp_prep[i].last_ds); + cb(somestring_buf, strlen(somestring_buf), user); + if (isnan(rrd.pdp_prep[i].scratch[PDP_val].u_val)) { - fprintf(out_file, "\t\t<value> NaN </value>\n"); + snprintf(somestring_buf, 255, "\t\t<value> NaN </value>\n"); } else { - fprintf(out_file, "\t\t<value> %0.10e </value>\n", - rrd.pdp_prep[i].scratch[PDP_val].u_val); + snprintf(somestring_buf, 255, "\t\t<value> %0.10e </value>\n", + rrd.pdp_prep[i].scratch[PDP_val].u_val); } - fprintf(out_file, "\t\t<unknown_sec> %lu </unknown_sec>\n", - rrd.pdp_prep[i].scratch[PDP_unkn_sec_cnt].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); - fprintf(out_file, "\t</ds>\n\n"); + snprintf(somestring_buf, 255, "\t\t<unknown_sec> %lu </unknown_sec>\n", + rrd.pdp_prep[i].scratch[PDP_unkn_sec_cnt].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); + + cb("\t</ds>\n\n", + strlen("\t</ds>\n\n"), user); } - fputs("<!-- Round Robin Archives -->", out_file); + cb("<!-- Round Robin Archives -->", + strlen("<!-- Round Robin Archives -->"), user); rra_base = rrd_file->header_len; rra_next = rra_base; @@ -170,182 +212,224 @@ rra_start = rra_next; rra_next += (rrd.stat_head->ds_cnt * rrd.rra_def[i].row_cnt * sizeof(rrd_value_t)); - fprintf(out_file, "\t<rra>\n"); - fprintf(out_file, "\t\t<cf> %s </cf>\n", rrd.rra_def[i].cf_nam); - fprintf(out_file, - "\t\t<pdp_per_row> %lu </pdp_per_row> <!-- %lu seconds -->\n\n", - rrd.rra_def[i].pdp_cnt, - rrd.rra_def[i].pdp_cnt * rrd.stat_head->pdp_step); + + cb("\t<rra>\n", + strlen("\t<rra>\n"), user); + + snprintf(somestring_buf, 255, + "\t\t<cf> %s </cf>\n", rrd.rra_def[i].cf_nam); + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, + "\t\t<pdp_per_row> %lu </pdp_per_row> <!-- %lu seconds -->\n\n", + rrd.rra_def[i].pdp_cnt, + rrd.rra_def[i].pdp_cnt * rrd.stat_head->pdp_step); + cb(somestring_buf, strlen(somestring_buf), user); + /* support for RRA parameters */ - fprintf(out_file, "\t\t<params>\n"); + cb("\t\t<params>\n", + strlen("\t\t<params>\n"), user); + switch (cf_conv(rrd.rra_def[i].cf_nam)) { case CF_HWPREDICT: case CF_MHWPREDICT: - fprintf(out_file, "\t\t<hw_alpha> %0.10e </hw_alpha>\n", + snprintf(somestring_buf, 255, "\t\t<hw_alpha> %0.10e </hw_alpha>\n", rrd.rra_def[i].par[RRA_hw_alpha].u_val); - fprintf(out_file, "\t\t<hw_beta> %0.10e </hw_beta>\n", + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, "\t\t<hw_beta> %0.10e </hw_beta>\n", rrd.rra_def[i].par[RRA_hw_beta].u_val); - fprintf(out_file, + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, "\t\t<dependent_rra_idx> %lu </dependent_rra_idx>\n", rrd.rra_def[i].par[RRA_dependent_rra_idx].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); break; case CF_SEASONAL: case CF_DEVSEASONAL: - fprintf(out_file, + snprintf(somestring_buf, 255, "\t\t<seasonal_gamma> %0.10e </seasonal_gamma>\n", rrd.rra_def[i].par[RRA_seasonal_gamma].u_val); - fprintf(out_file, + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, "\t\t<seasonal_smooth_idx> %lu </seasonal_smooth_idx>\n", rrd.rra_def[i].par[RRA_seasonal_smooth_idx].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); + if (atoi(rrd.stat_head->version) >= 4) { - fprintf(out_file, - "\t\t<smoothing_window> %0.10e </smoothing_window>\n", - rrd.rra_def[i].par[RRA_seasonal_smoothing_window]. - u_val); + snprintf(somestring_buf, 255, + "\t\t<smoothing_window> %0.10e </smoothing_window>\n", + rrd.rra_def[i].par[RRA_seasonal_smoothing_window].u_val); + cb(somestring_buf, strlen(somestring_buf), user); } - fprintf(out_file, + snprintf(somestring_buf, 255, "\t\t<dependent_rra_idx> %lu </dependent_rra_idx>\n", rrd.rra_def[i].par[RRA_dependent_rra_idx].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); break; case CF_FAILURES: - fprintf(out_file, "\t\t<delta_pos> %0.10e </delta_pos>\n", + snprintf(somestring_buf, 255, "\t\t<delta_pos> %0.10e </delta_pos>\n", rrd.rra_def[i].par[RRA_delta_pos].u_val); - fprintf(out_file, "\t\t<delta_neg> %0.10e </delta_neg>\n", + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, "\t\t<delta_neg> %0.10e </delta_neg>\n", rrd.rra_def[i].par[RRA_delta_neg].u_val); - fprintf(out_file, "\t\t<window_len> %lu </window_len>\n", + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, "\t\t<window_len> %lu </window_len>\n", rrd.rra_def[i].par[RRA_window_len].u_cnt); - fprintf(out_file, + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, "\t\t<failure_threshold> %lu </failure_threshold>\n", rrd.rra_def[i].par[RRA_failure_threshold].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); + /* fall thru */ case CF_DEVPREDICT: - fprintf(out_file, + snprintf(somestring_buf, 255, "\t\t<dependent_rra_idx> %lu </dependent_rra_idx>\n", rrd.rra_def[i].par[RRA_dependent_rra_idx].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); break; case CF_AVERAGE: case CF_MAXIMUM: case CF_MINIMUM: case CF_LAST: default: - fprintf(out_file, "\t\t<xff> %0.10e </xff>\n", + snprintf(somestring_buf, 255, "\t\t<xff> %0.10e </xff>\n", rrd.rra_def[i].par[RRA_cdp_xff_val].u_val); + cb(somestring_buf, strlen(somestring_buf), user); break; } - fprintf(out_file, "\t\t</params>\n"); - fprintf(out_file, "\t\t<cdp_prep>\n"); + + cb("\t\t</params>\n", + strlen("\t\t</params>\n"), user); + cb("\t\t<cdp_prep>\n", + strlen("\t\t<cdp_prep>\n"), user); + for (ii = 0; ii < rrd.stat_head->ds_cnt; ii++) { unsigned long ivalue; - fprintf(out_file, "\t\t\t<ds>\n"); + cb("\t\t\t<ds>\n", + strlen("\t\t\t<ds>\n"), user); /* support for exporting all CDP parameters */ /* parameters common to all CFs */ /* primary_val and secondary_val do not need to be saved between updates * so strictly speaking they could be omitted. * However, they can be useful for diagnostic purposes, so are included here. */ - value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt - + ii].scratch[CDP_primary_val].u_val; + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_primary_val].u_val; if (isnan(value)) { - fprintf(out_file, - "\t\t\t<primary_value> NaN </primary_value>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<primary_value> NaN </primary_value>\n"); } else { - fprintf(out_file, - "\t\t\t<primary_value> %0.10e </primary_value>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<primary_value> %0.10e </primary_value>\n", value); } - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_secondary_val].u_val; + cb(somestring_buf, strlen(somestring_buf), user); + + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_secondary_val].u_val; if (isnan(value)) { - fprintf(out_file, - "\t\t\t<secondary_value> NaN </secondary_value>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<secondary_value> NaN </secondary_value>\n"); } else { - fprintf(out_file, - "\t\t\t<secondary_value> %0.10e </secondary_value>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<secondary_value> %0.10e </secondary_value>\n", value); } + cb(somestring_buf, strlen(somestring_buf), user); + switch (cf_conv(rrd.rra_def[i].cf_nam)) { case CF_HWPREDICT: case CF_MHWPREDICT: - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_hw_intercept].u_val; + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_hw_intercept].u_val; if (isnan(value)) { - fprintf(out_file, "\t\t\t<intercept> NaN </intercept>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<intercept> NaN </intercept>\n"); } else { - fprintf(out_file, - "\t\t\t<intercept> %0.10e </intercept>\n", value); + snprintf(somestring_buf, 255, + "\t\t\t<intercept> %0.10e </intercept>\n", value); } - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_hw_last_intercept].u_val; + cb(somestring_buf, strlen(somestring_buf), user); + + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_hw_last_intercept].u_val; if (isnan(value)) { - fprintf(out_file, - "\t\t\t<last_intercept> NaN </last_intercept>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<last_intercept> NaN </last_intercept>\n"); } else { - fprintf(out_file, - "\t\t\t<last_intercept> %0.10e </last_intercept>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<last_intercept> %0.10e </last_intercept>\n", value); } - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_hw_slope].u_val; + cb(somestring_buf, strlen(somestring_buf), user); + + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_hw_slope].u_val; if (isnan(value)) { - fprintf(out_file, "\t\t\t<slope> NaN </slope>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<slope> NaN </slope>\n"); } else { - fprintf(out_file, "\t\t\t<slope> %0.10e </slope>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<slope> %0.10e </slope>\n", value); } - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_hw_last_slope].u_val; + cb(somestring_buf, strlen(somestring_buf), user); + + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_hw_last_slope].u_val; if (isnan(value)) { - fprintf(out_file, - "\t\t\t<last_slope> NaN </last_slope>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<last_slope> NaN </last_slope>\n"); } else { - fprintf(out_file, - "\t\t\t<last_slope> %0.10e </last_slope>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<last_slope> %0.10e </last_slope>\n", value); } - ivalue = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_null_count].u_cnt; - fprintf(out_file, "\t\t\t<nan_count> %lu </nan_count>\n", - ivalue); - ivalue = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_last_null_count].u_cnt; - fprintf(out_file, - "\t\t\t<last_nan_count> %lu </last_nan_count>\n", - ivalue); + cb(somestring_buf, strlen(somestring_buf), user); + + ivalue = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_null_count].u_cnt; + snprintf(somestring_buf, 255, + "\t\t\t<nan_count> %lu </nan_count>\n", ivalue); + cb(somestring_buf, strlen(somestring_buf), user); + + ivalue = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_last_null_count].u_cnt; + snprintf(somestring_buf, 255, + "\t\t\t<last_nan_count> %lu </last_nan_count>\n", ivalue); + cb(somestring_buf, strlen(somestring_buf), user); break; case CF_SEASONAL: case CF_DEVSEASONAL: - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_hw_seasonal].u_val; + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_hw_seasonal].u_val; if (isnan(value)) { - fprintf(out_file, "\t\t\t<seasonal> NaN </seasonal>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<seasonal> NaN </seasonal>\n"); } else { - fprintf(out_file, "\t\t\t<seasonal> %0.10e </seasonal>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<seasonal> %0.10e </seasonal>\n", value); } - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_hw_last_seasonal].u_val; + cb(somestring_buf, strlen(somestring_buf), user); + + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_hw_last_seasonal].u_val; if (isnan(value)) { - fprintf(out_file, - "\t\t\t<last_seasonal> NaN </last_seasonal>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<last_seasonal> NaN </last_seasonal>\n"); } else { - fprintf(out_file, - "\t\t\t<last_seasonal> %0.10e </last_seasonal>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<last_seasonal> %0.10e </last_seasonal>\n", value); } - ivalue = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_init_seasonal].u_cnt; - fprintf(out_file, "\t\t\t<init_flag> %lu </init_flag>\n", - ivalue); + cb(somestring_buf, strlen(somestring_buf), user); + + ivalue = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_init_seasonal].u_cnt; + snprintf(somestring_buf, 255, + "\t\t\t<init_flag> %lu </init_flag>\n", ivalue); + cb(somestring_buf, strlen(somestring_buf), user); break; case CF_DEVPREDICT: break; @@ -359,13 +443,16 @@ ds_cnt + ii]. scratch); - fprintf(out_file, "\t\t\t<history> "); + cb("\t\t\t<history> ", + strlen("\t\t\t<history> "), user); for (vidx = 0; - vidx < rrd.rra_def[i].par[RRA_window_len].u_cnt; - ++vidx) { - fprintf(out_file, "%d", violations_array[vidx]); + vidx < rrd.rra_def[i].par[RRA_window_len].u_cnt; + ++vidx) { + snprintf(somestring_buf, 255, "%d", violations_array[vidx]); + cb(somestring_buf, strlen(somestring_buf), user); } - fprintf(out_file, " </history>\n"); + cb(" </history>\n", + strlen(" </history>\n"), user); } break; case CF_AVERAGE: @@ -373,26 +460,31 @@ case CF_MINIMUM: case CF_LAST: default: - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_val].u_val; + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii].scratch[CDP_val].u_val; if (isnan(value)) { - fprintf(out_file, "\t\t\t<value> NaN </value>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<value> NaN </value>\n"); } else { - fprintf(out_file, "\t\t\t<value> %0.10e </value>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<value> %0.10e </value>\n", value); } - fprintf(out_file, - "\t\t\t<unknown_datapoints> %lu </unknown_datapoints>\n", - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_unkn_pdp_cnt].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, + "\t\t\t<unknown_datapoints> %lu </unknown_datapoints>\n", + rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_unkn_pdp_cnt].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); break; } - fprintf(out_file, "\t\t\t</ds>\n"); + cb("\t\t\t</ds>\n", + strlen("\t\t\t</ds>\n"), user); } - fprintf(out_file, "\t\t</cdp_prep>\n"); + cb("\t\t</cdp_prep>\n", + strlen("\t\t</cdp_prep>\n"), user); - fprintf(out_file, "\t\t<database>\n"); + cb("\t\t<database>\n", + strlen("\t\t<database>\n"), user); rrd_seek(rrd_file, (rra_start + (rrd.rra_ptr[i].cur_row + 1) * rrd.stat_head->ds_cnt * sizeof(rrd_value_t)), SEEK_SET); @@ -412,36 +504,75 @@ timer++; #if HAVE_STRFTIME localtime_r(&now, &tm); - strftime(somestring, 200, "%Y-%m-%d %H:%M:%S %Z", &tm); + strftime(somestring, 255, "%Y-%m-%d %H:%M:%S %Z", &tm); #else # error "Need strftime" #endif - fprintf(out_file, "\t\t\t<!-- %s / %d --> <row>", somestring, - (int) now); + snprintf(somestring_buf, 255, + "\t\t\t<!-- %s / %d --> <row>", somestring, (int) now); + cb(somestring_buf, strlen(somestring_buf), user); for (iii = 0; iii < rrd.stat_head->ds_cnt; iii++) { rrd_read(rrd_file, &my_cdp, sizeof(rrd_value_t) * 1); if (isnan(my_cdp)) { - fprintf(out_file, "<v> NaN </v>"); + snprintf(somestring_buf, 255, "<v> NaN </v>"); } else { - fprintf(out_file, "<v> %0.10e </v>", my_cdp); - }; + snprintf(somestring_buf, 255, "<v> %0.10e </v>", my_cdp); + } + cb(somestring_buf, strlen(somestring_buf), user); } - fprintf(out_file, "</row>\n"); + cb("</row>\n", + strlen("</row>\n"), user); } - fprintf(out_file, "\t\t</database>\n\t</rra>\n"); + cb("\t\t</database>\n\t</rra>\n", + strlen("\t\t</database>\n\t</rra>\n"), user); + } - } - fprintf(out_file, "</rrd>\n"); + cb("</rrd>\n", + strlen("</rrd>\n"), user); + rrd_free(&rrd); - if (out_file != stdout) { - fclose(out_file); - } + #ifdef HAVE_SETLOCALE setlocale(LC_NUMERIC, old_locale); #endif + return rrd_close(rrd_file); } +size_t rrd_dump_opt_cb_fileout( + const void *data, + size_t len, + void *user) +{ + return fwrite(data, 1, len, (FILE *)user); +} + +int rrd_dump_opt_r( + const char *filename, + char *outname, + int opt_noheader) +{ + FILE *out_file; + int res; + + out_file = NULL; + if (outname) { + if (!(out_file = fopen(outname, "w"))) { + return (-1); + } + } else { + out_file = stdout; + } + + res = rrd_dump_opt_cb_r(filename, opt_noheader, rrd_dump_opt_cb_fileout, (void *)out_file); + + if (out_file != stdout) { + fclose(out_file); + } + + return res; +} + /* backward compatibility with 1.2.x */ int rrd_dump_r( const char *filename, Index: Z:/data/programming/own/rrdtool/trunk/lib/src/rrd.h =================================================================== --- Z:/data/programming/own/rrdtool/trunk/lib/src/rrd.h (Revision 19) +++ Z:/data/programming/own/rrdtool/trunk/lib/src/rrd.h (Revision 23) @@ -122,6 +122,10 @@ struct rrd_info_t *next; } rrd_info_t; + typedef size_t (* rrd_output_callback_t)( + const void *, + size_t, + void *); /* main function blocks */ int rrd_create( @@ -243,6 +247,12 @@ const char *filename, int rraindex); + int rrd_dump_opt_cb_r( + const char *filename, + int opt_noheader, + rrd_output_callback_t cb, + void *user); + /* Transplanted from rrd_parsetime.h */ typedef enum { ABSOLUTE_TIME, Index: Z:/data/programming/own/rrdtool/trunk/lib/CONTRIBUTORS =================================================================== --- Z:/data/programming/own/rrdtool/trunk/lib/CONTRIBUTORS (Revision 19) +++ Z:/data/programming/own/rrdtool/trunk/lib/CONTRIBUTORS (Revision 23) @@ -7,6 +7,7 @@ Amos Shapira <amos with gezernet.co.il> Andreas Kroomaa <andre with ml.ee> Andrew Turner <turner with mint.net> (LAST consolidator) +Benny Baumann <benbe with geshi.org) rrd_dump with callback support Bernard Fischer <bfischer with syslog.ch> 64bit stuff, --alt-autoscale-max Bernhard Fischer <rep dot dot dot nop with gmail.com> MMAP rewrite Bill Fenner <fenner with research.att.com> _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
|
Dan Cech-2
|
Hi Benny,
Benny Baumann wrote: > I recently discovered the PHP extension for RRDTool which is available > as a contribution to RRDTool. This extension runs successfully with > latest PHP5, but misses some functions of current RRDTool releases (no > rrd_dump, no rrd_info). I'm currently working on updating the PHP > extension to also include these operations. Funny, I've been doing some work on the PHP rrdtool extension myself in the last couple of weeks. It's not quite ready for prime-time yet, but I have completed the following: - remove requirement to send argument count to rrd_graph - add support for rrd_xport - remove requirement to send argument count to rrd_fetch - support multiple update values in rrd_update - support rrd_lastupdate - support rrd_first - clean up phpinfo() output I was planning to tackle rrd_info next, but haven't looked at rrd_dump. -- Dan Cech Ubersmith, Inc. http://www.ubersmith.com _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
Benny Baumann
|
Hi,
Am 30.07.2009 15:18, schrieb Dan Cech: > Hi Benny, > > Benny Baumann wrote: > >> I recently discovered the PHP extension for RRDTool which is available >> as a contribution to RRDTool. This extension runs successfully with >> latest PHP5, but misses some functions of current RRDTool releases (no >> rrd_dump, no rrd_info). I'm currently working on updating the PHP >> extension to also include these operations. >> > > Funny, I've been doing some work on the PHP rrdtool extension myself in > the last couple of weeks. It's not quite ready for prime-time yet, but > I have completed the following: > > - remove requirement to send argument count to rrd_graph > - remove requirement to send argument count to rrd_fetch > > - add support for rrd_xport > > - support multiple update values in rrd_update > Will need to have a look at this, but in general this sounds positive ;-) > - support rrd_lastupdate > - support rrd_first > Great ;-) > - clean up phpinfo() output > Yep. That one was really messy :P Though rrdtool_info will need some cleanup too. Simply returning an array will all the needed values should be sufficiant and much more practical. > I was planning to tackle rrd_info next, but haven't looked at rrd_dump. > I already completed rrd_info, but didn't parse the returned key names. I'm writing a PHP wrapper for this PHP extension and as such I can do parsing of the keynames there. Some work on Open Basedir Restriction can be found at http://blog.benny-baumann.de/?p=352 (German). The patch there isn't fully up-to-date, bbut basically works (except for one minor typo I already fixed in my local dev version). Would be nice if we could exchange our versions to merge the changes, thus avoiding duplicate work. One problem (I mentioned in my blog) I couldn't solve yet is enforcing the OpenBasedir Restriction for filenames mentioned in the command line arguments (e.g. for rrd_graph. IDK if you've got a solution. But well, back to the API extension suggested here: What do you think of offering a callback based variant in addition to a file based variant thus programs that need the returned data in a buffer can grab it without needing to write to the filesystem? Other functions (e.g. for rrd_graph) that write to disk might use such a callback too, or offer returning the ressources they created to the caller (i.e. giving the generated image as a internal ressource to the caller). Regards, BenBE. _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
Dan Cech-2
|
Benny Baumann wrote:
> Though rrdtool_info will need some cleanup too. Simply returning an > array will all the needed values should be sufficiant and much more > practical. This is an interesting BC question, all the updates I've made so far are completely backwards compatible, not requiring any changes to existing code using the extension. Changing the rrdtool_info function to return an array rather than printing the information is a pretty major change. The rrd_fetch output presents a similar problem, all the values are returned in a single long array at present. It would be nice to return the data as a multi-dimensional array with each 'row' of data grouped together (and very simple to do in the extension) but it would break BC with existing scripts using the current version of this function. >> I was planning to tackle rrd_info next, but haven't looked at rrd_dump. >> > I already completed rrd_info, but didn't parse the returned key names. > I'm writing a PHP wrapper for this PHP extension and as such I can do > parsing of the keynames there. Yeah, returning the raw data as it comes from rrd_info_r is easy, but I was planning to parse it out into a multi-dimensional php array. A separate php-code library to parse the output of the rrd_info function in the php extension seems like it would make it more difficult to use the system because you would need to have the userland php code as well as the extension itself. > Some work on Open Basedir Restriction can be found at > http://blog.benny-baumann.de/?p=352 (German). The patch there isn't > fully up-to-date, bbut basically works (except for one minor typo I > already fixed in my local dev version). I hadn't considered open_basedir, but it seems like a good idea to add this safety feature. Enforcing open_basedir on the actual RRDs referenced in graph, xport, etc will be more difficult though! > Would be nice if we could exchange our versions to merge the changes, > thus avoiding duplicate work. Agreed, any suggestions on the best way to do that? > What do you think of offering a callback based variant in addition to a > file based variant thus programs that need the returned data in a buffer > can grab it without needing to write to the filesystem? > > Other functions (e.g. for rrd_graph) that write to disk might use such a > callback too, or offer returning the ressources they created to the > caller (i.e. giving the generated image as a internal ressource to the > caller). I can see how this might be a useful thing to support, in the case of rrd_graph I was considering implementing and interface to graphv which would allow returning the image data as a php string directly, but the volume of data returned by rrd_dump can obviously be much larger than the typical graph image. I typically output the graph images to temporary files named according to a hash of the graph arguments, which allows me to re-use graph images for multiple clients without needing to regenerate them from the source rrds (very useful when multiple clients are requesting identical graphs). I'm no expert but it seems like the data will still all need to be collected into a buffer before it can be returned to the calling php script as a string or php array, so in the case of rrd_dump it may be more efficient to write out the xml to disk and perform any operations on it using a stream-based xml parser. Dan -- Dan Cech Ubersmith, Inc. http://www.ubersmith.com _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
Benny Baumann
|
Am 30.07.2009 17:46, schrieb Dan Cech:
> Benny Baumann wrote: > >> Though rrdtool_info will need some cleanup too. Simply returning an >> array will all the needed values should be sufficiant and much more >> practical. >> > > This is an interesting BC question, all the updates I've made so far are > completely backwards compatible, not requiring any changes to existing > code using the extension. Changing the rrdtool_info function to return > an array rather than printing the information is a pretty major change. > given or an array with all the information should be returned. But since I doubt this extension in the old form has reached any significant distribution even having tthe array being returned as default shouldn't matter that much. > The rrd_fetch output presents a similar problem, all the values are > returned in a single long array at present. It would be nice to return > the data as a multi-dimensional array with each 'row' of data grouped > together (and very simple to do in the extension) but it would break BC > with existing scripts using the current version of this function. > As the existing extension is quite incomplete I guess noone should really complain about if it is more or less rewritten and thus modified to be of much more use. Even then I'm writing on a Library wrapper in PHP that will take care of wrapping the functional interface of the extension into an OOP interface for PHP5. More on this wrapper library once the basic usage works properly. >>> I was planning to tackle rrd_info next, but haven't looked at rrd_dump. >>> >> I already completed rrd_info, but didn't parse the returned key names. >> I'm writing a PHP wrapper for this PHP extension and as such I can do >> parsing of the keynames there. >> > Yeah, returning the raw data as it comes from rrd_info_r is easy, but I > was planning to parse it out into a multi-dimensional php array. A > I planned to do this in my wrapper library as I have to postprocess the data anyways. > separate php-code library to parse the output of the rrd_info function > in the php extension seems like it would make it more difficult to use > the system because you would need to have the userland php code as well > as the extension itself. > The userland code is a wrapper around the functions in such a way that you won't need to care about the command line interface for RRDTool. For example, even though the creation of a new RRD file is much more verbose, the following source saves you from doing all the calculations yourself: --- include 'rrdtool.php'; $RRD = RRDTool::Archive(); if(!$RRD->Load('net.rrd')) { $RRD->Start = 0; $RRD->addDS( RRDTool_DataSource::createDS( 'trafficIn', RRD_DS_COUNTER, 600, array( RRD_PARAM_CF_MIN => 0, RRD_PARAM_CF_MAX => 65535 ) ) ); $RRD->addDS( RRDTool_DataSource::createDS( 'trafficOut', RRD_DS_COUNTER, 600, array( RRD_PARAM_CF_MIN => 0, RRD_PARAM_CF_MAX => 65535 ) ) ); $RRD->addRRA(RRDTool_Archive::createRRACF( RRD_RRA_CF_AVG, 0.5, $RRD->getPDPIC(RRD_DEFAULT_STEP), $RRD->getPDPIC(RRD_TIME_DAY) )); $RRD->addRRA(RRDTool_Archive::createRRACF( RRD_RRA_CF_AVG, 0.5, $RRD->getPDPIC(15 * RRD_TIME_MINUTE), $RRD->getPDPIC(RRD_TIME_WEEK, 15 * RRD_TIME_MINUTE) )); $RRD->addRRA(RRDTool_Archive::createRRACF( RRD_RRA_CF_AVG, 0.5, $RRD->getPDPIC(RRD_TIME_HOUR), $RRD->getPDPIC(RRD_TIME_MONTH, RRD_TIME_HOUR) )); $RRD->addRRA(RRDTool_Archive::createRRACF( RRD_RRA_CF_AVG, 0.5, $RRD->getPDPIC(4 * RRD_TIME_HOUR), $RRD->getPDPIC(RRD_TIME_YEAR, 4 * RRD_TIME_HOUR) )); $RRD->addRRA(RRDTool_Archive::createRRACF( RRD_RRA_CF_MAX, 0.5, $RRD->getPDPIC(15 * RRD_TIME_MINUTE), $RRD->getPDPIC(RRD_TIME_DAY, 15 * RRD_TIME_MINUTE) )); $RRD->addRRA(RRDTool_Archive::createRRACF( RRD_RRA_CF_MAX, 0.5, $RRD->getPDPIC(RRD_TIME_HOUR), $RRD->getPDPIC(RRD_TIME_WEEK, RRD_TIME_HOUR) )); $RRD->addRRA(RRDTool_Archive::createRRACF( RRD_RRA_CF_MAX, 0.5, $RRD->getPDPIC(4 * RRD_TIME_HOUR), $RRD->getPDPIC(RRD_TIME_MONTH, 4 * RRD_TIME_HOUR) )); $RRD->addRRA(RRDTool_Archive::createRRACF( RRD_RRA_CF_MAX, 0.5, $RRD->getPDPIC(12 * RRD_TIME_HOUR), $RRD->getPDPIC(RRD_TIME_YEAR, 12 * RRD_TIME_HOUR) )); $RRD->Save('net.rrd'); } --- The rrdtool.php is still in developement and thus without any real function yet (I needed the rrd_info function first for reading back structural information). What the above code will create should be obvious. As I said: It's a bit more verbose, but much more logical to the causal observer. The Function getPDPIC stands for Primary Data Point Interval Calculation ... What it does should be obvious :P >> Some work on Open Basedir Restriction can be found at >> http://blog.benny-baumann.de/?p=352 (German). The patch there isn't >> fully up-to-date, bbut basically works (except for one minor typo I >> already fixed in my local dev version). >> > I hadn't considered open_basedir, but it seems like a good idea to add > this safety feature. Enforcing open_basedir on the actual RRDs > referenced in graph, xport, etc will be more difficult though! > Yes. Correct, though this might require some parsing on the parameters given. I spared that point for now though as xport and graph don't write to files referenced in the parameter strings, yet only to the filename given as target. And as RRD rejects files not confirming to its fileformat no real harm should be possible (You only might compromise existing RRDs if you know where they are). But completing the Basedir Restriction should be included in a final version of the extension. That's why I emphasized this in my blog posting, as the current implementation leaves some holes people should be aware of when using this extension. >> Would be nice if we could exchange our versions to merge the changes, >> thus avoiding duplicate work. >> > Agreed, any suggestions on the best way to do that? > I have my version in a SVN repo. You either could: 1. Send me your file (or attach it) and I'll merge it into my SVN, sending back the result OR 2. Send me a username+APR1 password hash (htpasswd) and I give you RW access on that repository. OR 3. We use a completely different approach :P >> What do you think of offering a callback based variant in addition to a >> file based variant thus programs that need the returned data in a buffer >> can grab it without needing to write to the filesystem? >> >> Other functions (e.g. for rrd_graph) that write to disk might use such a >> callback too, or offer returning the ressources they created to the >> caller (i.e. giving the generated image as a internal ressource to the >> caller). >> > I can see how this might be a useful thing to support, in the case of > rrd_graph I was considering implementing and interface to graphv which > would allow returning the image data as a php string directly, but the > volume of data returned by rrd_dump can obviously be much larger than > the typical graph image. > data as string shouldn't be wrong neither (IIRC GD supports loading images from string input - and be it (ab)using some fopen wrapper magic). > I typically output the graph images to temporary files named according > to a hash of the graph arguments, which allows me to re-use graph images > for multiple clients without needing to regenerate them from the source > rrds (very useful when multiple clients are requesting identical graphs). > I see. The downside (and I exactly have this issue on my server) is if your HDD is connected by slow media (e.g. iSCSI) and thus reading from or writing to disk takes considerable time, compared to allocating about 100k. > I'm no expert but it seems like the data will still all need to be > collected into a buffer before it can be returned to the calling php > script as a string or php array, so in the case of rrd_dump it may be > more efficient to write out the xml to disk and perform any operations > on it using a stream-based xml parser. > Well, given the callback you can essentially just do exactly this: Every time you get a new line from rrd_dump you can feed it as next input to the XML parser. Anyway for my implementation of rrd_dump I want the extension function to return the output as a bare string - as if you hade piped the output into a memory buffer. Using such a callback method seems most reasonable to me as you should do as much as possible in primary memory. > Dan > Regards, BenBE. _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
oetiker
|
In reply to this post
by Benny Baumann
Hi Benny,
Jul 30 Benny Baumann wrote: > Hi, > > I recently discovered the PHP extension for RRDTool which is available > as a contribution to RRDTool. This extension runs successfully with > latest PHP5, but misses some functions of current RRDTool releases (no > rrd_dump, no rrd_info). I'm currently working on updating the PHP > extension to also include these operations. > > And here comes the issue: The RRDTool library is more or less written to > be used in the command line interface thus either returns only single > values or dumps output to a file\terminal. For the PHP extension I need > output to be written into a buffer though. > > To solve this I suggest adding a callback interface to e.g. rrd_dump > that is called whenever string data (or binary data) has to be returned. > An example implementation for rrd_dump_opt_cb_r can be found in the > attached patch. Feedback appreciated. IDK if this patch is complete and > if the mentioned rrd_dump_opt_cb_r function is properly exported; but at > least dumping should work. > > The basic rrd_dump_opt_r now calls this Callback method internally to > give an example of how this callback interface can be used and to avoid > duplicate source. the patch seems sane to me from looking at your code ... I will be glad to include it if you can provide a version that applies to svn://svn.oetiker.ch/rrdtool/trunk/program also you should add a section to doc/librrd.pod explaining this new access method ... cheers tobi > > Regards, > BenBE. > -- Tobi Oetiker, OETIKER+PARTNER AG, Aarweg 15 CH-4600 Olten, Switzerland http://it.oetiker.ch [hidden email] ++41 62 775 9902 / sb: -9900 _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
Yann JOUANIN
|
Hello list,
It seems the behavior of RRD client when using RRDCacheD can make trouble when using RRDCacheD on another host than the one where using client. The translation between relative path and absolute path is done in client code (rrdc_flush) while it should only be done by the server. This causes update to fail because the path is badly rewritten. I had to comment the following line to fix this. int rrdc_flush (const char *filename) /* {{{ */ { ......... /* change to absolute path for rrdcached */ /* if (*filename != '/' && realpath(filename, file_path) != NULL) filename = file_path; */ ............ Yann Jouanin http://www.yannj.fr _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
oetiker
|
Hi Yann,
Today Yann Jouanin wrote: > Hello list, > > It seems the behavior of RRD client when using RRDCacheD can make trouble > when using RRDCacheD on another host than the one where using client. > > The translation between relative path and absolute path is done in client > code (rrdc_flush) while it should only be done by the server. > > This causes update to fail because the path is badly rewritten. > I had to comment the following line to fix this. > > > > int rrdc_flush (const char *filename) /* {{{ */ > { > ......... > > > /* change to absolute path for rrdcached */ > /* if (*filename != '/' && realpath(filename, file_path) != NULL) > filename = file_path; */ > ............ Yann, can you elaborate how you would handle relative path names on the server side ? cheers tobi -- Tobi Oetiker, OETIKER+PARTNER AG, Aarweg 15 CH-4600 Olten, Switzerland http://it.oetiker.ch [hidden email] ++41 62 775 9902 / sb: -9900 _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
Yann JOUANIN
|
Isn't it already done by this function in RRDCacheD and defining base dir?
/* when using a base dir, convert relative paths to absolute paths. * if necessary, modifies the "filename" pointer to point * to the new path created in "tmp". "tmp" is provided * by the caller and sizeof(tmp) must be >= PATH_MAX. * * this allows us to optimize for the expected case (absolute path) * with a no-op. */ 1059 static void get_abs_path(char **filename, char *tmp) Yann Jouanin http://www.yannj.fr -----Original Message----- From: Tobias Oetiker [mailto:[hidden email]] Sent: Saturday, August 08, 2009 4:19 PM To: Yann Jouanin Cc: [hidden email] Subject: Re: [rrd-developers] RRDCacheD - Client rewriting path Hi Yann, Today Yann Jouanin wrote: > Hello list, > > It seems the behavior of RRD client when using RRDCacheD can make trouble > when using RRDCacheD on another host than the one where using client. > > The translation between relative path and absolute path is done in client > code (rrdc_flush) while it should only be done by the server. > > This causes update to fail because the path is badly rewritten. > I had to comment the following line to fix this. > > > > int rrdc_flush (const char *filename) /* {{{ */ > { > ......... > > > /* change to absolute path for rrdcached */ > /* if (*filename != '/' && realpath(filename, file_path) != NULL) > filename = file_path; */ > ............ Yann, can you elaborate how you would handle relative path names on the server side ? cheers tobi -- Tobi Oetiker, OETIKER+PARTNER AG, Aarweg 15 CH-4600 Olten, Switzerland http://it.oetiker.ch [hidden email] ++41 62 775 9902 / sb: -9900 _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
Benny Baumann
|
In reply to this post
by oetiker
Hi Tobias,
Am 08.08.2009 11:49, schrieb Tobias Oetiker: > Hi Benny, > > the patch seems sane to me from looking at your code ... I will be > glad to include it if you can provide a version that applies to > > svn://svn.oetiker.ch/rrdtool/trunk/program > As the version available in the public download directory and the SVN differ quite a bit, I had some work on merging in my changes, thus you might notice some differences between the old and the new API extension patch. In particular I chose to rename the API function from rrd_dump_opt_cb_r to rrd_dump_cb_r. I furthermore hope I didn't miss any changes made in the SVN when I merged the files. > also you should add a section to doc/librrd.pod explaining this new > access method ... > The API documentation explains the usage of this interface using the internally used rrd_dump_opt_cb_fileout callback which should, because of its simplicity, be a well-enough example. > cheers > tobi > IDK if the rrd_dump_opt_r function should be exported too (and not only used internally wrapped by rrd_dump and rrd_dump_r).? Anyway: The working patch, based on revision 1886 is attached. If there are any questions left, feel free to contact me. Regards, BenBE. [rrd_dump_cb_r.patch] Index: CONTRIBUTORS =================================================================== --- CONTRIBUTORS (Revision 1886) +++ CONTRIBUTORS (Arbeitskopie) @@ -9,6 +9,7 @@ Amos Shapira <amos with gezernet.co.il> Andreas Kroomaa <andre with ml.ee> Andrew Turner <turner with mint.net> (LAST consolidator) +Benny Baumann <benbe with geshi.org) rrd_dump with callback support Bernard Fischer <bfischer with syslog.ch> 64bit stuff, --alt-autoscale-max Bernhard Fischer <rep dot dot dot nop with gmail.com> MMAP rewrite Bill Fenner <fenner with research.att.com> Index: doc/librrd.pod =================================================================== --- doc/librrd.pod (Revision 1886) +++ doc/librrd.pod (Arbeitskopie) @@ -58,4 +58,41 @@ rrd_free_ptrs(&arr, &arr_size); /* here, arr == NULL && arr_size == 0 */ +=item B<rrd_dump_cr_r(char *filename, int opt_header, rrd_output_callback_t cb, void *user)> + +In some situations it is necessary to get the output of C<rrd_dump> without +writing it to a file or the standard output. In such cases an application +can ask B<rrd_dump_cb_r> to call an user-defined function each time there +is output to be stored somewhere. This can be used, to e.g. directly feed +an XML parser with the dumped output or transfer the resulting string +in memory. + +The arguments for B<rrd_dump_cb_r> are the same as for B<rrd_dump_opt_r> +except that the output filename parameter is replaced by the user-defined +callback function and an additional parameter for the callback function +that is passed untouched, i.e. to store information about the callback state +needed for the user-defined callback to function properly. + +Recent versions of B<rrd_dump_opt_r> internally use this callback mechanism +to write their output to the file provided by the user. + + size_t rrd_dump_opt_cb_fileout( + const void *data, + size_t len, + void *user) + { + return fwrite(data, 1, len, (FILE *)user); + } + +The associated call for B<rrd_dump_cb_r> looks like + + res = rrd_dump_cb_r(filename, opt_header, + rrd_dump_opt_cb_fileout, (void *)out_file); + +where the last parameter specifies the file handle B<rrd_dump_opt_cb_fileout> +should write to. There's no specific condition for the callback to detect +when it is called for the first time, nor for the last time. If you require +this for initialization and cleanup you should do those tasks before and +after calling B<rrd_dump_cr_r> respectively. + =back Index: src/rrd.h =================================================================== --- src/rrd.h (Revision 1886) +++ src/rrd.h (Arbeitskopie) @@ -130,6 +130,10 @@ struct rrd_info_t *next; } rrd_info_t; + typedef size_t (* rrd_output_callback_t)( + const void *, + size_t, + void *); /* main function blocks */ int rrd_create( @@ -252,6 +256,12 @@ const char *filename, int rraindex); + int rrd_dump_cb_r( + const char *filename, + int opt_header, + rrd_output_callback_t cb, + void *user); + /* Transplanted from rrd_parsetime.h */ typedef enum { ABSOLUTE_TIME, Index: src/rrd_dump.c =================================================================== --- src/rrd_dump.c (Revision 1886) +++ src/rrd_dump.c (Arbeitskopie) @@ -49,123 +49,172 @@ #include <locale.h> #endif - #if !(defined(NETWARE) || defined(WIN32)) extern char *tzname[2]; #endif -static int rrd_dump_opt_r( +//Local prototypes +size_t rrd_dump_opt_cb_fileout( + const void *data, + size_t len, + void *user); + + +int rrd_dump_cb_r( const char *filename, - char *outname, - int opt_header) + int opt_header, + rrd_output_callback_t cb, + void *user) { unsigned int i, ii, ix, iii = 0; time_t now; char somestring[255]; + char somestring_buf[255]; rrd_value_t my_cdp; off_t rra_base, rra_start, rra_next; rrd_file_t *rrd_file; - FILE *out_file; rrd_t rrd; rrd_value_t value; struct tm tm; char *old_locale = ""; + //Check if we got a (valid) callback method + if (!cb) { + return (-1); + } + rrd_init(&rrd); + rrd_file = rrd_open(filename, &rrd, RRD_READONLY | RRD_READAHEAD); if (rrd_file == NULL) { rrd_free(&rrd); return (-1); } - out_file = NULL; - if (outname) { - if (!(out_file = fopen(outname, "w"))) { - return (-1); - } - } else { - out_file = stdout; - } #ifdef HAVE_SETLOCALE old_locale = setlocale(LC_NUMERIC, "C"); #endif if (opt_header == 1) { - fputs("<?xml version=\"1.0\" encoding=\"utf-8\"?>\n", out_file); - fputs - ("<!DOCTYPE rrd SYSTEM \"http://oss.oetiker.ch/rrdtool/rrdtool.dtd\">\n", - out_file); - fputs("<!-- Round Robin Database Dump -->\n", out_file); - fputs("<rrd>\n", out_file); + cb("<?xml version=\"1.0\" encoding=\"utf-8\"?>\n", + strlen("<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"), user); + cb("<!DOCTYPE rrd SYSTEM \"http://oss.oetiker.ch/rrdtool/rrdtool.dtd\">\n", + strlen("<!DOCTYPE rrd SYSTEM \"http://oss.oetiker.ch/rrdtool/rrdtool.dtd\">\n"), user); + cb("<!-- Round Robin Database Dump -->\n", + strlen("<!-- Round Robin Database Dump -->\n"), user); + cb("<rrd>\n", + strlen("<rrd>\n"), user); } else if (opt_header == 2) { - fputs("<?xml version=\"1.0\" encoding=\"utf-8\"?>\n", out_file); - fputs("<!-- Round Robin Database Dump -->\n", out_file); - fputs("<rrd xmlns=\"http://oss.oetiker.ch/rrdtool/rrdtool-dump.xml\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"\n", out_file); - fputs("\txsi:schemaLocation=\"http://oss.oetiker.ch/rrdtool/rrdtool-dump.xml http://oss.oetiker.ch/rrdtool/rrdtool-dump.xsd\">\n", out_file); + cb("<?xml version=\"1.0\" encoding=\"utf-8\"?>\n", + strlen("<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"), user); + cb("<!-- Round Robin Database Dump -->\n", + strlen("<!-- Round Robin Database Dump -->\n"), user); + cb("<rrd xmlns=\"http://oss.oetiker.ch/rrdtool/rrdtool-dump.xml\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"\n", + strlen("<rrd xmlns=\"http://oss.oetiker.ch/rrdtool/rrdtool-dump.xml\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\"\n"), user); + cb("\txsi:schemaLocation=\"http://oss.oetiker.ch/rrdtool/rrdtool-dump.xml http://oss.oetiker.ch/rrdtool/rrdtool-dump.xsd\">\n", + strlen("\txsi:schemaLocation=\"http://oss.oetiker.ch/rrdtool/rrdtool-dump.xml http://oss.oetiker.ch/rrdtool/rrdtool-dump.xsd\">\n"), user); } else { - fputs("<!-- Round Robin Database Dump -->\n", out_file); - fputs("<rrd>\n", out_file); + cb("<!-- Round Robin Database Dump -->\n", + strlen("<!-- Round Robin Database Dump -->\n"), user); + cb("<rrd>\n", + strlen("<rrd>\n"), user); } + cb("<!-- Round Robin Database Dump -->", + strlen("<!-- Round Robin Database Dump -->"), user); + + cb("<rrd>", + strlen("<rrd>"), user); + if (atoi(rrd.stat_head->version) <= 3) { - fprintf(out_file, "\t<version>%s</version>\n", RRD_VERSION3); + snprintf(somestring, 255, "\t<version>%s</version>\n", RRD_VERSION3); } else { - fprintf(out_file, "\t<version>%s</version>\n", RRD_VERSION); + snprintf(somestring, 255, "\t<version>%s</version>\n", RRD_VERSION); } - fprintf(out_file, "\t<step>%lu</step> <!-- Seconds -->\n", - rrd.stat_head->pdp_step); + cb(somestring, strlen(somestring), user); + + snprintf(somestring, 255, "\t<step>%lu</step> <!-- Seconds -->\n", + rrd.stat_head->pdp_step); + cb(somestring, strlen(somestring), user); + #ifdef HAVE_STRFTIME localtime_r(&rrd.live_head->last_up, &tm); - strftime(somestring, 200, "%Y-%m-%d %H:%M:%S %Z", &tm); + strftime(somestring, 255, "%Y-%m-%d %H:%M:%S %Z", &tm); #else # error "Need strftime" #endif - fprintf(out_file, "\t<lastupdate>%lld</lastupdate> <!-- %s -->\n\n", - (long long) rrd.live_head->last_up, somestring); + snprintf(somestring_buf, 255, "\t<lastupdate>%lld</lastupdate> <!-- %s -->\n\n", + (long long) rrd.live_head->last_up, somestring); + cb(somestring_buf, strlen(somestring_buf), user); for (i = 0; i < rrd.stat_head->ds_cnt; i++) { - fprintf(out_file, "\t<ds>\n"); - fprintf(out_file, "\t\t<name>%s</name>\n", rrd.ds_def[i].ds_nam); - fprintf(out_file, "\t\t<type>%s</type>\n", rrd.ds_def[i].dst); + cb("\t<ds>\n", + strlen("\t<ds>\n"), user); + + snprintf(somestring_buf, 255, "\t\t<name> %s </name>\n", rrd.ds_def[i].ds_nam); + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, "\t\t<type> %s </type>\n", rrd.ds_def[i].dst); + cb(somestring_buf, strlen(somestring_buf), user); + if (dst_conv(rrd.ds_def[i].dst) != DST_CDEF) { - fprintf(out_file, - "\t\t<minimal_heartbeat>%lu</minimal_heartbeat>\n", + snprintf(somestring_buf, 255, "\t\t<minimal_heartbeat>%lu</minimal_heartbeat>\n", rrd.ds_def[i].par[DS_mrhb_cnt].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); + if (isnan(rrd.ds_def[i].par[DS_min_val].u_val)) { - fprintf(out_file, "\t\t<min>NaN</min>\n"); + snprintf(somestring_buf, 255, "\t\t<min>NaN</min>\n"); } else { - fprintf(out_file, "\t\t<min>%0.10e</min>\n", - rrd.ds_def[i].par[DS_min_val].u_val); + snprintf(somestring_buf, 255, "\t\t<min>%0.10e</min>\n", + rrd.ds_def[i].par[DS_min_val].u_val); } + cb(somestring_buf, strlen(somestring_buf), user); + if (isnan(rrd.ds_def[i].par[DS_max_val].u_val)) { - fprintf(out_file, "\t\t<max>NaN</max>\n"); + snprintf(somestring_buf, 255, "\t\t<max>NaN</max>\n"); } else { - fprintf(out_file, "\t\t<max>%0.10e</max>\n", - rrd.ds_def[i].par[DS_max_val].u_val); + snprintf(somestring_buf, 255, "\t\t<max>%0.10e</max>\n", + rrd.ds_def[i].par[DS_max_val].u_val); } + cb(somestring_buf, strlen(somestring_buf), user); } else { /* DST_CDEF */ char *str = NULL; rpn_compact2str((rpn_cdefds_t *) &(rrd.ds_def[i].par[DS_cdef]), - rrd.ds_def, &str); - fprintf(out_file, "\t\t<cdef>%s</cdef>\n", str); + rrd.ds_def, &str); + + //Splitting into 3 writes to avoid allocating memory + //This is better compared to snprintf as str may be of arbitrary size + cb("\t\t<cdef> ", strlen("\t\t<cdef>"), user); + cb(str, strlen(str), user); + cb(" </cdef>\n", strlen("</cdef>\n"), user); + free(str); } - fprintf(out_file, "\n\t\t<!-- PDP Status -->\n"); - fprintf(out_file, "\t\t<last_ds>%s</last_ds>\n", - rrd.pdp_prep[i].last_ds); + + cb("\n\t\t<!-- PDP Status -->\n", + strlen("\n\t\t<!-- PDP Status -->\n"), user); + snprintf(somestring_buf, 255, "\t\t<last_ds>%s</last_ds>\n", + rrd.pdp_prep[i].last_ds); + cb(somestring_buf, strlen(somestring_buf), user); + if (isnan(rrd.pdp_prep[i].scratch[PDP_val].u_val)) { - fprintf(out_file, "\t\t<value>NaN</value>\n"); + snprintf(somestring_buf, 255, "\t\t<value>NaN</value>\n"); } else { - fprintf(out_file, "\t\t<value>%0.10e</value>\n", - rrd.pdp_prep[i].scratch[PDP_val].u_val); + snprintf(somestring_buf, 255, "\t\t<value>%0.10e</value>\n", + rrd.pdp_prep[i].scratch[PDP_val].u_val); } - fprintf(out_file, "\t\t<unknown_sec>%lu</unknown_sec>\n", - rrd.pdp_prep[i].scratch[PDP_unkn_sec_cnt].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); - fprintf(out_file, "\t</ds>\n\n"); + snprintf(somestring_buf, 255, "\t\t<unknown_sec> %lu </unknown_sec>\n", + rrd.pdp_prep[i].scratch[PDP_unkn_sec_cnt].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); + + cb("\t</ds>\n\n", + strlen("\t</ds>\n\n"), user); } - fputs("<!-- Round Robin Archives -->\n", out_file); + cb("<!-- Round Robin Archives -->\n", + strlen("<!-- Round Robin Archives -->\n"), user); rra_base = rrd_file->header_len; rra_next = rra_base; @@ -177,182 +226,224 @@ rra_start = rra_next; rra_next += (rrd.stat_head->ds_cnt * rrd.rra_def[i].row_cnt * sizeof(rrd_value_t)); - fprintf(out_file, "\t<rra>\n"); - fprintf(out_file, "\t\t<cf>%s</cf>\n", rrd.rra_def[i].cf_nam); - fprintf(out_file, - "\t\t<pdp_per_row>%lu</pdp_per_row> <!-- %lu seconds -->\n\n", - rrd.rra_def[i].pdp_cnt, - rrd.rra_def[i].pdp_cnt * rrd.stat_head->pdp_step); + + cb("\t<rra>\n", + strlen("\t<rra>\n"), user); + + snprintf(somestring_buf, 255, + "\t\t<cf>%s</cf>\n", rrd.rra_def[i].cf_nam); + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, + "\t\t<pdp_per_row>%lu</pdp_per_row> <!-- %lu seconds -->\n\n", + rrd.rra_def[i].pdp_cnt, + rrd.rra_def[i].pdp_cnt * rrd.stat_head->pdp_step); + cb(somestring_buf, strlen(somestring_buf), user); + /* support for RRA parameters */ - fprintf(out_file, "\t\t<params>\n"); + cb("\t\t<params>\n", + strlen("\t\t<params>\n"), user); + switch (cf_conv(rrd.rra_def[i].cf_nam)) { case CF_HWPREDICT: case CF_MHWPREDICT: - fprintf(out_file, "\t\t<hw_alpha>%0.10e</hw_alpha>\n", + snprintf(somestring_buf, 255, "\t\t<hw_alpha>%0.10e</hw_alpha>\n", rrd.rra_def[i].par[RRA_hw_alpha].u_val); - fprintf(out_file, "\t\t<hw_beta>%0.10e</hw_beta>\n", + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, "\t\t<hw_beta>%0.10e</hw_beta>\n", rrd.rra_def[i].par[RRA_hw_beta].u_val); - fprintf(out_file, + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, "\t\t<dependent_rra_idx>%lu</dependent_rra_idx>\n", rrd.rra_def[i].par[RRA_dependent_rra_idx].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); break; case CF_SEASONAL: case CF_DEVSEASONAL: - fprintf(out_file, + snprintf(somestring_buf, 255, "\t\t<seasonal_gamma>%0.10e</seasonal_gamma>\n", rrd.rra_def[i].par[RRA_seasonal_gamma].u_val); - fprintf(out_file, + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, "\t\t<seasonal_smooth_idx>%lu</seasonal_smooth_idx>\n", rrd.rra_def[i].par[RRA_seasonal_smooth_idx].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); + if (atoi(rrd.stat_head->version) >= 4) { - fprintf(out_file, - "\t\t<smoothing_window>%0.10e</smoothing_window>\n", - rrd.rra_def[i].par[RRA_seasonal_smoothing_window]. - u_val); + snprintf(somestring_buf, 255, + "\t\t<smoothing_window>%0.10e</smoothing_window>\n", + rrd.rra_def[i].par[RRA_seasonal_smoothing_window].u_val); + cb(somestring_buf, strlen(somestring_buf), user); } - fprintf(out_file, + snprintf(somestring_buf, 255, "\t\t<dependent_rra_idx>%lu</dependent_rra_idx>\n", rrd.rra_def[i].par[RRA_dependent_rra_idx].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); break; case CF_FAILURES: - fprintf(out_file, "\t\t<delta_pos>%0.10e</delta_pos>\n", + snprintf(somestring_buf, 255, "\t\t<delta_pos>%0.10e</delta_pos>\n", rrd.rra_def[i].par[RRA_delta_pos].u_val); - fprintf(out_file, "\t\t<delta_neg>%0.10e</delta_neg>\n", + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, "\t\t<delta_neg>%0.10e</delta_neg>\n", rrd.rra_def[i].par[RRA_delta_neg].u_val); - fprintf(out_file, "\t\t<window_len>%lu</window_len>\n", + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, "\t\t<window_len>%lu</window_len>\n", rrd.rra_def[i].par[RRA_window_len].u_cnt); - fprintf(out_file, + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, "\t\t<failure_threshold>%lu</failure_threshold>\n", rrd.rra_def[i].par[RRA_failure_threshold].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); + /* fall thru */ case CF_DEVPREDICT: - fprintf(out_file, + snprintf(somestring_buf, 255, "\t\t<dependent_rra_idx>%lu</dependent_rra_idx>\n", rrd.rra_def[i].par[RRA_dependent_rra_idx].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); break; case CF_AVERAGE: case CF_MAXIMUM: case CF_MINIMUM: case CF_LAST: default: - fprintf(out_file, "\t\t<xff>%0.10e</xff>\n", + snprintf(somestring_buf, 255, "\t\t<xff>%0.10e</xff>\n", rrd.rra_def[i].par[RRA_cdp_xff_val].u_val); + cb(somestring_buf, strlen(somestring_buf), user); break; } - fprintf(out_file, "\t\t</params>\n"); - fprintf(out_file, "\t\t<cdp_prep>\n"); + + cb("\t\t</params>\n", + strlen("\t\t</params>\n"), user); + cb("\t\t<cdp_prep>\n", + strlen("\t\t<cdp_prep>\n"), user); + for (ii = 0; ii < rrd.stat_head->ds_cnt; ii++) { unsigned long ivalue; - fprintf(out_file, "\t\t\t<ds>\n"); + cb("\t\t\t<ds>\n", + strlen("\t\t\t<ds>\n"), user); /* support for exporting all CDP parameters */ /* parameters common to all CFs */ /* primary_val and secondary_val do not need to be saved between updates * so strictly speaking they could be omitted. * However, they can be useful for diagnostic purposes, so are included here. */ - value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt - + ii].scratch[CDP_primary_val].u_val; + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_primary_val].u_val; if (isnan(value)) { - fprintf(out_file, - "\t\t\t<primary_value>NaN</primary_value>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<primary_value>NaN</primary_value>\n"); } else { - fprintf(out_file, - "\t\t\t<primary_value>%0.10e</primary_value>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<primary_value>%0.10e</primary_value>\n", value); } - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_secondary_val].u_val; + cb(somestring_buf, strlen(somestring_buf), user); + + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_secondary_val].u_val; if (isnan(value)) { - fprintf(out_file, - "\t\t\t<secondary_value>NaN</secondary_value>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<secondary_value>NaN</secondary_value>\n"); } else { - fprintf(out_file, - "\t\t\t<secondary_value>%0.10e</secondary_value>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<secondary_value>%0.10e</secondary_value>\n", value); } + cb(somestring_buf, strlen(somestring_buf), user); + switch (cf_conv(rrd.rra_def[i].cf_nam)) { case CF_HWPREDICT: case CF_MHWPREDICT: - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_hw_intercept].u_val; + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_hw_intercept].u_val; if (isnan(value)) { - fprintf(out_file, "\t\t\t<intercept>NaN</intercept>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<intercept>NaN</intercept>\n"); } else { - fprintf(out_file, - "\t\t\t<intercept>%0.10e</intercept>\n", value); + snprintf(somestring_buf, 255, + "\t\t\t<intercept>%0.10e</intercept>\n", value); } - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_hw_last_intercept].u_val; + cb(somestring_buf, strlen(somestring_buf), user); + + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_hw_last_intercept].u_val; if (isnan(value)) { - fprintf(out_file, - "\t\t\t<last_intercept>NaN</last_intercept>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<last_intercept>NaN</last_intercept>\n"); } else { - fprintf(out_file, - "\t\t\t<last_intercept>%0.10e</last_intercept>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<last_intercept>%0.10e</last_intercept>\n", value); } - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_hw_slope].u_val; + cb(somestring_buf, strlen(somestring_buf), user); + + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_hw_slope].u_val; if (isnan(value)) { - fprintf(out_file, "\t\t\t<slope>NaN</slope>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<slope>NaN</slope>\n"); } else { - fprintf(out_file, "\t\t\t<slope>%0.10e</slope>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<slope>%0.10e</slope>\n", value); } - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_hw_last_slope].u_val; + cb(somestring_buf, strlen(somestring_buf), user); + + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_hw_last_slope].u_val; if (isnan(value)) { - fprintf(out_file, - "\t\t\t<last_slope>NaN</last_slope>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<last_slope>NaN</last_slope>\n"); } else { - fprintf(out_file, - "\t\t\t<last_slope>%0.10e</last_slope>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<last_slope>%0.10e</last_slope>\n", value); } - ivalue = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_null_count].u_cnt; - fprintf(out_file, "\t\t\t<nan_count>%lu</nan_count>\n", - ivalue); - ivalue = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_last_null_count].u_cnt; - fprintf(out_file, - "\t\t\t<last_nan_count>%lu</last_nan_count>\n", - ivalue); + cb(somestring_buf, strlen(somestring_buf), user); + + ivalue = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_null_count].u_cnt; + snprintf(somestring_buf, 255, + "\t\t\t<nan_count>%lu</nan_count>\n", ivalue); + cb(somestring_buf, strlen(somestring_buf), user); + + ivalue = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_last_null_count].u_cnt; + snprintf(somestring_buf, 255, + "\t\t\t<last_nan_count>%lu</last_nan_count>\n", ivalue); + cb(somestring_buf, strlen(somestring_buf), user); break; case CF_SEASONAL: case CF_DEVSEASONAL: - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_hw_seasonal].u_val; + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_hw_seasonal].u_val; if (isnan(value)) { - fprintf(out_file, "\t\t\t<seasonal>NaN</seasonal>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<seasonal>NaN</seasonal>\n"); } else { - fprintf(out_file, "\t\t\t<seasonal>%0.10e</seasonal>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<seasonal>%0.10e</seasonal>\n", value); } - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_hw_last_seasonal].u_val; + cb(somestring_buf, strlen(somestring_buf), user); + + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_hw_last_seasonal].u_val; if (isnan(value)) { - fprintf(out_file, - "\t\t\t<last_seasonal>NaN</last_seasonal>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<last_seasonal>NaN</last_seasonal>\n"); } else { - fprintf(out_file, - "\t\t\t<last_seasonal>%0.10e</last_seasonal>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<last_seasonal>%0.10e</last_seasonal>\n", value); } - ivalue = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_init_seasonal].u_cnt; - fprintf(out_file, "\t\t\t<init_flag>%lu</init_flag>\n", - ivalue); + cb(somestring_buf, strlen(somestring_buf), user); + + ivalue = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_init_seasonal].u_cnt; + snprintf(somestring_buf, 255, + "\t\t\t<init_flag>%lu</init_flag>\n", ivalue); + cb(somestring_buf, strlen(somestring_buf), user); break; case CF_DEVPREDICT: break; @@ -366,13 +457,16 @@ ds_cnt + ii]. scratch); - fprintf(out_file, "\t\t\t<history> "); + cb("\t\t\t<history> ", + strlen("\t\t\t<history> "), user); for (vidx = 0; - vidx < rrd.rra_def[i].par[RRA_window_len].u_cnt; - ++vidx) { - fprintf(out_file, "%d", violations_array[vidx]); + vidx < rrd.rra_def[i].par[RRA_window_len].u_cnt; + ++vidx) { + snprintf(somestring_buf, 255, "%d", violations_array[vidx]); + cb(somestring_buf, strlen(somestring_buf), user); } - fprintf(out_file, " </history>\n"); + cb(" </history>\n", + strlen(" </history>\n"), user); } break; case CF_AVERAGE: @@ -380,26 +474,31 @@ case CF_MINIMUM: case CF_LAST: default: - value = - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_val].u_val; + value = rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii].scratch[CDP_val].u_val; if (isnan(value)) { - fprintf(out_file, "\t\t\t<value>NaN</value>\n"); + snprintf(somestring_buf, 255, + "\t\t\t<value>NaN</value>\n"); } else { - fprintf(out_file, "\t\t\t<value>%0.10e</value>\n", - value); + snprintf(somestring_buf, 255, + "\t\t\t<value>%0.10e</value>\n", value); } - fprintf(out_file, - "\t\t\t<unknown_datapoints>%lu</unknown_datapoints>\n", - rrd.cdp_prep[i * rrd.stat_head->ds_cnt + - ii].scratch[CDP_unkn_pdp_cnt].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); + + snprintf(somestring_buf, 255, + "\t\t\t<unknown_datapoints>%lu</unknown_datapoints>\n", + rrd.cdp_prep[i * rrd.stat_head->ds_cnt + ii]. + scratch[CDP_unkn_pdp_cnt].u_cnt); + cb(somestring_buf, strlen(somestring_buf), user); break; } - fprintf(out_file, "\t\t\t</ds>\n"); + cb("\t\t\t</ds>\n", + strlen("\t\t\t</ds>\n"), user); } - fprintf(out_file, "\t\t</cdp_prep>\n"); + cb("\t\t</cdp_prep>\n", + strlen("\t\t</cdp_prep>\n"), user); - fprintf(out_file, "\t\t<database>\n"); + cb("\t\t<database>\n", + strlen("\t\t<database>\n"), user); rrd_seek(rrd_file, (rra_start + (rrd.rra_ptr[i].cur_row + 1) * rrd.stat_head->ds_cnt * sizeof(rrd_value_t)), SEEK_SET); @@ -419,36 +518,75 @@ timer++; #if HAVE_STRFTIME localtime_r(&now, &tm); - strftime(somestring, 200, "%Y-%m-%d %H:%M:%S %Z", &tm); + strftime(somestring, 255, "%Y-%m-%d %H:%M:%S %Z", &tm); #else # error "Need strftime" #endif - fprintf(out_file, "\t\t\t<!-- %s / %lld --> <row>", somestring, - (long long) now); + snprintf(somestring_buf, 255, + "\t\t\t<!-- %s / %lld --> <row>", somestring, (long long) now); + cb(somestring_buf, strlen(somestring_buf), user); for (iii = 0; iii < rrd.stat_head->ds_cnt; iii++) { rrd_read(rrd_file, &my_cdp, sizeof(rrd_value_t) * 1); if (isnan(my_cdp)) { - fprintf(out_file, "<v>NaN</v>"); + snprintf(somestring_buf, 255, "<v>NaN</v>"); } else { - fprintf(out_file, "<v>%0.10e</v>", my_cdp); - }; + snprintf(somestring_buf, 255, "<v>%0.10e</v>", my_cdp); + } + cb(somestring_buf, strlen(somestring_buf), user); } - fprintf(out_file, "</row>\n"); + cb("</row>\n", + strlen("</row>\n"), user); } - fprintf(out_file, "\t\t</database>\n\t</rra>\n"); + cb("\t\t</database>\n\t</rra>\n", + strlen("\t\t</database>\n\t</rra>\n"), user); + } - } - fprintf(out_file, "</rrd>\n"); + cb("</rrd>\n", + strlen("</rrd>\n"), user); + rrd_free(&rrd); - if (out_file != stdout) { - fclose(out_file); - } + #ifdef HAVE_SETLOCALE setlocale(LC_NUMERIC, old_locale); #endif + return rrd_close(rrd_file); } +size_t rrd_dump_opt_cb_fileout( + const void *data, + size_t len, + void *user) +{ + return fwrite(data, 1, len, (FILE *)user); +} + +int rrd_dump_opt_r( + const char *filename, + char *outname, + int opt_noheader) +{ + FILE *out_file; + int res; + + out_file = NULL; + if (outname) { + if (!(out_file = fopen(outname, "w"))) { + return (-1); + } + } else { + out_file = stdout; + } + + res = rrd_dump_cb_r(filename, opt_noheader, rrd_dump_opt_cb_fileout, (void *)out_file); + + if (out_file != stdout) { + fclose(out_file); + } + + return res; +} + /* backward compatibility with 1.2.x */ int rrd_dump_r( const char *filename, _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
kevin brintnall
|
In reply to this post
by Yann JOUANIN
On Sat, Aug 08, 2009 at 12:22:26PM +0200, Yann Jouanin wrote:
> It seems the behavior of RRD client when using RRDCacheD can make trouble > when using RRDCacheD on another host than the one where using client. > > The translation between relative path and absolute path is done in client > code (rrdc_flush) while it should only be done by the server. Yann, The client must convert to absolute path because the daemon has no idea what the cwd is of the client. This is true even when the client and server run on the same machine. i.e.: rrdtool update 1.rrd N:1:2 cd elsewhere rrdtool update 1.rrd N:1:2 > This causes update to fail because the path is badly rewritten. > I had to comment the following line to fix this. You could: * use the same file hierarchy on client and server * speak to the daemon directly from your application -- kevin brintnall =~ /[hidden email]/ _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
Sebastian Harl
|
Hi Kevin, et.al.,
On Sun, Aug 09, 2009 at 03:44:41PM -0500, kevin brintnall wrote: > On Sat, Aug 08, 2009 at 12:22:26PM +0200, Yann Jouanin wrote: > > It seems the behavior of RRD client when using RRDCacheD can make trouble > > when using RRDCacheD on another host than the one where using client. > > > > The translation between relative path and absolute path is done in client > > code (rrdc_flush) while it should only be done by the server. > The client must convert to absolute path because the daemon has no idea > what the cwd is of the client. This is true even when the client and > server run on the same machine. i.e.: > > rrdtool update 1.rrd N:1:2 > cd elsewhere > rrdtool update 1.rrd N:1:2 Hrm, I'm not sure that I agree with that. Why does the client's current working directory matter at all to the server? Think about other ways of remote access to files - all of them use some kind of base directory that's used whenever relative path names are specified: scp uses $HOME, Apache uses the document root, git-daemon supports a base path, etc. So, I think that by following the principle of least surprise RRDcacheD should behave like that as well. Let people think about $HOST:1.rrd instead of 1.rrd (where $HOST might be remote or local) - we're referring to a file on some $HOST after all … I agree that it's not quite as obvious when using RRDcacheD locally only (and I guess that's why it's currently implemented that way, since that was the very first approach). Then, you'll actually see the files in your local file system, so from a user's POV, $CWD probably does matter. So, maybe the question is: Is RRDcacheD supposed to be a networking daemon or rather some service that just happens to support remote access as well because it was easy and convenient to implement that as well? I'd probably go for the former and I'd suppose that this is going to be the, by far, more common use case. Then again, I guess you can find a few more pros and cons for either way. So, maybe the actual problem is that too much magic has been put into the way the daemon may be enabled (on the client side). By allowing to hide the fact that, in fact, you're accessing some daemon (e.g. by specifying an environment variable) it might not be obvious what's going on and thus either way might be confusing for a rather large fraction of users. OTOH, users who hide that fact have to do so explicitly, so they should know what they are doing. So, again, I'm in favor of not letting the client rewrite any path names. Just my 2 ¢. Cheers, Sebastian -- Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/ Those who would give up Essential Liberty to purchase a little Temporary Safety, deserve neither Liberty nor Safety. -- Benjamin Franklin _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
kevin brintnall
|
> Hrm, I'm not sure that I agree with that. Why does the client's current
> working directory matter at all to the server? Think about other ways of > remote access to files - all of them use some kind of base directory > that's used whenever relative path names are specified: scp uses $HOME, > Apache uses the document root, git-daemon supports a base path, etc. So, > I think that by following the principle of least surprise RRDcacheD > should behave like that as well. Let people think about $HOST:1.rrd > instead of 1.rrd (where $HOST might be remote or local) - we're > referring to a file on some $HOST after all ??? The existing code allows the daemon to work with any existing client work-flow without requiring them to change their code. That is important. -- kevin brintnall =~ /[hidden email]/ _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
Benny Baumann
|
In reply to this post
by Benny Baumann
Short correction since I missed some changes in the original patch:
Regards, BenBE. Index: a/src/rrd_dump.c =================================================================== --- a/src/rrd_dump.c (old) +++ b/src/rrd_dump.c (new) @@ -120,12 +120,6 @@ strlen("<rrd>\n"), user); } - cb("<!-- Round Robin Database Dump -->", - strlen("<!-- Round Robin Database Dump -->"), user); - - cb("<rrd>", - strlen("<rrd>"), user); - if (atoi(rrd.stat_head->version) <= 3) { snprintf(somestring, 255, "\t<version>%s</version>\n", RRD_VERSION3); } else { @@ -207,8 +207,8 @@ strlen("\t</ds>\n\n"), user); } - cb("<!-- Round Robin Archives -->\n", - strlen("<!-- Round Robin Archives -->\n"), user); + cb("\t<!-- Round Robin Archives -->\n", + strlen("\t<!-- Round Robin Archives -->\n"), user); rra_base = rrd_file->header_len; rra_next = rra_base; _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
Florian Forster
|
In reply to this post
by Sebastian Harl
Hi,
I have talked to Sebastian in person about this, but I think it'd be good if I repeat this here: On Sat, Aug 08, 2009 at 12:22:26PM +0200, Yann Jouanin wrote: > It seems the behavior of RRD client when using RRDCacheD can make > trouble when using RRDCacheD on another host than the one where using > client. The intention behind RRDCacheD was to move the cache outside of the clients and into a generic daemon. The client would communicate with the daemon using UNIX domain sockets and in general is was just two parts of one application glued together by some IPC. That's why the relative path behavior basically assumes that the daemon is on the same host. The network sockets were just added because it was only a couple of lines extra code. They were initially not meant to be used in production. > The translation between relative path and absolute path is done in > client code (rrdc_flush) while it should only be done by the server. No. Yes. Well, this depends on whether you regard RRDCacheD as a local acceleration cache or a remote, networked daemon. * If you regard RRDCacheD as a rather complex abstraction layer over your local RRD files, then having the client resolve relative names is just the thing to do. It is hard for the daemon to figure out the client's current working directory, as Kevin has pointed out. Pro: Transparent and fast update. Existing applications don't need to be changed. Con: Limitation to the same host. * If RRDCacheD is a normal networked daemon for you, then having a daemon base directory and denying all absolute paths is probably the best thing you can do. However, if we do that, we should definitely follow through: Then *all* file access has to happen via the daemon, not just write (update) access. This means we need to add read- (fetch), create-, and delete-commands as well. Sooner or later we also need some sort of file browsing. Pro: More flexibility when designing setups. Possibly better scalability. I have the impression a lot of people want this. Con: Transparency is compromised; existing applications need to handle this somehow. Security becomes an issue. On Mon, Aug 10, 2009 at 10:10:38AM +0200, Sebastian Harl wrote: > I think that by following the principle of least surprise RRDcacheD > should behave like that as well. Let people think about $HOST:1.rrd > instead of 1.rrd (where $HOST might be remote or local) - we're > referring to a file on some $HOST after all … If the daemon should work as a real network daemon, then this style of addressing is the way to go in my opinion. IIrc this was the outcome of our discussion about the ‘rrdc_fetch’ patch I have sent a while back (I have some docs ready by the way, I'll send an updated patch soon). In my ideal world, this would look somewhat like this: * The ‘rrd_*_r’ functions always work on local files, the ‘rrdc_*’ always use a daemon connection. Maybe it'd be a good idea to rename the `rrd_*_r' functions. ‘rrdl_*’ (‘l’ as in “local”) comes to mind. * Some magical version of the functions will check some sort of environment and call the appropriate function. * Files are addressed using some special syntax. Using something like rrdcache://user@host/relative/path could work okay. If ‘user’ is omitted, the local user name is used. The paths are always relative paths so the administrator can be sure all the files controlled by the daemon are beneath one directory. File names that do not contain “://” are assumed to be local files. (Personally I like the “user@host:path” syntax of ‘scp’ best, but I fear that there could be incompatibilities.) * The ‘RRDCACHE_HOST’ and ‘RRDCACHE_USER’ environment variables can be used to avoid the special syntax. If they are set, the daemon will be used. * To authenticate, the client code looks for a file called ~/.rrdcache_privkey which contains a private key file (the public key is required on the server and should be associated with the user name). If found, public key authentication is attempted, possibly via TLS. If no such file exists, the code will check for a password in ~/.rrdcache_password and, if found, will use it for password authentication. If neither file exists and the process has a controlling terminal, then it will try to read a password from /dev/console. If all else fails, try authenticating with an empty password. > So, maybe the actual problem is that too much magic has been put into > the way the daemon may be enabled (on the client side). I hope the environment variables don't add too much confusion. Other than that, I think the above approach is pretty straight forward and easy to understand. Regards, -octo -- Florian octo Forster Hacker in training GnuPG: 0x91523C3D http://verplant.org/ _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
oetiker
|
Hi Florian,
Yesterday Florian Forster wrote: > In my ideal world, this would look somewhat like this: > > * The ?rrd_*_r? functions always work on local files, the ?rrdc_*? > always use a daemon connection. Maybe it'd be a good idea to rename > the `rrd_*_r' functions. ?rrdl_*? (?l? as in ?local?) comes to mind. > > * Some magical version of the functions will check some sort of > environment and call the appropriate function. this is to maintain the present interface ? > * Files are addressed using some special syntax. Using something like > rrdcache://user@host/relative/path > could work okay. If ?user? is omitted, the local user name is used. > The paths are always relative paths so the administrator can be sure > all the files controlled by the daemon are beneath one directory. > File names that do not contain ?://? are assumed to be local files. > (Personally I like the ?user@host:path? syntax of ?scp? best, but I > fear that there could be incompatibilities.) > > * The ?RRDCACHE_HOST? and ?RRDCACHE_USER? environment variables can be > used to avoid the special syntax. If they are set, the daemon will > be used. > > * To authenticate, the client code looks for a file called > ~/.rrdcache_privkey which contains a private key file (the public > key is required on the server and should be associated with the user > name). If found, public key authentication is attempted, possibly > via TLS. > > If no such file exists, the code will check for a password in > ~/.rrdcache_password and, if found, will use it for password > authentication. > > If neither file exists and the process has a controlling terminal, > then it will try to read a password from /dev/console. > > If all else fails, try authenticating with an empty password. this does sound fine to me ... I will make a note of it on the wiki I guess its time to finally push out 1.4 so that the code can move again ... cheers tobi -- Tobi Oetiker, OETIKER+PARTNER AG, Aarweg 15 CH-4600 Olten, Switzerland http://it.oetiker.ch [hidden email] ++41 62 775 9902 / sb: -9900 _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
Florian Forster
|
Hi Tobi,
On Tue, Aug 11, 2009 at 07:48:30AM +0200, Tobias Oetiker wrote: > > * Some magical version of the functions will check some sort of > > environment and call the appropriate function. > > this is to maintain the present interface ? well, I would much rather see the end of that argv/argc style interface and have all functions use an interface in the style of the `*_r' functions. But as I said: That's my ideal dreamworld. Maybe a possible transition would be to break ABI compatibility but provide an optional legacy API. I. e. rename `rrd_update' and friends to `rrd_update_legacy' (this brakes ABI compatibility). Then provide something like this: /* declaration of rrd_update_local and rrd_update_remote */ #if RRD_LEGACY_INTERFACE /* declaration of rrd_update_legacy */ # define rrd_update rrd_update_legacy # define rrd_update_r rrd_update_local # define rrdc_update rrd_update_remote ... #else /* declaration of rrd_update */ #endif > I guess its time to finally push out 1.4 so that the code can move > again ... I'd be glad if you could consider my per-socket authorization patch for the 1.4 release. I think it's a big improvement over the ``restricted sockets'' (the `-L' command line option). I'll send an updates patch with docs in a minute.. Regards, -octo -- Florian octo Forster Hacker in training GnuPG: 0x91523C3D http://verplant.org/ _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
oetiker
|
Today Florian Forster wrote:
> Hi Tobi, > > On Tue, Aug 11, 2009 at 07:48:30AM +0200, Tobias Oetiker wrote: > > > * Some magical version of the functions will check some sort of > > > environment and call the appropriate function. > > > > this is to maintain the present interface ? > > well, I would much rather see the end of that argv/argc style interface > and have all functions use an interface in the style of the `*_r' > functions. But as I said: That's my ideal dreamworld. > > Maybe a possible transition would be to break ABI compatibility but > provide an optional legacy API. I. e. rename `rrd_update' and friends to > `rrd_update_legacy' (this brakes ABI compatibility). Then provide > something like this: > > /* declaration of rrd_update_local and rrd_update_remote */ > #if RRD_LEGACY_INTERFACE > /* declaration of rrd_update_legacy */ > # define rrd_update rrd_update_legacy > # define rrd_update_r rrd_update_local > # define rrdc_update rrd_update_remote > ... > #else > /* declaration of rrd_update */ > #endif that does sound reasonable ... I think it is important to allow for a backward compatible interface ... > > I guess its time to finally push out 1.4 so that the code can move > > again ... > > I'd be glad if you could consider my per-socket authorization patch for > the 1.4 release. I think it's a big improvement over the ``restricted > sockets'' (the `-L' command line option). I'll send an updates patch > with docs in a minute.. to me this patch looks good ... as you know the whole inet socket interface without authentication is a thorn in my side. your patch makes this a bit better ... I'll wait to hear what kevin has to say on the patch. cheers tobi > > Regards, > -octo > -- Tobi Oetiker, OETIKER+PARTNER AG, Aarweg 15 CH-4600 Olten, Switzerland http://it.oetiker.ch [hidden email] ++41 62 775 9902 / sb: -9900 _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
Sebastian Harl
|
In reply to this post
by Florian Forster
Hi,
On Mon, Aug 10, 2009 at 09:51:26PM +0200, Florian Forster wrote: > On Sat, Aug 08, 2009 at 12:22:26PM +0200, Yann Jouanin wrote: > > The translation between relative path and absolute path is done in > > client code (rrdc_flush) while it should only be done by the server. > > No. Yes. Well, this depends on whether you regard RRDCacheD as a local > acceleration cache or a remote, networked daemon. […] Thanks for being a bit more verbose about that than I was. I fully agree with this differentiation. > On Mon, Aug 10, 2009 at 10:10:38AM +0200, Sebastian Harl wrote: > > I think that by following the principle of least surprise RRDcacheD > > should behave like that as well. Let people think about $HOST:1.rrd > > instead of 1.rrd (where $HOST might be remote or local) - we're > > referring to a file on some $HOST after all … > > If the daemon should work as a real network daemon, then this style of > addressing is the way to go in my opinion. Imho, if we're going to decide that this is _not_ what we want in the long run, we should seriously think about removing networking support again (which would have to happen now, before releasing 1.4). Imho, having something that exist just because it was easy to implement but is not really supported (as in: having some somewhat "half-baked" solution) by the developers causes more pain than good. Anyway, I don't think that this is what you want, so lets try to make this a real network daemon :-) > In my ideal world, this would look somewhat like this: > > * The ‘rrd_*_r’ functions always work on local files, the ‘rrdc_*’ > always use a daemon connection. Ack! Let's not put too much magic into the core. > * Some magical version of the functions will check some sort of > environment and call the appropriate function. That would imho be fine though as long as a (library) user still has access to non-magical functions as well to be able to _explicitly_ state what's supposed to be going on. > * Files are addressed using some special syntax. Using something like > rrdcache://user@host/relative/path > could work okay. If ‘user’ is omitted, the local user name is used. > The paths are always relative paths so the administrator can be sure > all the files controlled by the daemon are beneath one directory. > File names that do not contain “://” are assumed to be local files. Sounds good to me. > (Personally I like the “user@host:path” syntax of ‘scp’ best, but I > fear that there could be incompatibilities.) Hrm … scp needs that to be able to tell apart absolute path names from relative path names. Since we don't need / want that, I'd actually prefer the solution suggested above. > * The ‘RRDCACHE_HOST’ and ‘RRDCACHE_USER’ environment variables can be > used to avoid the special syntax. If they are set, the daemon will > be used. So, if those variables are defined, a user could simply use some relative path name ("some/path/name.rrd") to access the daemon and that path would be sent to the daemon without any modifications on the client side? Imho, that would be the only sensible approach - handling some special cases differently imho introduces too much magic that would definitely cause a lot more confusion. > * To authenticate, the client code looks for a file called > ~/.rrdcache_privkey which contains a private key file (the public > key is required on the server and should be associated with the user > name). If found, public key authentication is attempted, possibly > via TLS. > > If no such file exists, the code will check for a password in > ~/.rrdcache_password and, if found, will use it for password > authentication. > > If neither file exists and the process has a controlling terminal, > then it will try to read a password from /dev/console. > > If all else fails, try authenticating with an empty password. > > So, maybe the actual problem is that too much magic has been put into > > the way the daemon may be enabled (on the client side). > > I hope the environment variables don't add too much confusion. Well, that has the most potential for causing confusion but, then again, the user should know what she's doing when using such features. > Other than that, I think the above approach is pretty straight forward > and easy to understand. Full ack! Cheers, Sebastian -- Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/ Those who would give up Essential Liberty to purchase a little Temporary Safety, deserve neither Liberty nor Safety. -- Benjamin Franklin _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
|
Sebastian Harl
|
In reply to this post
by oetiker
Hi Tobi,
On Tue, Aug 11, 2009 at 07:48:30AM +0200, Tobias Oetiker wrote: > I guess its time to finally push out 1.4 so that the code can move > again ... As much as I'd hate to further delay the 1.4 release, I think we should come to a conclusion about further directions first. Else, we might end up with some functionality (I'm e.g. thinking about the client side path "translation") that's going to be hard to support in the future. I know how much you hate to break backward-compatibility, so let's try to avoid some trouble trying to keep that up. Cheers, Sebastian -- Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/ Those who would give up Essential Liberty to purchase a little Temporary Safety, deserve neither Liberty nor Safety. -- Benjamin Franklin _______________________________________________ rrd-developers mailing list [hidden email] https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers |
||||||||||||||||
| Free Embeddable Forum Powered by Nabble | Help |