On Fri, Jul 31, 2015 at 12:04:48PM -0400, Mikulas Patocka wrote:
> On Fri, 31 Jul 2015, Dmitry V. Levin wrote:
> > On Fri, Jul 31, 2015 at 10:49:44AM -0400, Mikulas Patocka wrote:
> > > Hi
> > > 
> > > Here I'm sending a patch that makes it possible to decode device mapper 
> > > ioctls in strace.
> > > 
> > > How to apply the patch:
> > > 
> > > Download strace 4.10 from 
> > > http://downloads.sourceforge.net/project/strace/strace/4.10/strace-4.10.tar.xz
> > 
> > Please rebase onto strace.git HEAD (v4.10-277-gd9fb450 at this moment).
> > Use ./bootstrap to regenerate dev files.
> 
> This is the updated patch for strace.git.

Thanks.

Unfortunately, this one doesn't apply at all, you must be using a quite
outdated strace.git repository.

The up to date strace.git is located at git://git.code.sf.net/p/strace/code.git,
and when it is not available because of sf.net issues (which are quite
often nowadays), one can use mirrors at https://github.com/ldv-alt/strace.git
and git://git.altlinux.org/people/ldv/public/strace.git

Just a few comments after very cursory look at your patch:

> --- /dev/null
> +++ strace/dm.c
> @@ -0,0 +1,335 @@
> +#include "defs.h"
> +
> +#ifdef HAVE_LINUX_DM_IOCTL_H
> +
> +#include <sys/ioctl.h>

Please include <linux/ioctl.h> instead.

