[lttng-dev] [PATCH lttng-tools] Better error reporting when a snapshot record size is too small
David Goulet
dgoulet at efficios.com
Tue Apr 15 11:16:00 EDT 2014
On 15 Apr (11:14:18), Jérémie Galarneau wrote:
> On Tue, Apr 15, 2014 at 10:44 AM, David Goulet <dgoulet at efficios.com> 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 <jeremie.galarneau at efficios.com>
> >> ---
> >> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 603 bytes
Desc: Digital signature
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20140415/f7ddb364/attachment-0001.sig>
More information about the lttng-dev
mailing list