On 15 Apr (11:14:18), Jérémie Galarneau wrote: > On Tue, Apr 15, 2014 at 10:44 AM, David Goulet <[email protected]> wrote: > > On 15 Apr (10:32:09), Jérémie Galarneau wrote: > >> Print an error message when a snapshot record is made with a > >> max size smaller than the subbuffers. This limitation is now > >> documented in the man page. > > > > Why is this not done on the session daemon side? Like "Oh you provided > > me with a wrong max size" type of error message. > > > > Because here this fixes the issue only on the command line side but a > > user using the API would still fail somehow silently or ... ? > > > > The API call could return something like > -LTTNG_ERR_SNAPSHOT_TOO_SMALL... It sure would be better than what we > have now (a DBG3(...) message on the sessiond side). > > > Not that I don't like the approach here but I just need to know if there > > is a specific reason it's done on the lttng client side only. This patch > > adds two calls that can exchange quite a bit of data with the sessiond > > (list domains and channels) so unless there is no other way, I would go > > for a sessiond side validation instead. > > > > Well, I think clients will end up implementing the same logic > anyway... If the record call fails because the max size is too small, > API users will query the sessiond to know what size would fit. > Ideally, the lttng client will also report the smallest suitable size > in the error message (like it does in my patch). > > I understand your concern about data exchanges delaying the snapshot, > though. What I'd propose is to report the new -LTTNG_ERR_SNAPSHOT_... > code from the API, and have the client report the smallest size > possible size to the user using get_largest_subbuf() when such an > error occurs. What do you think?
Perfect solution for me! David > > Thanks, > Jérémie > > > Cheers! > > David > > > >> > >> Signed-off-by: Jérémie Galarneau <[email protected]> > >> --- > >> doc/man/lttng.1 | 4 ++- > >> src/bin/lttng/commands/snapshot.c | 59 > >> +++++++++++++++++++++++++++++++++++++++ > >> 2 files changed, 62 insertions(+), 1 deletion(-) > >> > >> diff --git a/doc/man/lttng.1 b/doc/man/lttng.1 > >> index 972f71c..bf1b67e 100644 > >> --- a/doc/man/lttng.1 > >> +++ b/doc/man/lttng.1 > >> @@ -787,7 +787,9 @@ List the output of a session. Attributes of the output > >> are printed. > >> Snapshot a session's buffer(s) for all domains. If an URL is specified, > >> it is > >> used instead of a previously added output. Specifying only a name or/and > >> a max > >> size will override the current output values. For instance, you can > >> record a > >> -snapshot with a custom maximum size or with a different name. > >> +snapshot with a custom maximum size or with a different name. However, the > >> +max size must be high enough to contain a complete subbuffer. See the > >> +--subbuf-size switch for default subbuffer sizes. > >> > >> .nf > >> $ lttng snapshot add-output -n mysnapshot file:///data/snapshot > >> diff --git a/src/bin/lttng/commands/snapshot.c > >> b/src/bin/lttng/commands/snapshot.c > >> index c704eee..7fda949 100644 > >> --- a/src/bin/lttng/commands/snapshot.c > >> +++ b/src/bin/lttng/commands/snapshot.c > >> @@ -125,6 +125,56 @@ static int count_arguments(const char **argv) > >> return i; > >> } > >> > >> +static uint64_t get_largest_subbuf() > >> +{ > >> + int domain_count; > >> + int channel_count; > >> + int domain_idx; > >> + int channel_idx; > >> + struct lttng_domain *domains; > >> + uint64_t largest_subbuf = 0; > >> + > >> + domain_count = lttng_list_domains(current_session_name, &domains); > >> + if (domain_count < 0) { > >> + ERR("Unable to list session %s's domains", > >> + current_session_name); > >> + goto end; > >> + } > >> + > >> + for (domain_idx = 0; domain_idx < domain_count; domain_idx++) { > >> + struct lttng_channel *channels; > >> + struct lttng_handle *handle = lttng_create_handle( > >> + current_session_name, &domains[domain_idx]); > >> + > >> + if (!handle) { > >> + ERR("Unable to create handle for session %s", > >> + current_session_name); > >> + goto end; > >> + } > >> + > >> + channel_count = lttng_list_channels(handle, &channels); > >> + if (channel_count < 0) { > >> + ERR("Unable to list channels for session %s", > >> + current_session_name); > >> + goto end; > >> + } > >> + > >> + for (channel_idx = 0; channel_idx < channel_count; > >> + channel_idx++) { > >> + if (channels[channel_idx].attr.subbuf_size > > >> + largest_subbuf) { > >> + largest_subbuf = > >> + > >> channels[channel_idx].attr.subbuf_size; > >> + } > >> + } > >> + free(channels); > >> + lttng_destroy_handle(handle); > >> + } > >> +end: > >> + free(domains); > >> + return largest_subbuf; > >> +} > >> + > >> /* > >> * Create a snapshot output object from arguments using the given URL. > >> * > >> @@ -160,6 +210,15 @@ static struct lttng_snapshot_output > >> *create_output_from_args(const char *url) > >> } > >> > >> if (opt_max_size) { > >> + /* Validate that the max size can contain one subbuffer. */ > >> + uint64_t largest_subbuf = get_largest_subbuf(); > >> + if (largest_subbuf == 0) { > >> + goto error; > >> + } else if (largest_subbuf > opt_max_size) { > >> + ERR("Snapshot size must be greater or equal to the > >> largest subbuffer's size (%zu).", > >> + largest_subbuf); > >> + goto error; > >> + } > >> ret = lttng_snapshot_output_set_size(opt_max_size, output); > >> if (ret < 0) { > >> goto error; > >> -- > >> 1.9.1 > >> > > > > -- > Jérémie Galarneau > EfficiOS Inc. > http://www.efficios.com
signature.asc
Description: Digital signature
_______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