> +#include <linux/dm-ioctl.h>
> +
> +static void
> +dm_decode_device(const unsigned int code, const struct dm_ioctl *ioc)
> +{
> +     if (code != DM_REMOVE_ALL &&
> +         code != DM_LIST_DEVICES &&
> +         code != DM_LIST_VERSIONS) {

Please use switch statement instead.

> +             if (ioc->dev)
> +                     tprintf(", dev=makedev(%u, %u)", major(ioc->dev), 
> minor(ioc->dev));
> +             if (ioc->name[0]) {
> +                     tprints(", name=");
> +                     print_quoted_string(ioc->name, DM_NAME_LEN, 
> QUOTE_0_TERMINATED);
> +             }
> +             if (ioc->uuid[0]) {
> +                     tprints(", uuid=");
> +                     print_quoted_string(ioc->uuid, DM_UUID_LEN, 
> QUOTE_0_TERMINATED);
> +             }
> +     }
> +}
> +
> +static void
> +dm_decode_values(struct tcb *tcp, const unsigned int code, const struct 
> dm_ioctl *ioc)
> +{
> +     if (entering(tcp)) {
> +             if (code == DM_TABLE_LOAD)
> +                     tprintf(", target_count=%u", 
> (unsigned)ioc->target_count);
> +             if (code == DM_DEV_RENAME ||
> +                 code == DM_DEV_REMOVE ||
> +                 (code == DM_DEV_SUSPEND && !(ioc->flags & DM_SUSPEND_FLAG)) 
> ||
> +                 code == DM_DEV_WAIT)

Please use switch statement instead.

> +                     tprintf(", event_nr=%u", (unsigned)ioc->event_nr);
> +     } else if (!tcp->u_error) {

Please use !syserror(tcp) instead.

> +             if (code == DM_DEV_CREATE ||
> +                 code == DM_DEV_RENAME ||
> +                 code == DM_DEV_SUSPEND ||
> +                 code == DM_DEV_STATUS ||
> +                 code == DM_DEV_WAIT ||
> +                 code == DM_TABLE_LOAD ||
> +                 code == DM_TABLE_CLEAR ||
> +                 code == DM_TABLE_DEPS ||
> +                 code == DM_TABLE_STATUS ||
> +                 code == DM_TARGET_MSG) {

Please use switch statement instead.

> +                     tprintf(", target_count=%u", 
> (unsigned)ioc->target_count);
> +                     tprintf(", open_count=%u", (unsigned)ioc->open_count);
> +                     tprintf(", event_nr=%u", (unsigned)ioc->event_nr);
> +             }
> +     }
> +}
> +
> +static void
> +dm_decode_flags(const struct dm_ioctl *ioc)
> +{
> +     if (ioc->flags) {
> +             static const struct {
> +                     uint32_t flag;
> +                     const char *string;
> +             } dm_flags[] = {
> +                     { DM_READONLY_FLAG, "DM_READONLY_FLAG" },
> +                     { DM_SUSPEND_FLAG, "DM_SUSPEND_FLAG" },
> +                     { DM_PERSISTENT_DEV_FLAG, "DM_PERSISTENT_DEV_FLAG" },
> +                     { DM_STATUS_TABLE_FLAG, "DM_STATUS_TABLE_FLAG" },
> +                     { DM_ACTIVE_PRESENT_FLAG, "DM_ACTIVE_PRESENT_FLAG" },
> +                     { DM_INACTIVE_PRESENT_FLAG, "DM_INACTIVE_PRESENT_FLAG" 
> },
> +                     { DM_BUFFER_FULL_FLAG, "DM_BUFFER_FULL_FLAG" },
> +                     { DM_SKIP_BDGET_FLAG, "DM_SKIP_BDGET_FLAG" },
> +#ifdef DM_SKIP_LOCKFS_FLAG
> +                     { DM_SKIP_LOCKFS_FLAG, "DM_SKIP_LOCKFS_FLAG" },
> +#endif
> +#ifdef DM_NOFLUSH_FLAG
> +                     { DM_NOFLUSH_FLAG, "DM_NOFLUSH_FLAG" },
> +#endif
> +#ifdef DM_QUERY_INACTIVE_TABLE_FLAG
> +                     { DM_QUERY_INACTIVE_TABLE_FLAG, 
> "DM_QUERY_INACTIVE_TABLE_FLAG" },
> +#endif
> +#ifdef DM_UEVENT_GENERATED_FLAG
> +                     { DM_UEVENT_GENERATED_FLAG, "DM_UEVENT_GENERATED_FLAG" 
> },
> +#endif
> +#ifdef DM_UUID_FLAG
> +                     { DM_UUID_FLAG, "DM_UUID_FLAG" },
> +#endif
> +#ifdef DM_SECURE_DATA_FLAG
> +                     { DM_SECURE_DATA_FLAG, "DM_SECURE_DATA_FLAG" },
> +#endif
> +#ifdef DM_DATA_OUT_FLAG
> +                     { DM_DATA_OUT_FLAG, "DM_DATA_OUT_FLAG" },
> +#endif
> +#ifdef DM_DEFERRED_REMOVE
> +                     { DM_DEFERRED_REMOVE, "DM_DEFERRED_REMOVE" },
> +#endif
> +#ifdef DM_INTERNAL_SUSPEND_FLAG
> +                     { DM_INTERNAL_SUSPEND_FLAG, "DM_INTERNAL_SUSPEND_FLAG" 
> },
> +#endif
> +             };

Please create an xlat file instead.
See e.g. xlat/evdev_*.in files.

> +             uint32_t flags = ioc->flags;
> +             bool first = true;
> +             unsigned i;
> +             tprints(", flags=");
> +             for (i = 0; i < sizeof(dm_flags) / sizeof(*dm_flags); i++) {
> +                     if (flags & dm_flags[i].flag) {
> +                             if (!first)
> +                                     tprints("|");
> +                             else
> +                                     first = false;
> +                             tprints(dm_flags[i].string);
> +                             flags &= ~dm_flags[i].flag;
> +                     }
> +             }
> +             if (flags) {
> +                     if (!first)
> +                             tprints("|");
> +                     else
> +                             first = false;
> +                     tprintf("0x%x", (unsigned)flags);
> +             }

Please use printflags instead.

> +
> +     }
> +}
> +
> +static void
> +dm_decode_dm_target_spec(struct tcb *tcp, const struct dm_ioctl *ioc, const 
> char *extra, size_t extra_size)
> +{
> +     uint32_t i;
> +     size_t offset = ioc->data_start;
> +     for (i = 0; i < ioc->target_count; i++) {
> +             if (offset + sizeof(struct dm_target_spec) >= offset &&
> +                 offset + sizeof(struct dm_target_spec) < extra_size) {
> +                     const struct dm_target_spec *s = (const struct 
> dm_target_spec *)(extra + offset);
> +                     tprintf(", {sector_start=%llu, length=%llu", (unsigned 
> long long)s->sector_start, (unsigned long long)s->length);
> +                     if (!entering(tcp))
> +                             tprintf(", status=%d", (int)s->status);
> +                     tprints(", target_type=");
> +                     print_quoted_string(s->target_type, DM_MAX_TYPE_NAME, 
> QUOTE_0_TERMINATED);
> +                     tprints(", string=");
> +                     print_quoted_string((const char *)(s + 1), extra_size - 
> (offset + sizeof(struct dm_target_spec)), QUOTE_0_TERMINATED);

Please wrap long lines so they won't exceed 80 symbols.


-- 
ldv

Attachment: pgpCOtrNTBclA.pgp
Description: PGP signature

------------------------------------------------------------------------------
_______________________________________________
Strace-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to