[PATCH 0/6] Add cross-compilation support for android

[PATCH 0/6] Add cross-compilation support for android

[Theodore Ts'o]

As part of my efforts to port xfstests so it can be used in an Android
runtime environment (where running fsstress by hand seems to be the
state-of-the-art in file system QA), I needed to make the following
changes to xfsprogs so it can be built using cross compilation using
Android's bionic C library.  Fortunately given XFS's Irix roots, the
number of glibc'isms that have kept in was relatively small.


Theodore Ts'o (6):
  xfsprogs: define NBBY if not defined by the system header files
  xfsprogs: pull in libgen.h to get prototype for basename()
  xfsprogs: define and use BUILD_CC in configure.ac for cross
    compilation
  xfsprogs: avoid use of si_tid in struct xlog_split_item
  xfsprogs: use "unsigned short" instead of ushort
  xfsprogs: add missing include of <stat.h>

 configure.ac               | 8 ++++++++
 include/builddefs.in       | 1 +
 include/platform_defs.h.in | 5 +++++
 include/xfs_inode_buf.h    | 4 ++--
 include/xfs_log_format.h   | 4 ++--
 include/xfs_log_recover.h  | 2 +-
 libdisk/evms.c             | 1 +
 libxfs/Makefile            | 4 ++--
 libxfs/logitem.c           | 2 +-
 libxfs/xfs_ialloc.c        | 4 ++--
 logprint/log_misc.c        | 8 ++++----
 11 files changed, 29 insertions(+), 14 deletions(-)

-- 
2.3.0

[PATCH 1/6] xfsprogs: define NBBY if not defined by the system header files

[Theodore Ts'o]

Android's bionic libc doesn't define NBBY; this isn't a standard
define, and since all modern/sane platforms have 8 bits per byte, use
this as a default.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 include/platform_defs.h.in | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/platform_defs.h.in b/include/platform_defs.h.in
index ac260bc..3e059af 100644
--- a/include/platform_defs.h.in
+++ b/include/platform_defs.h.in
@@ -178,4 +178,8 @@ typedef unsigned short umode_t;
 #define max(a,b)	(((a)>(b))?(a):(b))
 #endif
 
+#ifndef NBBY
+#define NBBY 8
+#endif
+
 #endif	/* __XFS_PLATFORM_DEFS_H__ */
-- 
2.3.0

[PATCH 2/6] xfsprogs: pull in libgen.h to get prototype for basename()

[Theodore Ts'o]

The function prototype for basename() is in <libgen.h>, per Posix.
Without the the function prototype, the build will throw errors due to
the missing prototype.

On glibc, using libgen.h will force the use of Poxis's basename(),
instead of glibc's basename() with GNU extensions.  However, xfsprogs
doesn't depend on any of the GNU extensions, so this is fine.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 include/platform_defs.h.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/platform_defs.h.in b/include/platform_defs.h.in
index 3e059af..bfab7cc 100644
--- a/include/platform_defs.h.in
+++ b/include/platform_defs.h.in
@@ -35,6 +35,7 @@
 #include <sys/types.h>
 #include <limits.h>
 #include <stdbool.h>
+#include <libgen.h>
 
 #undef HAVE___U32
 #ifdef HAVE___U32
-- 
2.3.0

[PATCH 3/6] xfsprogs: define and use BUILD_CC in configure.ac for cross compilation

[Theodore Ts'o]

In order to support cross-compilation, we need to build gen_crc32table
using the C compiler targetted for the build platform, since it is run
as part of the build process.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 configure.ac         | 8 ++++++++
 include/builddefs.in | 1 +
 libxfs/Makefile      | 4 ++--
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index ae17c68..4cfbd4e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -8,6 +8,14 @@ AC_PREFIX_DEFAULT(/usr)
 
 AC_PROG_LIBTOOL
 
+AC_PROG_CC
+if test $cross_compiling = no; then
+  BUILD_CC="$CC"
+  AC_SUBST(BUILD_CC)
+else
+  AC_CHECK_PROGS(BUILD_CC, gcc cc)
+fi
+
 AC_ARG_ENABLE(shared,
 [ --enable-shared=[yes/no] Enable use of shared libraries [default=yes]],,
 	enable_shared=yes)
diff --git a/include/builddefs.in b/include/builddefs.in
index 7e9f53d..1d2d22e 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -64,6 +64,7 @@ PKG_DOC_DIR	= @datadir@/doc/@pkg_name@
 PKG_LOCALE_DIR	= @datadir@/locale
 
 CC		= @cc@
+BUILD_CC	= @BUILD_CC@
 AWK		= @awk@
 SED		= @sed@
 TAR		= @tar@
diff --git a/libxfs/Makefile b/libxfs/Makefile
index ae15a5d..6323b81 100644
--- a/libxfs/Makefile
+++ b/libxfs/Makefile
@@ -67,7 +67,7 @@ default: crc32selftest ltdepend $(LTLIBRARY)
 
 crc32table.h: gen_crc32table.c
 	@echo "    [CC]     gen_crc32table"
-	$(Q) $(CC) $(CFLAGS) -o gen_crc32table $<
+	$(Q) $(BUILD_CC) $(CFLAGS) -o gen_crc32table $<
 	@echo "    [GENERATE] $@"
 	$(Q) ./gen_crc32table > crc32table.h
 
@@ -78,7 +78,7 @@ crc32table.h: gen_crc32table.c
 # disk.
 crc32selftest: gen_crc32table.c crc32table.h crc32.c
 	@echo "    [TEST]    CRC32"
-	$(Q) $(CC) $(CFLAGS) -D CRC32_SELFTEST=1 crc32.c -o $@
+	$(Q) $(BUILD_CC) $(CFLAGS) -D CRC32_SELFTEST=1 crc32.c -o $@
 	$(Q) ./$@
 
 include $(BUILDRULES)
-- 
2.3.0

[PATCH 4/6] xfsprogs: avoid use of si_tid in struct xlog_split_item

[Theodore Ts'o]

In Android's bionic libc (as well as the Linux kernel's
include/uapi/asm-generic/siginfo.h), si_tid is a #define to provide
backwards compatibility for the timerid in the siginfo structure.
This breaks the compile of logprint/log_misc.c.  Change this to be
si_xtid in order to avoid a namespace collision

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 logprint/log_misc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/logprint/log_misc.c b/logprint/log_misc.c
index c9286c6..c0a5c97 100644
--- a/logprint/log_misc.c
+++ b/logprint/log_misc.c
@@ -76,7 +76,7 @@ char *trans_type[] = {
 typedef struct xlog_split_item {
 	struct xlog_split_item	*si_next;
 	struct xlog_split_item	*si_prev;
-	xlog_tid_t		si_tid;
+	xlog_tid_t		si_xtid;
 	int			si_skip;
 } xlog_split_item_t;
 
@@ -161,7 +161,7 @@ xlog_print_add_to_trans(xlog_tid_t	tid,
     xlog_split_item_t *item;
 
     item	  = (xlog_split_item_t *)calloc(sizeof(xlog_split_item_t), 1);
-    item->si_tid  = tid;
+    item->si_xtid  = tid;
     item->si_skip = skip;
     item->si_next = split_list;
     item->si_prev = NULL;
@@ -183,7 +183,7 @@ xlog_print_find_tid(xlog_tid_t tid, uint was_cont)
 	    return 0;
     }
     while (listp) {
-	if (listp->si_tid == tid)
+	if (listp->si_xtid == tid)
 	    break;
 	listp = listp->si_next;
     }
-- 
2.3.0

[PATCH 5/6] xfsprogs: use "unsigned short" instead of ushort

[Theodore Ts'o]

Android's bionic libc doesn't define ushort.  There isn't a real
benefit (other than perhaps conciseness) to use ushort over "unsigned
short", and it's only used in a handful of files in xfsprogs.  So
change over to using unsigned short everywhere.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 include/xfs_inode_buf.h   | 4 ++--
 include/xfs_log_format.h  | 4 ++--
 include/xfs_log_recover.h | 2 +-
 libxfs/logitem.c          | 2 +-
 libxfs/xfs_ialloc.c       | 4 ++--
 logprint/log_misc.c       | 2 +-
 6 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/xfs_inode_buf.h b/include/xfs_inode_buf.h
index 9308c47..d2ebbe6 100644
--- a/include/xfs_inode_buf.h
+++ b/include/xfs_inode_buf.h
@@ -28,8 +28,8 @@ struct xfs_icdinode;
  */
 struct xfs_imap {
 	xfs_daddr_t	im_blkno;	/* starting BB of inode chunk */
-	ushort		im_len;		/* length in BBs of inode chunk */
-	ushort		im_boffset;	/* inode offset in block in bytes */
+	unsigned short	im_len;		/* length in BBs of inode chunk */
+	unsigned short	im_boffset;	/* inode offset in block in bytes */
 };
 
 int	xfs_imap_to_bp(struct xfs_mount *, struct xfs_trans *,
diff --git a/include/xfs_log_format.h b/include/xfs_log_format.h
index f0969c7..af212fb 100644
--- a/include/xfs_log_format.h
+++ b/include/xfs_log_format.h
@@ -456,8 +456,8 @@ static inline uint xfs_icdinode_size(int version)
 typedef struct xfs_buf_log_format {
 	unsigned short	blf_type;	/* buf log item type indicator */
 	unsigned short	blf_size;	/* size of this item */
-	ushort		blf_flags;	/* misc state */
-	ushort		blf_len;	/* number of blocks in this buf */
+	unsigned short	blf_flags;	/* misc state */
+	unsigned short	blf_len;	/* number of blocks in this buf */
 	__int64_t	blf_blkno;	/* starting blkno of this buf */
 	unsigned int	blf_map_size;	/* used size of data bitmap in words */
 	unsigned int	blf_data_map[XFS_BLF_DATAMAP_SIZE]; /* dirty bitmap */
diff --git a/include/xfs_log_recover.h b/include/xfs_log_recover.h
index 1c55ccb..2fea63d 100644
--- a/include/xfs_log_recover.h
+++ b/include/xfs_log_recover.h
@@ -52,7 +52,7 @@ typedef struct xlog_recover {
 	struct list_head	r_itemq;	/* q for items */
 } xlog_recover_t;
 
-#define ITEM_TYPE(i)	(*(ushort *)(i)->ri_buf[0].i_addr)
+#define ITEM_TYPE(i)	(*(unsigned short *)(i)->ri_buf[0].i_addr)
 
 /*
  * This is the number of entries in the l_buf_cancel_table used during
diff --git a/libxfs/logitem.c b/libxfs/logitem.c
index 73d5a9e..2cc29b6 100644
--- a/libxfs/logitem.c
+++ b/libxfs/logitem.c
@@ -112,7 +112,7 @@ xfs_buf_item_init(
 	bip->bli_buf = bp;
 	bip->bli_format.blf_type = XFS_LI_BUF;
 	bip->bli_format.blf_blkno = (__int64_t)XFS_BUF_ADDR(bp);
-	bip->bli_format.blf_len = (ushort)BTOBB(XFS_BUF_COUNT(bp));
+	bip->bli_format.blf_len = (unsigned short)BTOBB(XFS_BUF_COUNT(bp));
 	XFS_BUF_SET_FSPRIVATE(bp, bip);
 }
 
diff --git a/libxfs/xfs_ialloc.c b/libxfs/xfs_ialloc.c
index b20a9ec..89a57f5 100644
--- a/libxfs/xfs_ialloc.c
+++ b/libxfs/xfs_ialloc.c
@@ -1784,7 +1784,7 @@ xfs_imap(
 
 		imap->im_blkno = XFS_AGB_TO_DADDR(mp, agno, agbno);
 		imap->im_len = XFS_FSB_TO_BB(mp, 1);
-		imap->im_boffset = (ushort)(offset << mp->m_sb.sb_inodelog);
+		imap->im_boffset = (unsigned short)(offset << mp->m_sb.sb_inodelog);
 		return 0;
 	}
 
@@ -1812,7 +1812,7 @@ out_map:
 
 	imap->im_blkno = XFS_AGB_TO_DADDR(mp, agno, cluster_agbno);
 	imap->im_len = XFS_FSB_TO_BB(mp, blks_per_cluster);
-	imap->im_boffset = (ushort)(offset << mp->m_sb.sb_inodelog);
+	imap->im_boffset = (unsigned short)(offset << mp->m_sb.sb_inodelog);
 
 	/*
 	 * If the inode number maps to a block outside the bounds
diff --git a/logprint/log_misc.c b/logprint/log_misc.c
index c0a5c97..e0aee1c 100644
--- a/logprint/log_misc.c
+++ b/logprint/log_misc.c
@@ -252,7 +252,7 @@ xlog_print_trans_buffer(xfs_caddr_t *ptr, int len, int *i, int num_ops)
     xfs_buf_log_format_t lbuf;
     int			 size, blen, map_size, struct_size;
     __be64		 x, y;
-    ushort		 flags;
+    unsigned short	 flags;
 
     /*
      * memmove to ensure 8-byte alignment for the long longs in
-- 
2.3.0

[PATCH 6/6] xfsprogs: add missing include of <stat.h>

[Theodore Ts'o]

In libdisk/evms.c, fields from struct stat are used without including
<stat.h>.  Apparently stat.h is included indirectly in glibc's header
files; not so in Android's bionic libc.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 libdisk/evms.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libdisk/evms.c b/libdisk/evms.c
index 4c22ee1..7d41af6 100644
--- a/libdisk/evms.c
+++ b/libdisk/evms.c
@@ -22,6 +22,7 @@
 #include <fcntl.h>
 #include <sys/ioctl.h>
 #include <sys/sysmacros.h>
+#include <sys/stat.h>
 #include <disk/volume.h>
 #include "evms.h"
 
-- 
2.3.0

Re: [PATCH 1/6] xfsprogs: define NBBY if not defined by the system header files

[Christoph Hellwig]

On Sun, Jul 26, 2015 at 08:20:50AM -0400, Theodore Ts'o wrote:
> Android's bionic libc doesn't define NBBY; this isn't a standard
> define, and since all modern/sane platforms have 8 bits per byte, use
> this as a default.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>

This looks fine, but it'll clash with my patch that changes
platform_defs.h so that it's not run through autoconfs m4 anymore.

Otherwise:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 5/6] xfsprogs: use "unsigned short" instead of ushort

[Christoph Hellwig]

On Sun, Jul 26, 2015 at 08:20:54AM -0400, Theodore Ts'o wrote:
> Android's bionic libc doesn't define ushort.  There isn't a real
> benefit (other than perhaps conciseness) to use ushort over "unsigned
> short", and it's only used in a handful of files in xfsprogs.  So
> change over to using unsigned short everywhere.

For the files shared with the kernel this should go into the kernel
first or at least at the same time.  Can you also send
a kernel patch to remove all uses of ushort under fs/xfs?

Otherwise looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 6/6] xfsprogs: add missing include of <stat.h>

[Christoph Hellwig]

This looks fine, but hopefull this file will be gone for the next
major release anyway.

Re: [PATCH 3/6] xfsprogs: define and use BUILD_CC in configure.ac for cross compilation

[Christoph Hellwig]

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/6] xfsprogs: pull in libgen.h to get prototype for basename()

[Christoph Hellwig]

Looks fine, with the same caveat as the previous patch:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 4/6] xfsprogs: avoid use of si_tid in struct xlog_split_item

[Christoph Hellwig]

On Sun, Jul 26, 2015 at 08:20:53AM -0400, Theodore Ts'o wrote:
> In Android's bionic libc (as well as the Linux kernel's
> include/uapi/asm-generic/siginfo.h), si_tid is a #define to provide
> backwards compatibility for the timerid in the siginfo structure.
> This breaks the compile of logprint/log_misc.c.  Change this to be
> si_xtid in order to avoid a namespace collision

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/6] xfsprogs: define and use BUILD_CC in configure.ac for cross compilation

[Gwendal Grignou]

Theodore Ts'o <tytso <at> mit.edu> writes:


> -	$(Q) $(CC) $(CFLAGS) -o gen_crc32table $<
> +	$(Q) $(BUILD_CC) $(CFLAGS) -o gen_crc32table $<

Try to cross compile to armv7a, but I hit a compilation failure for 
gen_crc32table.c:
gcc.real: error: unrecognized command line option '-mfpu=neon'
gcc.real: error: unrecognized command line option '-mfloat-abi=hard'

The problem is CFLAGS is set up for the cross compiler (armv7a-cros-linux-
gnueabi-gcc) and some options does not work on the native compiler.

(CFLAGS is " -O2 -O2 -pipe -march=armv7-a -mtune=cortex-a15 -mfpu=neon -
mfloat-abi=hard -g -fno-exceptions -fno-unwind-tables -fno-asynchronous-
unwind-tables")

Looking into defining BUILD_CFLAGS.

Gwendal.

xfsprogs: Add BUILD_CFLAGS for cross compilation

[Gwendal Grignou]

When cross compiling, CFLAGS may contains option for the cros-compiler the
host compiler can not handle.
Add BUILD_CFLAGS variable that does not include $OPTIMIZER options.

Signed-off by: Gwendal Grignou <gwendal@chromium.org>

--- a/include/builddefs.in	2016-02-01 09:44:14.636018753 -0800
+++ b/include/builddefs.in	2016-02-01 09:37:29.000000000 -0800
@@ -27,6 +27,7 @@
 LOADERFLAGS = @LDFLAGS@
 LTLDFLAGS = @LDFLAGS@
 CFLAGS = @CFLAGS@
+BUILD_CFLAGS = @BUILD_CFLAGS@
 
 LIBRT = @librt@
 LIBUUID = @libuuid@
@@ -151,7 +151,7 @@
 endif
 
 
-GCFLAGS = $(OPTIMIZER) $(DEBUG) \
+GCFLAGS = $(DEBUG) \
 	  -DVERSION=\"$(PKG_VERSION)\" -DLOCALEDIR=\"$(PKG_LOCALE_DIR)\"  \
 	  -DPACKAGE=\"$(PKG_NAME)\" -I$(TOPDIR)/include -I$(TOPDIR)/libxfs
 
@@ -160,7 +160,8 @@
 endif
 
 # First, Global, Platform, Local CFLAGS
-CFLAGS += $(FCFLAGS) $(GCFLAGS) $(PCFLAGS) $(LCFLAGS)
+BUILD_CFLAGS += $(FCFLAGS) $(GCFLAGS) $(PCFLAGS) $(LCFLAGS)
+CFLAGS += $(FCFLAGS) $(OPTIMIZER) $(GCFLAGS) $(PCFLAGS) $(LCFLAGS)
 
 include $(TOPDIR)/include/buildmacros
 
--- a/libxfs/Makefile	2016-02-01 09:44:14.644018849 -0800
+++ b/libxfs/Makefile	2016-02-01 09:37:55.000000000 -0800
@@ -111,7 +111,7 @@
 
 crc32table.h: gen_crc32table.c
 	@echo "    [CC]     gen_crc32table"
-	$(Q) $(BUILD_CC) $(CFLAGS) -o gen_crc32table $<
+	$(Q) $(BUILD_CC) $(BUILD_CFLAGS) -o gen_crc32table $<
 	@echo "    [GENERATE] $@"
 	$(Q) ./gen_crc32table > crc32table.h
 
@@ -122,7 +122,7 @@
 # disk.
 crc32selftest: gen_crc32table.c crc32table.h crc32.c
 	@echo "    [TEST]    CRC32"
-	$(Q) $(BUILD_CC) $(CFLAGS) -D CRC32_SELFTEST=1 crc32.c -o $@
+	$(Q) $(BUILD_CC) $(BUILD_CFLAGS) -D CRC32_SELFTEST=1 crc32.c -o $@
 	$(Q) ./$@
 
 # set up include/xfs header directory
--- a/configure.ac	2016-02-01 09:56:42.873010767 -0800
+++ b/configure.ac	2016-02-01 09:56:45.885047014 -0800
@@ -11,9 +11,12 @@
 AC_PROG_CC
 if test $cross_compiling = no; then
   BUILD_CC="$CC"
+  BUILD_CFLAGS="$CFLAGS"
   AC_SUBST(BUILD_CC)
 else
   AC_CHECK_PROGS(BUILD_CC, gcc cc)
+  BUILD_CFLAGS="-g"
 fi
+AC_SUBST(BUILD_CFLAGS)
 
 AC_ARG_ENABLE(shared,
 [ --enable-shared=[yes/no] Enable use of shared libraries [default=yes]],,

Re: xfsprogs: Add BUILD_CFLAGS for cross compilation

[Christoph Hellwig]

On Mon, Feb 01, 2016 at 01:25:09PM -0800, Gwendal Grignou wrote:
> When cross compiling, CFLAGS may contains option for the cros-compiler the
> host compiler can not handle.
> Add BUILD_CFLAGS variable that does not include $OPTIMIZER options.

I would have expected this to be HOST_CFLAGS, but it seems the
BUILD_ convention already exists in xfsprogs, so sticking to it
looks fine to me.

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: xfsprogs: Add BUILD_CFLAGS for cross compilation

[Gwendal Grignou]

[-- Attachment #1.1: Type: text/plain, Size: 657 bytes --]

Looking at xfsprogs-4.5.0, that patch has not been applied. Did it not work?
It still applies cleanly on 4.5.

Gwendal.

On Tue, Feb 2, 2016 at 10:08 AM, Christoph Hellwig <hch@infradead.org>
wrote:

> On Mon, Feb 01, 2016 at 01:25:09PM -0800, Gwendal Grignou wrote:
> > When cross compiling, CFLAGS may contains option for the cros-compiler
> the
> > host compiler can not handle.
> > Add BUILD_CFLAGS variable that does not include $OPTIMIZER options.
>
> I would have expected this to be HOST_CFLAGS, but it seems the
> BUILD_ convention already exists in xfsprogs, so sticking to it
> looks fine to me.
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>

[-- Attachment #1.2: Type: text/html, Size: 1084 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: xfsprogs: Add BUILD_CFLAGS for cross compilation

[Christoph Hellwig]

On Fri, May 27, 2016 at 09:34:50AM -0700, Gwendal Grignou wrote:
> Looking at xfsprogs-4.5.0, that patch has not been applied. Did it not work?
> It still applies cleanly on 4.5.

It probably just got missed by Dave between all the patches on the list.
Can you just resend it with my Reviewed-by tag added?

Re: xfsprogs: Add BUILD_CFLAGS for cross compilation

[Gwendal Grignou]

[+Mike]
Sure. Mike suggested improvements (ability to specify the host
compiler), and found I did not properly regenerate ./configure. I will
resubmit a better patch.
Gwendal.

On Tue, May 31, 2016 at 5:51 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Fri, May 27, 2016 at 09:34:50AM -0700, Gwendal Grignou wrote:
>> Looking at xfsprogs-4.5.0, that patch has not been applied. Did it not work?
>> It still applies cleanly on 4.5.
>
> It probably just got missed by Dave between all the patches on the list.
> Can you just resend it with my Reviewed-by tag added?