On Tue, 2022-05-17 at 18:02 -0700, Darrick J. Wong wrote:
> On Tue, May 17, 2022 at 05:12:27PM -0700, Allison Henderson wrote:
> > This patch implements a new set of log printing functions to print
> > the
> > ATTRI and ATTRD items and vectors in the log. These will be used
> > during
> > log dump and log recover operations.
> >
> > RFC: Though most attributes are strings, the attribute operations
> > accept
> > any binary payload, so we should not assume them printable. This
> > was
> > done intentionally in preparation for parent pointers. Until
> > parent
> > pointers get here, attributes have no discernible format. So the
> > print
> > routines are just a simple print or hex dump for now.
> >
> > Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
> > ---
> > logprint/log_misc.c | 48 +++++++++-
> > logprint/log_print_all.c | 12 +++
> > logprint/log_redo.c | 197
> > +++++++++++++++++++++++++++++++++++++++
> > logprint/logprint.h | 12 +++
> > 4 files changed, 268 insertions(+), 1 deletion(-)
> >
> > diff --git a/logprint/log_misc.c b/logprint/log_misc.c
> > index 35e926a3baec..d8c60388375b 100644
> > --- a/logprint/log_misc.c
> > +++ b/logprint/log_misc.c
> > @@ -54,11 +54,46 @@ print_stars(void)
> > "***********************************\n");
> > } /* print_stars */
> >
> > +void
> > +print_hex_dump(char *ptr, int len) {
> > + int i = 0;
> > +
> > + for (i = 0; i < len; i++) {
> > + if (i % 16 == 0)
> > + printf("%08x ", i);
> > +
> > + printf("%02x", ptr[i]);
> > +
> > + if ((i+1)%16 == 0)
> > + printf("\n");
> > + else if ((i+1)%2 == 0)
> > + printf(" ");
> > + }
> > + printf("\n");
> > +}
> > +
> > +bool
> > +is_printable(char *ptr, int len) {
> > + int i = 0;
> > +
> > + for (i = 0; i < len; i++)
> > + if (!isprint(ptr[i]) )
> > + return false;
> > + return true;
> > +}
> > +
> > +void print_or_dump(char *ptr, int len) {
>
> Nits: indentation and whatnot.
Ok, will add line return after the void
>
> > + if (is_printable(ptr, len))
> > + printf("%.*s\n", len, ptr);
> > + else
> > + print_hex_dump(ptr, len);
> > +}
> > +
> > /*
> > * Given a pointer to a data segment, print out the data as if it
> > were
> > * a log operation header.
> > */
> > -static void
> > +void
> > xlog_print_op_header(xlog_op_header_t *op_head,
> > int i,
> > char **ptr)
> > @@ -961,6 +996,17 @@ xlog_print_record(
> > be32_to_cpu(op_head->oh_len));
> > break;
> > }
> > + case XFS_LI_ATTRI: {
> > + skip = xlog_print_trans_attri(&ptr,
> > + be32_to_cpu(op_head->oh_len),
> > + &i);
> > + break;
> > + }
> > + case XFS_LI_ATTRD: {
> > + skip = xlog_print_trans_attrd(&ptr,
> > + be32_to_cpu(op_head->oh_len));
> > + break;
> > + }
> > case XFS_LI_RUI: {
> > skip = xlog_print_trans_rui(&ptr,
> > be32_to_cpu(op_head->oh_len),
> > diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
> > index 182b9d53aaaa..79d37a2d28b7 100644
> > --- a/logprint/log_print_all.c
> > +++ b/logprint/log_print_all.c
> > @@ -404,6 +404,12 @@ xlog_recover_print_logitem(
> > case XFS_LI_EFI:
> > xlog_recover_print_efi(item);
> > break;
> > + case XFS_LI_ATTRD:
> > + xlog_recover_print_attrd(item);
> > + break;
> > + case XFS_LI_ATTRI:
> > + xlog_recover_print_attri(item);
> > + break;
> > case XFS_LI_RUD:
> > xlog_recover_print_rud(item);
> > break;
> > @@ -456,6 +462,12 @@ xlog_recover_print_item(
> > case XFS_LI_EFI:
> > printf("EFI");
> > break;
> > + case XFS_LI_ATTRD:
> > + printf("ATTRD");
> > + break;
> > + case XFS_LI_ATTRI:
> > + printf("ATTRI");
> > + break;
> > case XFS_LI_RUD:
> > printf("RUD");
> > break;
> > diff --git a/logprint/log_redo.c b/logprint/log_redo.c
> > index 297e203d0976..502345d1a842 100644
> > --- a/logprint/log_redo.c
> > +++ b/logprint/log_redo.c
> > @@ -653,3 +653,200 @@ xlog_recover_print_bud(
> > f = item->ri_buf[0].i_addr;
> > xlog_print_trans_bud(&f, sizeof(struct xfs_bud_log_format));
> > }
> > +
> > +/* Attr Items */
> > +
> > +static int
> > +xfs_attri_copy_log_format(
> > + char *buf,
> > + uint len,
> > + struct xfs_attri_log_format *dst_attri_fmt)
> > +{
> > + uint dst_len = sizeof(struct xfs_attri_log_format);
> > +
> > + if (len == dst_len) {
> > + memcpy((char *)dst_attri_fmt, buf, len);
> > + return 0;
> > + }
> > +
> > + fprintf(stderr, _("%s: bad size of attri format: %u; expected
> > %u\n"),
> > + progname, len, dst_len);
> > + return 1;
> > +}
> > +
> > +int
> > +xlog_print_trans_attri(
> > + char **ptr,
> > + uint src_len,
> > + int *i)
> > +{
> > + struct xfs_attri_log_format *src_f = NULL;
> > + xlog_op_header_t *head = NULL;
> > + uint dst_len;
> > + int error = 0;
> > +
> > + dst_len = sizeof(struct xfs_attri_log_format);
> > + if (src_len != dst_len) {
> > + fprintf(stderr, _("%s: bad size of attri format: %u;
> > expected %u\n"),
> > + progname, src_len, dst_len);
> > + return 1;
> > + }
> > +
> > + /*
> > + * memmove to ensure 8-byte alignment for the long longs in
> > + * xfs_attri_log_format_t structure
> > + */
> > + src_f = malloc(src_len);
> > + if (!src_f) {
> > + fprintf(stderr, _("%s: xlog_print_trans_attri: malloc
> > failed\n"),
> > + progname);
> > + exit(1);
> > + }
> > + memmove((char*)src_f, *ptr, src_len);
> > + *ptr += src_len;
> > +
> > + printf(_("ATTRI: #regs: %d name_len: %d, value_len: %d id:
> > 0x%llx\n"),
> > + src_f->alfi_size, src_f->alfi_name_len, src_f-
> > >alfi_value_len,
> > + (unsigned long long)src_f->alfi_id);
> > +
> > + if (src_f->alfi_name_len > 0) {
> > + printf(_("\n"));
> > + (*i)++;
> > + head = (xlog_op_header_t *)*ptr;
> > + xlog_print_op_header(head, *i, ptr);
> > + error = xlog_print_trans_attri_name(ptr,
> > be32_to_cpu(head->oh_len));
> > + if (error)
> > + goto error;
> > + }
> > +
> > + if (src_f->alfi_value_len > 0) {
> > + printf(_("\n"));
> > + (*i)++;
> > + head = (xlog_op_header_t *)*ptr;
> > + xlog_print_op_header(head, *i, ptr);
> > + error = xlog_print_trans_attri_value(ptr,
> > be32_to_cpu(head->oh_len),
> > + src_f->alfi_value_len);
> > + }
> > +error:
> > + free(src_f);
> > +
> > + return error;
> > +} /* xlog_print_trans_attri */
> > +
> > +int
> > +xlog_print_trans_attri_name(
> > + char **ptr,
> > + uint src_len)
> > +{
> > + printf(_("ATTRI: name len:%u\n"), src_len);
> > + print_or_dump(*ptr, src_len);
> > +
> > + *ptr += src_len;
> > +
> > + return 0;
> > +} /* xlog_print_trans_attri */
> > +
> > +int
> > +xlog_print_trans_attri_value(
> > + char **ptr,
> > + uint src_len,
> > + int value_len)
> > +{
> > + int len = value_len;
> > +
> > + if (len > MAX_ATTR_VAL_PRINT)
> > + len = MAX_ATTR_VAL_PRINT;
> > +
> > + printf(_("ATTRI: value len:%u\n"), value_len);
> > + print_or_dump(*ptr, len);
> > +
> > + *ptr += src_len;
> > +
> > + return 0;
> > +} /* xlog_print_trans_attri_value */
> > +
> > +void
> > +xlog_recover_print_attri(
> > + struct xlog_recover_item *item)
> > +{
> > + struct xfs_attri_log_format *f, *src_f = NULL;
> > + uint src_len, dst_len;
> > +
> > + int region = 0;
> > +
> > + src_f = (struct xfs_attri_log_format *)item->ri_buf[0].i_addr;
> > + src_len = item->ri_buf[region].i_len;
> > +
> > + /*
> > + * An xfs_attri_log_format structure contains a attribute name
> > and
> > + * variable length value as the last field.
> > + */
> > + dst_len = sizeof(struct xfs_attri_log_format);
> > +
> > + if ((f = ((struct xfs_attri_log_format *)malloc(dst_len))) ==
> > NULL) {
> > + fprintf(stderr, _("%s: xlog_recover_print_attri: malloc
> > failed\n"),
> > + progname);
> > + exit(1);
> > + }
> > + if (xfs_attri_copy_log_format((char*)src_f, src_len, f))
> > + goto out;
> > +
> > + printf(_("ATTRI: #regs: %d name_len: %d, value_len: %d id:
> > 0x%llx\n"),
> > + f->alfi_size, f->alfi_name_len, f->alfi_value_len,
> > (unsigned long long)f->alfi_id);
> > +
> > + if (f->alfi_name_len > 0) {
> > + region++;
> > + printf(_("ATTRI: name len:%u\n"), f->alfi_name_len);
> > + print_or_dump((char *)item->ri_buf[region].i_addr,
> > + f->alfi_name_len);
> > + }
> > +
> > + if (f->alfi_value_len > 0) {
> > + int len = f->alfi_value_len;
> > +
> > + if (len > MAX_ATTR_VAL_PRINT)
> > + len = MAX_ATTR_VAL_PRINT;
>
> max()?
Ok, will add the max() call
>
> Other than that, everything looks ok to me.
>
> You might want to change the subject of this one with the name of the
> tool it modifies, e.g.
>
> xfs_logprint: Add log item printing for ATTRI and ATTRD
>
Alrighty, will add to subject line. Thanks for the reviews!
Allison
> --D
>
> > +
> > + region++;
> > + printf(_("ATTRI: value len:%u\n"), f->alfi_value_len);
> > + print_or_dump((char *)item->ri_buf[region].i_addr,
> > len);
> > + }
> > +
> > +out:
> > + free(f);
> > +
> > +}
> > +
> > +int
> > +xlog_print_trans_attrd(char **ptr, uint len)
> > +{
> > + struct xfs_attrd_log_format *f;
> > + struct xfs_attrd_log_format lbuf;
> > + uint core_size = sizeof(struct xfs_attrd_log_format);
> > +
> > + memcpy(&lbuf, *ptr, MIN(core_size, len));
> > + f = &lbuf;
> > + *ptr += len;
> > + if (len >= core_size) {
> > + printf(_("ATTRD: #regs: %d id: 0x%llx\n"),
> > + f->alfd_size,
> > + (unsigned long long)f->alfd_alf_id);
> > + return 0;
> > + } else {
> > + printf(_("ATTRD: Not enough data to decode
> > further\n"));
> > + return 1;
> > + }
> > +} /* xlog_print_trans_attrd */
> > +
> > +void
> > +xlog_recover_print_attrd(
> > + struct xlog_recover_item *item)
> > +{
> > + struct xfs_attrd_log_format *f;
> > +
> > + f = (struct xfs_attrd_log_format *)item->ri_buf[0].i_addr;
> > +
> > + printf(_(" ATTRD: #regs: %d id: 0x%llx\n"),
> > + f->alfd_size,
> > + (unsigned long long)f->alfd_alf_id);
> > +}
> > diff --git a/logprint/logprint.h b/logprint/logprint.h
> > index 38a7d3fa80a9..b4479c240d94 100644
> > --- a/logprint/logprint.h
> > +++ b/logprint/logprint.h
> > @@ -29,6 +29,9 @@ extern void xfs_log_print_trans(struct xlog *,
> > int);
> > extern void print_xlog_record_line(void);
> > extern void print_xlog_op_line(void);
> > extern void print_stars(void);
> > +extern void print_hex_dump(char* ptr, int len);
> > +extern bool is_printable(char* ptr, int len);
> > +extern void print_or_dump(char* ptr, int len);
> >
> > extern struct xfs_inode_log_format *
> > xfs_inode_item_format_convert(char *, uint, struct
> > xfs_inode_log_format *);
> > @@ -53,4 +56,13 @@ extern void xlog_recover_print_bui(struct
> > xlog_recover_item *item);
> > extern int xlog_print_trans_bud(char **ptr, uint len);
> > extern void xlog_recover_print_bud(struct xlog_recover_item
> > *item);
> >
> > +#define MAX_ATTR_VAL_PRINT 128
> > +
> > +extern int xlog_print_trans_attri(char **ptr, uint src_len, int
> > *i);
> > +extern int xlog_print_trans_attri_name(char **ptr, uint src_len);
> > +extern int xlog_print_trans_attri_value(char **ptr, uint src_len,
> > int value_len);
> > +extern void xlog_recover_print_attri(struct xlog_recover_item
> > *item);
> > +extern int xlog_print_trans_attrd(char **ptr, uint len);
> > +extern void xlog_recover_print_attrd(struct xlog_recover_item
> > *item);
> > +extern void xlog_print_op_header(xlog_op_header_t *op_head, int i,
> > char **ptr);
> > #endif /* LOGPRINT_H */
> > --
> > 2.25.1
> >
