[PATCH 3/3] Add btrfs ioctl support.
Dmitry V. Levin
ldv at altlinux.org
Thu Mar 31 02:50:14 UTC 2016
On Wed, Mar 30, 2016 at 09:19:27PM -0400, Jeff Mahoney wrote:
[...]
> +#include "defs.h"
> +#include <linux/fs.h>
> +#include <linux/btrfs.h>
linux/btrfs.h was introduced by linux kernel commit v3.9-rc1~4^2~30^2~53,
while strace supports build with much older kernel headers than v3.9-rc1,
so please add a check. I suppose most of the file heavily relies on
constants defined in linux/btrfs.h; if wrapping all of them with ifdefs
doesn't worth the effort, just provide a stub btrfs_ioctl for the case
of missing linux/btrfs.h and that would be enough.
> +#ifndef HAVE_BTRFS_IOCTL_FEATURE_FLAGS
> +struct btrfs_ioctl_feature_flags {
> + __u64 compat_flags;
> + __u64 compat_ro_flags;
> + __u64 incompat_flags;
> +};
> +#endif
> +
> +#ifndef HAVE_BTRFS_IOCTL_DEFRAG_RANGE_ARGS
> +struct btrfs_ioctl_defrag_range_args {
> + __u64 start;
> + __u64 len;
> + __u64 flags;
> + __u32 extent_thresh;
> + __u32 compress_type;
> + __u32 unused[4];
> +};
> +#endif
Please use stdint types instead of __u32 and __u64.
> +#ifndef BTRFS_LABEL_SIZE
> +#define BTRFS_LABEL_SIZE 256
> +#endif
> +
> +#ifndef BTRFS_FIRST_FREE_OBJECTID
> +#define BTRFS_FIRST_FREE_OBJECTID 256ULL
> +#endif
> +
> +#ifndef BTRFS_IOC_GET_FEATURES
> +#define BTRFS_IOC_GET_FEATURES _IOR(BTRFS_IOCTL_MAGIC, 57, \
> + struct btrfs_ioctl_feature_flags)
> +#define BTRFS_IOC_SET_FEATURES _IOW(BTRFS_IOCTL_MAGIC, 57, \
> + struct btrfs_ioctl_feature_flags[2])
> +#define BTRFS_IOC_GET_SUPPORTED_FEATURES _IOR(BTRFS_IOCTL_MAGIC, 57, \
> + struct btrfs_ioctl_feature_flags[3])
> +#endif
> +
> +#include "xlat/btrfs_balance_flags.h"
> +#include "xlat/btrfs_defrag_flags.h"
> +#include "xlat/btrfs_dev_replace_args.h"
> +#include "xlat/btrfs_dev_replace_cmds.h"
> +#include "xlat/btrfs_dev_replace_results.h"
> +#include "xlat/btrfs_dev_replace_state.h"
> +#include "xlat/btrfs_dev_stats_flags.h"
> +#include "xlat/btrfs_dev_stats_values.h"
> +#include "xlat/btrfs_features_compat.h"
> +#include "xlat/btrfs_features_compat_ro.h"
> +#include "xlat/btrfs_features_incompat.h"
> +#include "xlat/btrfs_key_types.h"
> +#include "xlat/btrfs_qgroup_ctl_cmds.h"
> +#include "xlat/btrfs_qgroup_inherit_flags.h"
> +#include "xlat/btrfs_qgroup_limit_flags.h"
> +#include "xlat/btrfs_qgroup_status_flags.h"
> +#include "xlat/btrfs_scrub_flags.h"
> +#include "xlat/btrfs_snap_flags_v2.h"
> +#include "xlat/btrfs_space_info_flags.h"
> +#include "xlat/btrfs_tree_objectids.h"
> +
> +static inline char
> +prnibble(char v)
> +{
> + if (v >= 10)
> + return 'a' + (v - 10);
> + return '0' + v;
> +}
> +
> +/* Formats uuid, returns 0 if it's all zeroes */
> +static int
> +btrfs_unparse_uuid(unsigned char *uuid, char *out)
> +{
> + int i;
> + int ret = 0;
> + for (i = 0; i < BTRFS_UUID_SIZE; i++) {
> + if (i == 4 || i == 6 || i == 8 || i == 10)
> + *out++ = '-';
> + *out++ = prnibble(uuid[i] >> 4);
> + *out++ = prnibble(uuid[i] & 0xf);
> + if (uuid[i])
> + ret = 1;
> + }
> + out[36] = '\0';
Why 36? Is it by any chance BTRFS_UUID_SIZE * 2 + 4?
> + return ret;
> +}
> +
> +static void
> +print_u64(const char *name, uint64_t value)
> +{
> + tprintf(", %s=%" PRIu64, name, value);
> +}
> +
> +#define print_member_u64(obj, name) print_u64(#name, obj->name)
> +
> +static void
> +btrfs_print_balance_args(const char *name, struct btrfs_balance_args *bba)
> +{
> + tprintf(", %s={profiles=%" PRI__u64, name, bba->profiles);
> + print_member_u64(bba, usage);
> + print_member_u64(bba, devid);
> + print_member_u64(bba, pstart);
> + print_member_u64(bba, pend);
> + print_member_u64(bba, vstart);
> + print_member_u64(bba, vend);
> + print_member_u64(bba, target);
> + printflags(btrfs_balance_flags, bba->flags, NULL);
> + tprints("}");
> +}
> +
> +static void
> +btrfs_print_balance(struct tcb *tcp, const long arg, bool out)
> +{
> + struct btrfs_ioctl_balance_args balance_args;
> +
> + if (umove_or_printaddr(tcp, arg, &balance_args))
> + return;
> +
> + if (syserror(tcp))
> + return;
umove_or_printaddr has already checked for syserror.
> +
> + tprints("{flags=");
> + printflags(btrfs_balance_flags, balance_args.flags, NULL);
> + if (out)
> + tprintf(", state=%" PRI__u64, balance_args.state);
> +
> + if (balance_args.flags & BTRFS_BALANCE_DATA) {
> + tprints(", ");
> + btrfs_print_balance_args("data", &balance_args.data);
> + }
> + if (balance_args.flags & BTRFS_BALANCE_METADATA) {
> + tprints(", ");
> + btrfs_print_balance_args("meta", &balance_args.meta);
> + }
> + if (balance_args.flags & BTRFS_BALANCE_SYSTEM) {
> + tprints(", ");
> + btrfs_print_balance_args("sys", &balance_args.sys);
> + }
> + tprints("}");
> +}
> +
> +static void
> +btrfs_print_features(struct btrfs_ioctl_feature_flags *flags)
> +{
> + tprints("{compat_flags=");
> + printflags(btrfs_features_compat, flags->compat_flags,
> + "BTRFS_FEATURE_COMPAT_???");
> +
> + tprints(", compat_ro_flags=");
> + printflags(btrfs_features_compat_ro, flags->compat_ro_flags,
> + "BTRFS_FEATURE_COMPAT_RO_???");
> +
> + tprints(", incompat_flags=");
> + printflags(btrfs_features_incompat, flags->incompat_flags,
> + "BTRFS_FEATURE_INCOMPAT_???");
> + tprints("}");
> +}
> +
> +static void
> +btrfs_print_qgroup_limit(struct btrfs_qgroup_limit *lim)
> +{
> + tprints("{flags=");
> + printflags(btrfs_qgroup_limit_flags, lim->flags, NULL);
> + tprintf(", max_rfer=%" PRI__u64 ", max_excl=%" PRI__u64
> + ", rsv_rfer=%" PRI__u64 ", rsv_excl=%" PRI__u64 "}",
> + lim->max_rfer, lim->max_excl,
> + lim->rsv_rfer, lim->rsv_excl);
> +}
> +
> +static void
> +btrfs_print_key_type(uint32_t type)
> +{
> + const char *str = xlookup(btrfs_key_types, type);
> + tprintf("%u", type);
> + if (str)
> + tprintf(" /* %s */", str);
> +}
> +
> +static void
> +btrfs_print_objectid(uint64_t objectid)
> +{
> + const char *str = xlookup(btrfs_tree_objectids, objectid);
> + tprintf("%" PRIu64, objectid);
> + if (str)
> + tprintf(" /* %s */", str);
> +}
> +
> +int
> +btrfs_ioctl(struct tcb *tcp, const unsigned int code, const long arg)
> +{
> + int ret = 0;
> +
> + switch (code) {
> + /* Take no arguments; Command only. */
> + case BTRFS_IOC_TRANS_START:
> + case BTRFS_IOC_TRANS_END:
> + case BTRFS_IOC_SYNC:
> + case BTRFS_IOC_SCRUB_CANCEL:
> + case BTRFS_IOC_QUOTA_RESCAN_WAIT:
> + break;
> +
> + /* take a signed int */
> + case BTRFS_IOC_CLONE: /* also FICLONE */
> + case BTRFS_IOC_BALANCE_CTL:
> + printnum_int(tcp, arg, "%d");
A comma is missing.
> + break;
> +
> + /* returns a 64 */
> + case BTRFS_IOC_START_SYNC: /* R */
> + if (entering(tcp))
> + return 0;
> + /* fallthrough */
> + /* take a u64 */
> + case BTRFS_IOC_DEFAULT_SUBVOL: /* W */
> + case BTRFS_IOC_WAIT_SYNC: /* W */
> + tprints(", ");
> + printnum_int64(tcp, arg, "%" PRI__u64);
You can safely use PRIu64 here.
> + break;
> +
> + /* u64 but describe a flags bitfield; We can make that symbolic */
> + case BTRFS_IOC_SUBVOL_GETFLAGS: { /* R */
> + uint64_t flags;
> + if (entering(tcp))
> + return 0;
> + if (syserror(tcp))
> + break;
You has to print something before "break".
> + tprints(", ");
> + if (umove_or_printaddr(tcp, arg, &flags))
> + break;
The check for syserror is not needed here as umove_or_printaddr does
the right thing.
> + printflags(btrfs_snap_flags_v2, flags, "BTRFS_SUBVOL_???");
> + break;
> + }
> + case BTRFS_IOC_SUBVOL_SETFLAGS: { /* W */
> + uint64_t flags;
> + if (exiting(tcp))
> + break;
The check for exiting is redundant because of "break" -> RVAL_DECODED | 1.
> + tprints(", ");
> + if (umove_or_printaddr(tcp, arg, &flags))
> + break;
> + printflags(btrfs_snap_flags_v2, flags, "BTRFS_SUBVOL_???");
> + break;
> + }
> +
> + /* More complex types */
> + case BTRFS_IOC_BALANCE_V2: /* RW */
> + if (entering(tcp)) {
> + tprints(", ");
> + btrfs_print_balance(tcp, arg, false);
> + return 0;
> + }
> + if (exiting(tcp)) {
It's either entering or exiting, so you can use
if (entering(tcp)) {
...
} else {
...
}
> + if (syserror(tcp))
> + break;
> + tprints(" => ");
> + btrfs_print_balance(tcp, arg, true);
> + }
> + break;
> + case BTRFS_IOC_BALANCE_PROGRESS: /* R */
> + if (exiting(tcp)) {
> + tprints(", ");
> + btrfs_print_balance(tcp, arg, true);
> + }
> + break;
It has to be
if (entering(tcp))
return 0;
tprints(", ");
btrfs_print_balance(tcp, arg, true);
break;
> + case BTRFS_IOC_CLONE_RANGE: /* W */
> + if (entering(tcp)) {
> + struct btrfs_ioctl_clone_range_args args;
> +
> + tprints(", ");
> + if (umove_or_printaddr(tcp, arg, &args))
> + break;
> + tprintf("{src_fd=%" PRI__s64 ", "
> + "src_offset=%" PRI__u64 ", "
> + "src_length=%" PRI__u64 ", "
> + "dest_offset=%" PRI__u64 "}",
> + args.src_fd, args.src_offset,
> + args.src_length, args.dest_offset);
> + }
> + break;
As it always does a "break", it's never in an exiting state,
so the check for entering is redundant.
> +
> + case BTRFS_IOC_DEFRAG_RANGE: /* W */
> + if (entering(tcp)) {
> + struct btrfs_ioctl_defrag_range_args args;
> +
> + tprints(", ");
> + if (umove_or_printaddr(tcp, arg, &args))
> + break;
> + tprintf("{start=%" PRI__u64 ", len=%" PRI__u64 ", "
> + "flags=", args.start, args.len);
> + printflags(btrfs_defrag_flags, args.flags, NULL);
> + tprintf(", extent_thresh=%u, compress_type=%u}",
> + args.extent_thresh, args.compress_type);
> + }
> + break;
Likewise.
At this point I'm giving up reviewing btrfs.c,
please correct these issues.
> --- a/configure.ac
> +++ b/configure.ac
> @@ -433,6 +433,21 @@ AC_CHECK_HEADERS([linux/bpf.h], [
> fi
> ])
>
> +AC_CHECK_HEADERS([linux/btrfs.h], [
> + AC_CHECK_MEMBERS([struct btrfs_ioctl_feature_flags.compat_flags],
> + AC_DEFINE(HAVE_BTRFS_IOCTL_FEATURE_FLAGS, [1],
> + [Define 1 to if struct strfs_ioctl_feature_flags exists]),,
> + [#include <linux/btrfs.h>])
> + AC_CHECK_MEMBERS([struct btrfs_ioctl_fs_info_args.nodesize],
> + AC_DEFINE(HAVE_BTRFS_IOCTL_FS_INFO_ARGS_NODESIZE, [1],
> + [Define to 1 if struct btrfs_ioctl_fs_info_args.nodesize exists]),,
> + [#include <linux/btrfs.h>])
> + AC_CHECK_MEMBERS([struct btrfs_ioctl_defrag_range_args.start],
> + AC_DEFINE(HAVE_BTRFS_IOCTL_DEFRAG_RANGE_ARGS, [1],
> + [Define to 1 if struct btrfs_ioctl_defrag_range_args exists]),,
> + [#include <linux/btrfs.h>])
> +])
Do you really need all these AC_DEFINEs? There must be a simpler way, e.g.
AC_CHECK_HEADERS([linux/btrfs.h], [
AC_CHECK_MEMBERS(m4_normalize([
struct btrfs_ioctl_feature_flags.compat_flags,
struct btrfs_ioctl_fs_info_args.nodesize,
struct btrfs_ioctl_defrag_range_args.start
]),,, [#include <linux/btrfs.h>])
])
--
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20160331/b450111d/attachment.bin>
More information about the Strace-devel
mailing list