* [PATCH v3 1/6] overlay: implement fsck utility
2017-12-28 11:40 [PATCH v3 0/6] overlay: implement fsck.overlay utility zhangyi (F)
@ 2017-12-28 11:40 ` zhangyi (F)
2017-12-29 9:20 ` Amir Goldstein
2017-12-28 11:40 ` [PATCH v3 2/6] fsck.overlay: add -n -p and -y options zhangyi (F)
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: zhangyi (F) @ 2017-12-28 11:40 UTC (permalink / raw)
To: linux-unionfs, fstests
Cc: miklos, amir73il, eguan, yi.zhang, miaoxie, yangerkun
fsck.overlay
============
fsck.overlay is used to check and optionally repair underlying
directories of overlay-filesystem.
Check the following points:
Whiteouts
---------
A whiteout is a character device with 0/0 device number. It is used to
record the removed files or directories, When a whiteout is found in a
directory, there should be at least one directory or file with the
same name in any of the corresponding lower layers. If not exist, the
whiteout will be treated as orphan whiteout and remove.
Redirect directories
--------------------
An redirect directory is a directory with "trusted.overlay.redirect"
xattr valued to the path of the original location from the root of
the overlay. It is only used when renaming a directory and "redirect
dir" feature is enabled.
If an redirect directory is found, the following must be met:
1) The directory path pointed by redirect xattr should exist in one of
lower layers.
2) The origin directory should be redirected only once in one layer,
which means there is only one redirect xattr point to this origin
directory in the specified layer.
3) A whiteout or an opaque directory with the same name to origin should
exist in the same directory as the redirect directory.
If not, 1) The redirect xattr is invalid and need to remove 2) One of
the redirect xattr is redundant but not sure which one is, ask user or
warn in auto mode 3) Create a whiteout device or set opaque xattr to an
existing directory if the parent directory was meregd or remove xattr
if not.
Usage
=====
1. Ensure overlay filesystem is not mounted based on directories which
need to
check.
2. Run fsck.overlay program:
Usage:
fsck.overlay [-l lowerdir] [-u upperdir] [-w workdir] [-avhV]
Options:
-l, --lowerdir=LOWERDIR specify lower directories of overlayfs,
multiple lower use ':' as separator.
-u, --upperdir=UPPERDIR specify upper directory of overlayfs
-w, --workdir=WORKDIR specify work directory of overlayfs
-a, --auto repair automatically (no questions)
-v, --verbose print more messages of overlayfs
-h, --help display this usage of overlayfs
-V, --version display version information
3. Exit value:
0 No errors
1 Filesystem errors corrected
2 System should be rebooted
4 Filesystem errors left uncorrected
8 Operational error
16 Usage or syntax error
32 Checking canceled by user request
128 Shared-library error
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
Makefile | 31 ++++
README.md | 79 ++++++++++
check.c | 489 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
check.h | 25 ++++
common.c | 119 +++++++++++++++
common.h | 53 +++++++
config.h | 40 +++++
fsck.c | 201 ++++++++++++++++++++++++++
lib.c | 237 ++++++++++++++++++++++++++++++
lib.h | 101 +++++++++++++
mount.c | 330 ++++++++++++++++++++++++++++++++++++++++++
mount.h | 26 ++++
12 files changed, 1731 insertions(+)
create mode 100644 Makefile
create mode 100644 README.md
create mode 100644 check.c
create mode 100644 check.h
create mode 100644 common.c
create mode 100644 common.h
create mode 100644 config.h
create mode 100644 fsck.c
create mode 100644 lib.c
create mode 100644 lib.h
create mode 100644 mount.c
create mode 100644 mount.h
diff --git a/Makefile b/Makefile
new file mode 100644
index 0000000..ced5005
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,31 @@
+CFLAGS = -Wall -g
+LFLAGS = -lm
+CC = gcc
+
+all: overlay
+
+overlay: fsck.o common.o lib.o check.o mount.o
+ $(CC) $(LFLAGS) fsck.o common.o lib.o check.o mount.o -o fsck.overlay
+
+fsck.o:
+ $(CC) $(CFLAGS) -c fsck.c
+
+common.o:
+ $(CC) $(CFLAGS) -c common.c
+
+lib.o:
+ $(CC) $(CFLAGS) -c lib.c
+
+check.o:
+ $(CC) $(CFLAGS) -c check.c
+
+mount.o:
+ $(CC) $(CFLAGS) -c mount.c
+
+clean:
+ rm -f *.o fsck.overlay
+ rm -rf bin
+
+install: all
+ mkdir bin
+ cp fsck.overlay bin
diff --git a/README.md b/README.md
new file mode 100644
index 0000000..c2acb65
--- /dev/null
+++ b/README.md
@@ -0,0 +1,79 @@
+fsck.overlay
+============
+
+fsck.overlay is used to check and optionally repair underlying directories
+of overlay-filesystem.
+
+Check the following points:
+
+Whiteouts
+---------
+
+A whiteout is a character device with 0/0 device number. It is used to record
+the removed files or directories, When a whiteout is found in a directory,
+there should be at least one directory or file with the same name in any of the
+corresponding lower layers. If not exist, the whiteout will be treated as orphan
+whiteout and remove.
+
+Redirect directories
+--------------------
+
+An redirect directory is a directory with "trusted.overlay.redirect" xattr
+valued to the path of the original location from the root of the overlay. It
+is only used when renaming a directory and "redirect dir" feature is enabled.
+If an redirect directory is found, the following must be met:
+
+1) The directory path pointed by redirect xattr should exist in one of lower
+ layers.
+2) The origin directory should be redirected only once in one layer, which means
+ there is only one redirect xattr point to this origin directory in the
+ specified layer.
+3) A whiteout or an opaque directory with the same name to origin should exist
+ in the same directory as the redirect directory.
+
+If not, 1) The redirect xattr is invalid and need to remove 2) One of the
+redirect xattr is redundant but not sure which one is, ask user or warn in
+auto mode 3) Create a whiteout device or set opaque xattr to an existing
+directory if the parent directory was meregd or remove xattr if not.
+
+Usage
+=====
+
+1. Ensure overlay filesystem is not mounted based on directories which need to
+ check.
+
+2. Run fsck.overlay program:
+ Usage:
+ fsck.overlay [-l lowerdir] [-u upperdir] [-w workdir] [-avhV]
+
+ Options:
+ -l, --lowerdir=LOWERDIR specify lower directories of overlayfs,
+ multiple lower use ':' as separator.
+ -u, --upperdir=UPPERDIR specify upper directory of overlayfs
+ -w, --workdir=WORKDIR specify work directory of overlayfs
+ -a, --auto repair automatically (no questions)
+ -v, --verbose print more messages of overlayfs
+ -h, --help display this usage of overlayfs
+ -V, --version display version information
+
+3. Exit value:
+ 0 No errors
+ 1 Filesystem errors corrected
+ 2 System should be rebooted
+ 4 Filesystem errors left uncorrected
+ 8 Operational error
+ 16 Usage or syntax error
+ 32 Checking canceled by user request
+ 128 Shared-library error
+
+Todo
+====
+
+1. Overlay filesystem mounted check. Prevent fscking when overlay is
+ online. Now, We cannot distinguish mounted directories if overlayfs was
+ mounted with relative path. Should modify kernel together to support.
+2. Check and fix invalid redirect xattr through origin xattr.
+3. Symbolic link check.
+4. Check origin/impure/nlink xattr.
+5. Check index feature consistency.
+6. ...
diff --git a/check.c b/check.c
new file mode 100644
index 0000000..efa180f
--- /dev/null
+++ b/check.c
@@ -0,0 +1,489 @@
+/*
+ * check.c - Check and fix inconsistency for all underlying layers of overlay
+ *
+ * Copyright (c) 2017 Huawei. All Rights Reserved.
+ * Author: zhangyi (F) <yi.zhang@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <stdbool.h>
+#include <sys/types.h>
+#include <sys/xattr.h>
+#include <sys/stat.h>
+#include <linux/limits.h>
+
+#include "common.h"
+#include "lib.h"
+#include "check.h"
+
+/* Underlying information */
+struct ovl_lower_check {
+ unsigned int type; /* check extent type */
+
+ bool exist;
+ char path[PATH_MAX]; /* exist pathname found, only valid if exist */
+ struct stat st; /* only valid if exist */
+};
+
+/* Redirect information */
+struct ovl_redirect_entry {
+ struct ovl_redirect_entry *next;
+
+ char *origin; /* origin directory path */
+
+ char *path; /* redirect directory path */
+ int dirtype; /* redirect directory layer type: OVL_UPPER or OVL_LOWER */
+ int dirnum; /* redirect directory layer num (only valid in OVL_LOWER) */
+};
+
+/* Whiteout */
+#define WHITEOUT_DEV 0
+#define WHITEOUT_MOD 0
+
+extern char **lowerdir;
+extern char upperdir[];
+extern char workdir[];
+extern int lower_num;
+extern int flags;
+extern int status;
+
+static inline mode_t file_type(const struct stat *status)
+{
+ return status->st_mode & S_IFMT;
+}
+
+static inline bool is_whiteout(const struct stat *status)
+{
+ return (file_type(status) == S_IFCHR) && (status->st_rdev == WHITEOUT_DEV);
+}
+
+static inline bool is_dir(const struct stat *status)
+{
+ return file_type(status) == S_IFDIR;
+}
+
+static bool is_dir_xattr(const char *pathname, const char *xattrname)
+{
+ char val;
+ ssize_t ret;
+
+ ret = getxattr(pathname, xattrname, &val, 1);
+ if ((ret < 0) && !(errno == ENODATA || errno == ENOTSUP)) {
+ print_err(_("Cannot getxattr %s %s: %s\n"), pathname,
+ xattrname, strerror(errno));
+ return false;
+ }
+
+ return (ret == 1 && val == 'y') ? true : false;
+}
+
+static inline bool ovl_is_opaque(const char *pathname)
+{
+ return is_dir_xattr(pathname, OVL_OPAQUE_XATTR);
+}
+
+static inline int ovl_remove_opaque(const char *pathname)
+{
+ return remove_xattr(pathname, OVL_OPAQUE_XATTR);
+}
+
+static inline int ovl_set_opaque(const char *pathname)
+{
+ return set_xattr(pathname, OVL_OPAQUE_XATTR, "y", 1);
+}
+
+static int ovl_get_redirect(const char *pathname, size_t dirlen,
+ size_t filelen, char **redirect)
+{
+ char *buf = NULL;
+ ssize_t ret;
+
+ ret = get_xattr(pathname, OVL_REDIRECT_XATTR, &buf, NULL);
+ if (ret <= 0 || !buf)
+ return ret;
+
+ if (buf[0] != '/') {
+ size_t baselen = strlen(pathname)-filelen-dirlen;
+
+ buf = srealloc(buf, ret + baselen + 1);
+ memmove(buf + baselen, buf, ret);
+ memcpy(buf, pathname+dirlen, baselen);
+ ret += baselen;
+ }
+
+ ret -= 1;
+ memmove(buf, buf+1, ret);
+ buf[ret] = '\0';
+
+ *redirect = buf;
+ return 0;
+}
+
+static inline int ovl_remove_redirect(const char *pathname)
+{
+ return remove_xattr(pathname, OVL_REDIRECT_XATTR);
+}
+
+static inline int ovl_create_whiteout(const char *pathname)
+{
+ int ret;
+
+ ret = mknod(pathname, S_IFCHR | WHITEOUT_MOD, makedev(0, 0));
+ if (ret)
+ print_err(_("Cannot mknod %s:%s\n"),
+ pathname, strerror(errno));
+ return ret;
+}
+
+static inline int ovl_ask_invalid(const char *question, const char *pathname,
+ int action)
+{
+ print_info(_("%s: %s "), question, pathname);
+
+ return ask_question("Remove", action);
+}
+
+/*
+ * Scan each lower dir lower than 'start' and check type matching,
+ * we stop scan if we found something.
+ *
+ * skip: skip whiteout.
+ *
+ */
+static int ovl_check_lower(const char *pathname, int start,
+ struct ovl_lower_check *chk, bool skip)
+{
+ struct stat st;
+ int i;
+
+ for (i = start; i < lower_num; i++) {
+ path_pack(chk->path, sizeof(chk->path), lowerdir[i], pathname);
+
+ if (lstat(chk->path, &st) != 0) {
+ if (errno != ENOENT && errno != ENOTDIR) {
+ print_err(_("Cannot stat %s: %s\n"),
+ chk->path, strerror(errno));
+ return -1;
+ }
+ continue;
+ }
+ if (skip && is_whiteout(&st))
+ continue;
+
+ chk->exist = true;
+ chk->st = st;
+ break;
+ }
+ return 0;
+}
+
+/*
+ * Scan each underlying dirs under specified dir if a whiteout is
+ * found, check it's orphan or not. In auto-mode, orphan whiteouts
+ * will be removed directly.
+ */
+static int ovl_check_whiteout(struct scan_ctx *sctx)
+{
+ const char *pathname = sctx->pathname;
+ const struct stat *st = sctx->st;
+ struct ovl_lower_check chk = {0};
+ int start;
+ int ret = 0;
+
+ /* Is a whiteout ? */
+ if (!is_whiteout(st))
+ return 0;
+
+ sctx->t_whiteouts++;
+
+ /* Is Whiteout in the bottom lower dir ? */
+ if (sctx->dirtype == OVL_LOWER && sctx->num == lower_num-1)
+ goto remove;
+
+ /*
+ * Scan each corresponding lower directroy under this layer,
+ * check is there a file or dir with the same name.
+ */
+ start = (sctx->dirtype == OVL_LOWER) ? sctx->num + 1 : 0;
+ ret = ovl_check_lower(path_pick(pathname, sctx->dirlen),
+ start, &chk, true);
+ if (ret)
+ return ret;
+ if (chk.exist && !is_whiteout(&chk.st))
+ goto out;
+
+remove:
+ sctx->i_whiteouts++;
+
+ /* Remove orphan whiteout directly or ask user */
+ if (!ovl_ask_invalid("Orphan whiteout", pathname, 1))
+ return 0;
+
+ ret = unlink(pathname);
+ if (ret) {
+ print_err(_("Cannot unlink %s: %s\n"), pathname,
+ strerror(errno));
+ return ret;
+ }
+ sctx->i_whiteouts--;
+out:
+ return ret;
+}
+
+static struct ovl_redirect_entry *redirect_list = NULL;
+
+static void ovl_redirect_entry_add(const char *path, int dirtype,
+ int dirnum, const char *origin)
+{
+ struct ovl_redirect_entry *last, *new;
+
+ new = smalloc(sizeof(*new));
+
+ print_debug(_("Redirect entry add: [%s %s %s %d]\n"),
+ origin, path, (dirtype == OVL_UPPER) ? "UP" : "LOW",
+ (dirtype == OVL_UPPER) ? 0 : dirnum);
+
+ if (!redirect_list) {
+ redirect_list = new;
+ } else {
+ for (last = redirect_list; last->next; last = last->next);
+ last->next = new;
+ }
+
+ new->dirtype = dirtype;
+ new->dirnum = dirnum;
+ new->origin = sstrdup(origin);
+ new->path = sstrdup(path);
+ new->next = NULL;
+}
+
+static bool ovl_redirect_entry_find(const char *origin, int dirtype,
+ int dirnum, char **founded)
+{
+ struct ovl_redirect_entry *entry;
+
+ if (!redirect_list)
+ return false;
+
+ for (entry = redirect_list; entry; entry = entry->next) {
+ bool same_layer;
+
+ print_debug(_("Redirect entry found:[%s %s %s %d]\n"),
+ entry->origin, entry->path,
+ (entry->dirtype == OVL_UPPER) ? "UP" : "LOW",
+ (entry->dirtype == OVL_UPPER) ? 0 : entry->dirnum);
+
+ same_layer = ((entry->dirtype == dirtype) &&
+ (dirtype == OVL_LOWER ? (entry->dirnum == dirnum) : true));
+
+ if (same_layer && !strcmp(entry->origin, origin)) {
+ *founded = entry->path;
+ return true;
+ }
+ }
+
+ return false;
+}
+
+static void ovl_redirect_free(void)
+{
+ struct ovl_redirect_entry *entry;
+
+ while (redirect_list) {
+ entry = redirect_list;
+ redirect_list = redirect_list->next;
+ free(entry->origin);
+ free(entry->path);
+ free(entry);
+ }
+}
+
+/*
+ * Get redirect origin directory stored in the xattr, check it's invlaid
+ * or not, In auto-mode, invalid redirect xattr will be removed directly.
+ * Do the follow checking:
+ * 1) Check the origin directory exist or not. If not, remove xattr.
+ * 2) Count how many directories the origin directory was redirected by.
+ * If more than one in the same layer, there must be some inconsistency
+ * but not sure which one is invalid, ask user or warn in auto mode.
+ * 3) Check and fix the missing whiteout or opaque in redierct parent dir.
+ */
+static int ovl_check_redirect(struct scan_ctx *sctx)
+{
+ const char *pathname = sctx->pathname;
+ struct ovl_lower_check chk = {0};
+ char redirect_rpath[PATH_MAX];
+ struct stat rst;
+ char *redirect = NULL;
+ int start;
+ int ret = 0;
+
+ /* Get redirect */
+ ret = ovl_get_redirect(pathname, sctx->dirlen,
+ sctx->filelen, &redirect);
+ if (ret || !redirect)
+ return ret;
+
+ print_debug(_("Dir %s has redirect %s\n"), pathname, redirect);
+ sctx->t_redirects++;
+
+ /* Redirect dir in last lower dir ? */
+ if (sctx->dirtype == OVL_LOWER && sctx->num == lower_num-1)
+ goto remove;
+
+ /* Scan lower directories to check redirect dir exist or not */
+ start = (sctx->dirtype == OVL_LOWER) ? sctx->num + 1 : 0;
+ ret = ovl_check_lower(redirect, start, &chk, false);
+ if (ret)
+ goto out;
+ if (chk.exist && is_dir(&chk.st)) {
+ char *tmp;
+
+ /* Check duplicate in same layer */
+ if (ovl_redirect_entry_find(chk.path, sctx->dirtype,
+ sctx->num, &tmp)) {
+ print_info("Duplicate redirect directories found:\n");
+ print_info("origin:%s current:%s last:%s\n",
+ chk.path, pathname, tmp);
+
+ sctx->i_redirects++;
+
+ /* Don't remove in auto mode */
+ if (ovl_ask_invalid("Duplicate redirect xattr", pathname, 0))
+ goto remove_d;
+ }
+
+ ovl_redirect_entry_add(pathname, sctx->dirtype,
+ sctx->num, chk.path);
+
+ /* Check and fix whiteout or opaque dir */
+ path_pack(redirect_rpath, sizeof(redirect_rpath),
+ sctx->dirname, redirect);
+ if (lstat(redirect_rpath, &rst) != 0) {
+ if (errno != ENOENT && errno != ENOTDIR) {
+ print_err(_("Cannot stat %s: %s\n"),
+ redirect_rpath, strerror(errno));
+ goto out;
+ }
+
+ /* Found nothing, create a whiteout */
+ if ((ret = ovl_create_whiteout(redirect_rpath)))
+ goto out;
+
+ sctx->t_whiteouts++;
+
+ } else if (is_dir(&rst) && !ovl_is_opaque(redirect_rpath)) {
+ /* Found a directory but not opaqued, fix opaque xattr */
+ if ((ret = ovl_set_opaque(redirect_rpath)))
+ goto out;
+ }
+
+ goto out;
+ }
+
+remove:
+ sctx->i_redirects++;
+
+ /* Remove redirect xattr or ask user */
+ if (!ovl_ask_invalid("Invalid redirect xattr", pathname, 1))
+ goto out;
+remove_d:
+ ret = ovl_remove_redirect(pathname);
+ if (!ret)
+ sctx->i_redirects--;
+out:
+ free(redirect);
+ return ret;
+}
+
+static struct scan_operations ovl_scan_ops = {
+ .whiteout = ovl_check_whiteout,
+ .redirect = ovl_check_redirect,
+};
+
+static void ovl_scan_report(struct scan_ctx *sctx)
+{
+ if (flags & FL_VERBOSE) {
+ print_info(_("Scan %d directories, %d files, "
+ "%d/%d whiteouts, %d/%d redirect dirs\n"),
+ sctx->directories, sctx->files,
+ sctx->i_whiteouts, sctx->t_whiteouts,
+ sctx->i_redirects, sctx->t_redirects);
+ }
+
+ if (sctx->i_whiteouts)
+ print_info(_("Invalid whiteouts %d left!\n"), sctx->i_whiteouts);
+ else if (sctx->i_redirects)
+ print_info(_("Invalid redirect directories %d left!\n"), sctx->i_redirects);
+ else
+ return;
+
+ set_st_inconsistency(&status);
+}
+
+static void ovl_scan_clean(void)
+{
+ /* Clean redirect entry record */
+ ovl_redirect_free();
+}
+
+/* Scan upperdir and each lowerdirs, check and fix inconsistency */
+int ovl_scan_fix(void)
+{
+ struct scan_ctx sctx = {0};
+ int i;
+ int ret = 0;
+
+ if (flags & FL_VERBOSE)
+ print_info(_("Scan and fix: [whiteouts|redirect dir]\n"));
+
+ /* Scan every lower directories */
+ for (i = lower_num - 1; i >= 0; i--) {
+ if (flags & FL_VERBOSE)
+ print_info(_("Scan lower directory %d: %s\n"), i, lowerdir[i]);
+
+ sctx.dirname = lowerdir[i];
+ sctx.dirlen = strlen(lowerdir[i]);
+ sctx.dirtype = OVL_LOWER;
+ sctx.num = i;
+
+ ret = scan_dir(&sctx, &ovl_scan_ops);
+ if (ret)
+ goto out;
+ }
+
+ /* Scan upper directory */
+ if (flags & FL_UPPER) {
+ if (flags & FL_VERBOSE)
+ print_info(_("Scan upper directory: %s\n"), upperdir);
+
+ sctx.dirname = upperdir;
+ sctx.dirlen = strlen(upperdir);
+ sctx.dirtype = OVL_UPPER;
+
+ ret = scan_dir(&sctx, &ovl_scan_ops);
+ if (ret)
+ goto out;
+ }
+out:
+ ovl_scan_report(&sctx);
+ ovl_scan_clean();
+ return ret;
+}
diff --git a/check.h b/check.h
new file mode 100644
index 0000000..6af8c2d
--- /dev/null
+++ b/check.h
@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) 2017 Huawei. All Rights Reserved.
+ * Author: zhangyi (F) <yi.zhang@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef OVL_WHITECHECK_H
+#define OVL_WHITECHECK_H
+
+/* Scan upperdir and each lowerdirs, check and fix inconsistency */
+int ovl_scan_fix(void);
+
+#endif /* OVL_WHITECHECK_H */
diff --git a/common.c b/common.c
new file mode 100644
index 0000000..5ca7deb
--- /dev/null
+++ b/common.c
@@ -0,0 +1,119 @@
+/*
+ * common.c - Common things for all utilities
+ *
+ * Copyright (c) 2017 Huawei. All Rights Reserved.
+ * Author: zhangyi (F) <yi.zhang@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdarg.h>
+#include <errno.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include "common.h"
+#include "config.h"
+
+extern char *program_name;
+
+/* #define DEBUG 1 */
+#ifdef DEBUG
+void print_debug(char *fmtstr, ...)
+{
+ va_list args;
+
+ va_start(args, fmtstr);
+ fprintf(stdout, "%s:[Debug]: ", program_name);
+ vfprintf(stdout, fmtstr, args);
+ va_end(args);
+}
+#else
+void print_debug (char *fmtstr, ...) {}
+#endif
+
+void print_info(char *fmtstr, ...)
+{
+ va_list args;
+
+ va_start(args, fmtstr);
+ vfprintf(stdout, fmtstr, args);
+ va_end(args);
+}
+
+void print_err(char *fmtstr, ...)
+{
+ va_list args;
+
+ va_start(args, fmtstr);
+ fprintf(stderr, "%s:[Error]: ", program_name);
+ vfprintf(stderr, fmtstr, args);
+ va_end(args);
+}
+
+void *smalloc(size_t size)
+{
+ void *new = malloc(size);
+
+ if (!new) {
+ print_err(_("malloc error:%s\n"), strerror(errno));
+ exit(1);
+ }
+
+ memset(new, 0, size);
+ return new;
+}
+
+void *srealloc(void *addr, size_t size)
+{
+ void *re = realloc(addr, size);
+
+ if (!re) {
+ print_err(_("malloc error:%s\n"), strerror(errno));
+ exit(1);
+ }
+ return re;
+}
+
+char *sstrdup(const char *src)
+{
+ char *dst = strdup(src);
+
+ if (!dst) {
+ print_err(_("strdup error:%s\n"), strerror(errno));
+ exit(1);
+ }
+
+ return dst;
+}
+
+char *sstrndup(const char *src, size_t num)
+{
+ char *dst = strndup(src, num);
+
+ if (!dst) {
+ print_err(_("strndup error:%s\n"), strerror(errno));
+ exit(1);
+ }
+
+ return dst;
+}
+
+void version(void)
+{
+ printf(_("Overlay utilities version %s\n"), PACKAGE_VERSION);
+}
diff --git a/common.h b/common.h
new file mode 100644
index 0000000..7147e05
--- /dev/null
+++ b/common.h
@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2017 Huawei. All Rights Reserved.
+ * Author: zhangyi (F) <yi.zhang@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef OVL_COMMON_H
+#define OVL_COMMON_H
+
+#ifndef __attribute__
+# if !defined __GNUC__ || __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 8) || __STRICT_ANSI__
+# define __attribute__(x)
+# endif
+#endif
+
+#ifdef USE_GETTEXT
+#include <libintl.h>
+#define _(x) gettext((x))
+#else
+#define _(x) (x)
+#endif
+
+/* Print an error message */
+void print_err(char *, ...) __attribute__ ((__format__ (__printf__, 1, 2)));
+
+/* Print an info message */
+void print_info(char *, ...) __attribute__ ((__format__ (__printf__, 1, 2)));
+
+/* Print an debug message */
+void print_debug(char *, ...) __attribute__ ((__format__ (__printf__, 1, 2)));
+
+/* Safety wrapper */
+void *smalloc(size_t size);
+void *srealloc(void *addr, size_t size);
+char *sstrdup(const char *src);
+char *sstrndup(const char *src, size_t num);
+
+/* Print program version */
+void version(void);
+
+#endif /* OVL_COMMON_H */
diff --git a/config.h b/config.h
new file mode 100644
index 0000000..fa7eafa
--- /dev/null
+++ b/config.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2017 Huawei. All Rights Reserved.
+ * Author: zhangyi (F) <yi.zhang@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef OVL_CONFIG_H
+#define OVL_CONFIG_H
+
+/* program version */
+#define PACKAGE_VERSION "v1.0.0"
+
+/* overlay max lower stacks (the same to kernel overlayfs driver) */
+#define OVL_MAX_STACK 500
+
+/* File with mounted filesystems */
+#define MOUNT_TAB "/proc/mounts"
+
+/* Name of overlay filesystem type */
+#define OVERLAY_NAME "overlay"
+#define OVERLAY_NAME_OLD "overlayfs"
+
+/* Mount options */
+#define OPT_LOWERDIR "lowerdir="
+#define OPT_UPPERDIR "upperdir="
+#define OPT_WORKDIR "workdir="
+
+#endif
diff --git a/fsck.c b/fsck.c
new file mode 100644
index 0000000..d4e861a
--- /dev/null
+++ b/fsck.c
@@ -0,0 +1,201 @@
+/*
+ * fsck.c - Utility to fsck overlay
+ *
+ * Copyright (c) 2017 Huawei. All Rights Reserved.
+ * Author: zhangyi (F) <yi.zhang@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <getopt.h>
+#include <libgen.h>
+#include <stdbool.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <linux/limits.h>
+
+#include "common.h"
+#include "config.h"
+#include "lib.h"
+#include "check.h"
+#include "mount.h"
+
+char *program_name;
+
+char **lowerdir = NULL;
+char upperdir[PATH_MAX] = {0};
+char workdir[PATH_MAX] = {0};
+int lower_num = 0;
+int flags = 0; /* user input option flags */
+int status = 0; /* fsck scan status */
+
+/* Cleanup lower directories buf */
+static void ovl_clean_lowerdirs(void)
+{
+ int i;
+
+ for (i = 0; i < lower_num; i++) {
+ free(lowerdir[i]);
+ lowerdir[i] = NULL;
+ }
+ free(lowerdir);
+ lowerdir = NULL;
+ lower_num = 0;
+}
+
+static void usage(void)
+{
+ print_info(_("Usage:\n\t%s [-l lowerdir] [-u upperdir] [-w workdir] "
+ "[-avhV]\n\n"), program_name);
+ print_info(_("Options:\n"
+ "-l, --lowerdir=LOWERDIR specify lower directories of overlayfs,\n"
+ " multiple lower use ':' as separator\n"
+ "-u, --upperdir=UPPERDIR specify upper directory of overlayfs\n"
+ "-w, --workdir=WORKDIR specify work directory of overlayfs\n"
+ "-a, --auto repair automatically (no questions)\n"
+ "-v, --verbose print more messages of overlayfs\n"
+ "-h, --help display this usage of overlayfs\n"
+ "-V, --version display version information\n"));
+ exit(1);
+}
+
+static void parse_options(int argc, char *argv[])
+{
+ char *lowertemp;
+ int c;
+ int ret = 0;
+ bool show_usage = false;
+
+ struct option long_options[] = {
+ {"lowerdir", required_argument, NULL, 'l'},
+ {"upperdir", required_argument, NULL, 'u'},
+ {"workdir", required_argument, NULL, 'w'},
+ {"auto", no_argument, NULL, 'a'},
+ {"verbose", no_argument, NULL, 'v'},
+ {"version", no_argument, NULL, 'V'},
+ {"help", no_argument, NULL, 'h'},
+ {NULL, 0, NULL, 0}
+ };
+
+ while ((c = getopt_long(argc, argv, "l:u:w:avVh", long_options, NULL)) != -1) {
+ switch (c) {
+ case 'l':
+ lowertemp = strdup(optarg);
+ ret = ovl_resolve_lowerdirs(lowertemp, &lowerdir, &lower_num);
+ free(lowertemp);
+ break;
+ case 'u':
+ if (realpath(optarg, upperdir)) {
+ print_debug(_("Upperdir:%s\n"), upperdir);
+ flags |= FL_UPPER;
+ } else {
+ print_err(_("Failed to resolve upperdir:%s\n"), optarg);
+ ret = -1;
+ }
+ break;
+ case 'w':
+ if (realpath(optarg, workdir)) {
+ print_debug(_("Workdir:%s\n"), workdir);
+ flags |= FL_WORK;
+ } else {
+ print_err(_("Failed to resolve workdir:%s\n"), optarg);
+ ret = -1;
+ }
+ break;
+ case 'a':
+ flags |= FL_AUTO;
+ break;
+ case 'v':
+ flags |= FL_VERBOSE;
+ break;
+ case 'V':
+ version();
+ return;
+ case 'h':
+ default:
+ usage();
+ return;
+ }
+
+ if (ret)
+ goto err_out;
+ }
+
+ if (!lower_num || (!(flags & FL_UPPER) && lower_num == 1)) {
+ print_info(_("Please specify correct lowerdirs and upperdir!\n\n"));
+ show_usage = true;
+ goto err_out;
+ }
+
+ return;
+
+err_out:
+ if (show_usage)
+ usage();
+ exit(1);
+}
+
+void fsck_status_check(int *val)
+{
+ if (status & OVL_ST_INCONSISTNECY) {
+ *val |= FSCK_UNCORRECTED;
+ print_info(_("Still have unexpected inconsistency!\n"));
+ }
+
+ if (status & OVL_ST_ABORT) {
+ *val |= FSCK_ERROR;
+ print_info(_("Cannot continue, aborting\n"));
+ }
+}
+
+int main(int argc, char *argv[])
+{
+ bool mounted = false;
+ int err = 0;
+ int exit_value = 0;
+
+ program_name = basename(argv[0]);
+
+ parse_options(argc, argv);
+
+ /* Ensure overlay filesystem not mounted */
+ if ((err = ovl_check_mount(&mounted)))
+ goto out;
+ if (mounted) {
+ set_st_abort(&status);
+ goto out;
+ }
+
+ /* Scan and fix */
+ if ((err = ovl_scan_fix()))
+ goto out;
+
+out:
+ fsck_status_check(&exit_value);
+ ovl_clean_lowerdirs();
+
+ if (err)
+ exit_value |= FSCK_ERROR;
+ if (exit_value)
+ print_info("WARNING: Filesystem check failed, may not clean\n");
+ else
+ print_info("Filesystem clean\n");
+
+ return exit_value;
+}
diff --git a/lib.c b/lib.c
new file mode 100644
index 0000000..271cafd
--- /dev/null
+++ b/lib.c
@@ -0,0 +1,237 @@
+/*
+ * lib.c - Common things for all utilities
+ *
+ * Copyright (c) 2017 Huawei. All Rights Reserved.
+ * Author: zhangyi (F) <yi.zhang@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <errno.h>
+#include <string.h>
+#include <stdbool.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/xattr.h>
+#include <fts.h>
+
+#include "common.h"
+#include "lib.h"
+
+extern int flags;
+extern int status;
+
+static int ask_yn(const char *question, int def)
+{
+ char ans[16];
+
+ print_info(_("%s ? [%s]: \n"), question, def ? _("y") : _("n"));
+ fflush(stdout);
+ while (fgets(ans, sizeof(ans)-1, stdin)) {
+ if (ans[0] == '\n')
+ return def;
+ else if (!strcasecmp(ans, "y\n") || !strcasecmp(ans, "yes\n"))
+ return 1;
+ else if (!strcasecmp(ans, "n\n") || !strcasecmp(ans, "no\n"))
+ return 0;
+ else
+ print_info(_("Illegal answer. Please input y/n or yes/no:"));
+ fflush(stdout);
+ }
+ return def;
+}
+
+/* Ask user */
+int ask_question(const char *question, int def)
+{
+ if (flags & FL_AUTO) {
+ print_info(_("%s? %s\n"), question, def ? _("y") : _("n"));
+ return def;
+ }
+
+ return ask_yn(question, def);
+}
+
+ssize_t get_xattr(const char *pathname, const char *xattrname,
+ char **value, bool *exist)
+{
+ char *buf = NULL;
+ ssize_t ret;
+
+ ret = getxattr(pathname, xattrname, NULL, 0);
+ if (ret < 0) {
+ if (errno == ENODATA || errno == ENOTSUP) {
+ if (exist)
+ *exist = false;
+ return 0;
+ } else {
+ goto fail;
+ }
+ }
+
+ /* Zero size value means xattr exist but value unknown */
+ if (exist)
+ *exist = true;
+ if (ret == 0 || !value)
+ return 0;
+
+ buf = smalloc(ret+1);
+ ret = getxattr(pathname, xattrname, buf, ret);
+ if (ret <= 0)
+ goto fail2;
+
+ buf[ret] = '\0';
+ *value = buf;
+ return ret;
+
+fail2:
+ free(buf);
+fail:
+ print_err(_("Cannot getxattr %s %s: %s\n"), pathname,
+ xattrname, strerror(errno));
+ return -1;
+}
+
+int set_xattr(const char *pathname, const char *xattrname,
+ void *value, size_t size)
+{
+ int ret;
+
+ ret = setxattr(pathname, xattrname, value, size, XATTR_CREATE);
+ if (ret && errno != EEXIST)
+ goto fail;
+
+ if (errno == EEXIST) {
+ ret = setxattr(pathname, xattrname, value, size, XATTR_REPLACE);
+ if (ret)
+ goto fail;
+ }
+
+ return 0;
+fail:
+ print_err(_("Cannot setxattr %s %s: %s\n"), pathname,
+ xattrname, strerror(errno));
+ return ret;
+}
+
+int remove_xattr(const char *pathname, const char *xattrname)
+{
+ int ret;
+ if ((ret = removexattr(pathname, xattrname)))
+ print_err(_("Cannot removexattr %s %s: %s\n"), pathname,
+ xattrname, strerror(errno));
+ return ret;
+}
+
+char *path_pack(char *out, int len, const char *base, const char *append)
+{
+ int baselen = strlen(base);
+
+ if (!out) {
+ len = strlen(base) + strlen(append) + 2;
+ out = smalloc(len);
+ }
+
+ if (base[baselen-1] == '/' && append[0] == '/')
+ snprintf(out, len, "%s%s", base, append+1);
+ else if (base[baselen-1] != '/' && append[0] != '/')
+ snprintf(out, len, "%s/%s", base, append);
+ else
+ snprintf(out, len, "%s%s", base, append);
+
+ return out;
+}
+
+const char *path_pick(const char *path, int baselen)
+{
+ const char *p = path + baselen;
+
+ return (*p == '/') ? p+1 : p;
+}
+
+static inline int __check_entry(struct scan_ctx *sctx,
+ int (*do_check)(struct scan_ctx *))
+{
+ return do_check ? do_check(sctx) : 0;
+}
+
+/* Scan specified directories and invoke callback */
+int scan_dir(struct scan_ctx *sctx, struct scan_operations *sop)
+{
+ char *paths[2] = {(char *)sctx->dirname, NULL};
+ FTS *ftsp;
+ FTSENT *ftsent;
+ int ret = 0;
+
+ ftsp = fts_open(paths, FTS_NOCHDIR | FTS_PHYSICAL, NULL);
+ if (ftsp == NULL) {
+ print_err(_("Failed to fts open %s:%s\n"),
+ sctx->dirname, strerror(errno));
+ return -1;
+ }
+
+ while ((ftsent = fts_read(ftsp)) != NULL) {
+ int err;
+
+ print_debug(_("Scan:%-3s %2d %7jd %-40s %s\n"),
+ (ftsent->fts_info == FTS_D) ? "d" :
+ (ftsent->fts_info == FTS_DNR) ? "dnr" :
+ (ftsent->fts_info == FTS_DP) ? "dp" :
+ (ftsent->fts_info == FTS_F) ? "f" :
+ (ftsent->fts_info == FTS_NS) ? "ns" :
+ (ftsent->fts_info == FTS_SL) ? "sl" :
+ (ftsent->fts_info == FTS_SLNONE) ? "sln" :
+ (ftsent->fts_info == FTS_DEFAULT) ? "df" : "???",
+ ftsent->fts_level, ftsent->fts_statp->st_size,
+ ftsent->fts_path, ftsent->fts_name);
+
+ /* Fillup base context */
+ sctx->pathname = ftsent->fts_path;
+ sctx->pathlen = ftsent->fts_pathlen;
+ sctx->filename = ftsent->fts_name;
+ sctx->filelen = ftsent->fts_namelen;
+ sctx->st = ftsent->fts_statp;
+
+ switch (ftsent->fts_info) {
+ case FTS_F:
+ sctx->files++;
+ break;
+ case FTS_DEFAULT:
+ /* Check whiteouts */
+ err = __check_entry(sctx, sop->whiteout);
+ ret = (err) ? err : ret;
+ break;
+ case FTS_D:
+ /* Check redirect xattr */
+ sctx->directories++;
+ err = __check_entry(sctx, sop->redirect);
+ ret = (err) ? err : ret;
+ break;
+ case FTS_NS:
+ case FTS_DNR:
+ case FTS_ERR:
+ print_err(_("Failed to fts read %s:%s\n"),
+ ftsent->fts_path, strerror(ftsent->fts_errno));
+ ret = -1;
+ goto out;
+ }
+ }
+out:
+ fts_close(ftsp);
+ return ret;
+}
diff --git a/lib.h b/lib.h
new file mode 100644
index 0000000..235719c
--- /dev/null
+++ b/lib.h
@@ -0,0 +1,101 @@
+/*
+ * Copyright (c) 2017 Huawei. All Rights Reserved.
+ * Author: zhangyi (F) <yi.zhang@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef OVL_LIB_H
+#define OVL_LIB_H
+
+/* Common return value */
+#define FSCK_OK 0 /* No errors */
+#define FSCK_NONDESTRUCT 1 /* File system errors corrected */
+#define FSCK_REBOOT 2 /* System should be rebooted */
+#define FSCK_UNCORRECTED 4 /* File system errors left uncorrected */
+#define FSCK_ERROR 8 /* Operational error */
+#define FSCK_USAGE 16 /* Usage or syntax error */
+#define FSCK_CANCELED 32 /* Aborted with a signal or ^C */
+#define FSCK_LIBRARY 128 /* Shared library error */
+
+/* Fsck status */
+#define OVL_ST_INCONSISTNECY (1 << 0)
+#define OVL_ST_ABORT (1 << 1)
+
+/* Option flags */
+#define FL_VERBOSE (1 << 0) /* verbose */
+#define FL_UPPER (1 << 1) /* specify upper directory */
+#define FL_WORK (1 << 2) /* specify work directory */
+#define FL_AUTO (1 << 3) /* automactically scan dirs and repair */
+
+/* Scan path type */
+#define OVL_UPPER 0
+#define OVL_LOWER 1
+#define OVL_WORKER 2
+#define OVL_PTYPE_MAX 3
+
+/* Xattr */
+#define OVL_OPAQUE_XATTR "trusted.overlay.opaque"
+#define OVL_REDIRECT_XATTR "trusted.overlay.redirect"
+
+
+/* Directories scan data struct */
+struct scan_ctx {
+ const char *dirname; /* upper/lower root dir */
+ size_t dirlen; /* strlen(dirlen) */
+ int dirtype; /* OVL_UPPER or OVL_LOWER */
+ int num; /* lower dir depth, lower type use only */
+
+ const char *pathname; /* file path from the root */
+ size_t pathlen; /* strlen(pathname) */
+ const char *filename; /* filename */
+ size_t filelen; /* strlen(filename) */
+ struct stat *st; /* file stat */
+
+ int files;
+ int directories;
+ int t_whiteouts; /* total whiteouts */
+ int i_whiteouts; /* invalid whiteouts */
+ int t_redirects; /* total redirect dirs */
+ int i_redirects; /* invalid redirect dirs */
+};
+
+/* Directories scan callback operations struct */
+struct scan_operations {
+ int (*whiteout)(struct scan_ctx *);
+ int (*redirect)(struct scan_ctx *);
+};
+
+static inline void set_st_inconsistency(int *st)
+{
+ *st |= OVL_ST_INCONSISTNECY;
+}
+
+static inline void set_st_abort(int *st)
+{
+ *st |= OVL_ST_ABORT;
+}
+
+int scan_dir(struct scan_ctx *sctx, struct scan_operations *sop);
+int ask_question(const char *question, int def);
+ssize_t get_xattr(const char *pathname, const char *xattrname,
+ char **value, bool *exist);
+int set_xattr(const char *pathname, const char *xattrname,
+ void *value, size_t size);
+int remove_xattr(const char *pathname, const char *xattrname);
+char *path_pack(char *out, int len, const char *base, const char *append);
+char *path_truncate(char *out, int len, const char *path, const char *filename);
+const char *path_pick(const char *path, int baselen);
+
+#endif /* OVL_LIB_H */
diff --git a/mount.c b/mount.c
new file mode 100644
index 0000000..fba51e9
--- /dev/null
+++ b/mount.c
@@ -0,0 +1,330 @@
+/*
+ * mount.c - Check mounted overlay
+ *
+ * Copyright (c) 2017 Huawei. All Rights Reserved.
+ * Author: zhangyi (F) <yi.zhang@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <getopt.h>
+#include <libgen.h>
+#include <stdbool.h>
+#include <mntent.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <linux/limits.h>
+
+#include "common.h"
+#include "config.h"
+#include "lib.h"
+#include "check.h"
+#include "mount.h"
+
+struct ovl_mnt_entry {
+ char *lowers;
+ char **lowerdir;
+ int lowernum;
+ char upperdir[PATH_MAX];
+ char workdir[PATH_MAX];
+};
+
+/* Mount buf allocate a time */
+#define ALLOC_NUM 16
+
+extern char **lowerdir;
+extern char upperdir[];
+extern char workdir[];
+extern int lower_num;
+
+/*
+ * Split directories to individual one.
+ * (copied from linux kernel, see fs/overlayfs/super.c)
+ */
+static unsigned int ovl_split_lowerdirs(char *lower)
+{
+ unsigned int ctr = 1;
+ char *s, *d;
+
+ for (s = d = lower;; s++, d++) {
+ if (*s == '\\') {
+ s++;
+ } else if (*s == ':') {
+ *d = '\0';
+ ctr++;
+ continue;
+ }
+ *d = *s;
+ if (!*s)
+ break;
+ }
+ return ctr;
+}
+
+/* Resolve each lower directories and check the validity */
+int ovl_resolve_lowerdirs(char *loweropt, char ***lowerdir,
+ int *lowernum)
+{
+ int num;
+ char **dirs;
+ char *p;
+ int i;
+
+ num = ovl_split_lowerdirs(loweropt);
+ if (num > OVL_MAX_STACK) {
+ print_err(_("Too many lower directories:%u, max:%u\n"),
+ num, OVL_MAX_STACK);
+ return -1;
+ }
+
+ dirs = smalloc(sizeof(char *) * num);
+
+ p = loweropt;
+ for (i = 0; i < num; i++) {
+ dirs[i] = smalloc(PATH_MAX);
+ if (!realpath(p, dirs[i])) {
+ print_err(_("Failed to resolve lowerdir:%s:%s\n"),
+ p, strerror(errno));
+ goto err;
+ }
+ print_debug(_("Lowerdir %u:%s\n"), i, dirs[i]);
+ p = strchr(p, '\0') + 1;
+ }
+
+ *lowerdir = dirs;
+ *lowernum = num;
+
+ return 0;
+err:
+ for (i--; i >= 0; i--)
+ free(dirs[i]);
+ free(dirs);
+ *lowernum = 0;
+ return -1;
+}
+
+/*
+ * Split and return next opt.
+ * (copied from linux kernel, see fs/overlayfs/super.c)
+ */
+static char *ovl_next_opt(char **s)
+{
+ char *sbegin = *s;
+ char *p;
+
+ if (sbegin == NULL)
+ return NULL;
+
+ for (p = sbegin; *p; p++) {
+ if (*p == '\\') {
+ p++;
+ if (!*p)
+ break;
+ } else if (*p == ',') {
+ *p = '\0';
+ *s = p + 1;
+ return sbegin;
+ }
+ }
+ *s = NULL;
+ return sbegin;
+}
+
+/*
+ * Split and parse opt to each directories.
+ *
+ * Note: FIXME: We cannot distinguish mounted directories if overlayfs was
+ * mounted use relative path, so there may have misjudgment.
+ */
+static int ovl_parse_opt(char *opt, struct ovl_mnt_entry *entry)
+{
+ char tmp[PATH_MAX] = {0};
+ char *p;
+ int len;
+ int ret;
+ int i;
+
+ while ((p = ovl_next_opt(&opt)) != NULL) {
+ if (!*p)
+ continue;
+
+ if (!strncmp(p, OPT_UPPERDIR, strlen(OPT_UPPERDIR))) {
+ len = strlen(p) - strlen(OPT_UPPERDIR) + 1;
+ strncpy(tmp, p+strlen(OPT_UPPERDIR), len);
+
+ if (!realpath(tmp, entry->upperdir)) {
+ print_err(_("Faile to resolve path:%s:%s\n"),
+ tmp, strerror(errno));
+ ret = -1;
+ goto errout;
+ }
+ } else if (!strncmp(p, OPT_LOWERDIR, strlen(OPT_LOWERDIR))) {
+ len = strlen(p) - strlen(OPT_LOWERDIR) + 1;
+ entry->lowers = smalloc(len);
+ strncpy(entry->lowers, p+strlen(OPT_LOWERDIR), len);
+
+ if ((ret = ovl_resolve_lowerdirs(entry->lowers,
+ &entry->lowerdir, &entry->lowernum)))
+ goto errout;
+
+ } else if (!strncmp(p, OPT_WORKDIR, strlen(OPT_WORKDIR))) {
+ len = strlen(p) - strlen(OPT_WORKDIR) + 1;
+ strncpy(tmp, p+strlen(OPT_WORKDIR), len);
+
+ if (!realpath(tmp, entry->workdir)) {
+ print_err(_("Faile to resolve path:%s:%s\n"),
+ tmp, strerror(errno));
+ ret = -1;
+ goto errout;
+ }
+ }
+ }
+
+errout:
+ if (entry->lowernum) {
+ for (i = 0; i < entry->lowernum; i++)
+ free(entry->lowerdir[i]);
+ free(entry->lowerdir);
+ entry->lowerdir = NULL;
+ entry->lowernum = 0;
+ }
+ free(entry->lowers);
+ entry->lowers = NULL;
+
+ return ret;
+}
+
+/* Scan current mounted overlayfs and get used underlying directories */
+static int ovl_scan_mount_init(struct ovl_mnt_entry **ovl_mnt_entries,
+ int *ovl_mnt_count)
+{
+ struct ovl_mnt_entry *mnt_entries;
+ struct mntent *mnt;
+ FILE *fp;
+ char *opt;
+ int allocated, num = 0;
+
+ fp = setmntent(MOUNT_TAB, "r");
+ if (!fp) {
+ print_err(_("Fail to setmntent %s:%s\n"),
+ MOUNT_TAB, strerror(errno));
+ return -1;
+ }
+
+ allocated = ALLOC_NUM;
+ mnt_entries = smalloc(sizeof(struct ovl_mnt_entry) * allocated);
+
+ while ((mnt = getmntent(fp))) {
+ if (!strcmp(mnt->mnt_type, OVERLAY_NAME) ||
+ !strcmp(mnt->mnt_type, OVERLAY_NAME_OLD)) {
+
+ opt = sstrdup(mnt->mnt_opts);
+ if (ovl_parse_opt(opt, &mnt_entries[num])) {
+ free(opt);
+ continue;
+ }
+
+ num++;
+ if (num % ALLOC_NUM == 0) {
+ allocated += ALLOC_NUM;
+ mnt_entries = srealloc(mnt_entries,
+ sizeof(struct ovl_mnt_entry) * allocated);
+ }
+
+ free(opt);
+ }
+ }
+
+ *ovl_mnt_entries = mnt_entries;
+ *ovl_mnt_count = num;
+
+ endmntent(fp);
+ return 0;
+}
+
+static void ovl_scan_mount_exit(struct ovl_mnt_entry *ovl_mnt_entries,
+ int ovl_mnt_count)
+{
+ int i,j;
+
+ for (i = 0; i < ovl_mnt_count; i++) {
+ for (j = 0; j < ovl_mnt_entries[i].lowernum; j++)
+ free(ovl_mnt_entries[i].lowerdir[j]);
+ free(ovl_mnt_entries[i].lowerdir);
+ free(ovl_mnt_entries[i].lowers);
+ }
+ free(ovl_mnt_entries);
+}
+
+/*
+ * Scan every mounted filesystem, check the overlay directories want
+ * to check is already mounted. Check and fix an online overlay is not
+ * allowed.
+ *
+ * Note: fsck may modify lower layers, so even match only one directory
+ * is triggered as mounted.
+ */
+int ovl_check_mount(bool *mounted)
+{
+ struct ovl_mnt_entry *ovl_mnt_entries = NULL;
+ int ovl_mnt_entry_count = 0;
+ char *mounted_path = NULL;
+ int i,j,k;
+ int ret;
+
+ ret = ovl_scan_mount_init(&ovl_mnt_entries, &ovl_mnt_entry_count);
+ if (ret)
+ return ret;
+
+ /* Only check hard matching */
+ for (i = 0; i < ovl_mnt_entry_count; i++) {
+ /* Check lower */
+ for (j = 0; j < ovl_mnt_entries[i].lowernum; j++) {
+ for (k = 0; k < lower_num; k++) {
+ if (!strcmp(lowerdir[k],
+ ovl_mnt_entries[i].lowerdir[j])) {
+ mounted_path = lowerdir[k];
+ *mounted = true;
+ goto out;
+ }
+ }
+ }
+
+ /* Check upper */
+ if (!(strcmp(upperdir, ovl_mnt_entries[i].upperdir))) {
+ mounted_path = upperdir;
+ *mounted = true;
+ goto out;
+ }
+
+ /* Check worker */
+ if (workdir[0] != '\0' && !(strcmp(workdir, ovl_mnt_entries[i].workdir))) {
+ mounted_path = workdir;
+ *mounted = true;
+ goto out;
+ }
+ }
+out:
+ ovl_scan_mount_exit(ovl_mnt_entries, ovl_mnt_entry_count);
+
+ if (*mounted)
+ print_info(_("WARNING: Dir %s is mounted\n"), mounted_path);
+
+ return 0;
+}
diff --git a/mount.h b/mount.h
new file mode 100644
index 0000000..7eed0d0
--- /dev/null
+++ b/mount.h
@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2017 Huawei. All Rights Reserved.
+ * Author: zhangyi (F) <yi.zhang@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef OVL_MOUNT_H
+#define OVL_MOUNT_H
+
+int ovl_resolve_lowerdirs(char *loweropt, char ***lowerdir,
+ int *lowernum);
+int ovl_check_mount(bool *mounted);
+
+#endif /* OVL_MOUNT_H */
--
2.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 1/6] overlay: implement fsck utility
2017-12-28 11:40 ` [PATCH v3 1/6] overlay: implement fsck utility zhangyi (F)
@ 2017-12-29 9:20 ` Amir Goldstein
0 siblings, 0 replies; 18+ messages in thread
From: Amir Goldstein @ 2017-12-29 9:20 UTC (permalink / raw)
To: zhangyi (F)
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On Thu, Dec 28, 2017 at 1:40 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> fsck.overlay
> ============
>
> fsck.overlay is used to check and optionally repair underlying
> directories of overlay-filesystem.
>
> Check the following points:
>
> Whiteouts
> ---------
>
> A whiteout is a character device with 0/0 device number. It is used to
> record the removed files or directories, When a whiteout is found in a
> directory, there should be at least one directory or file with the
> same name in any of the corresponding lower layers. If not exist, the
> whiteout will be treated as orphan whiteout and remove.
>
> Redirect directories
> --------------------
>
> An redirect directory is a directory with "trusted.overlay.redirect"
> xattr valued to the path of the original location from the root of
> the overlay. It is only used when renaming a directory and "redirect
> dir" feature is enabled.
> If an redirect directory is found, the following must be met:
>
> 1) The directory path pointed by redirect xattr should exist in one of
> lower layers.
> 2) The origin directory should be redirected only once in one layer,
"in one layer" - how about:
mkdir lower2/origin
make_redirect lower/test1 origin
make_redirect upper/test2 origin
Two redirects to the same lower(2) dir from two different layers
Zhangyi,
This patch is a little big for me to review properly, so you have a few options:
1. break it up to reviewable chunks, starting with minimal skeleton and adding
functionality slowly in chewable chunks
2. wait for other reviewers to review it as is
3. push the code to your repository and announce 0.1-alpha without Reviewed-by
approval and get the approval on the tool from testers and users
I do have one high level comments about the path string parsing/manipulation
code - I think it would be better if you could use an existing library for that.
I cannot recommend any, but I assume that there are options. At the very leasy
you could maybe use the dirname(3)/basename(3) libgen functions.
Even if you do end up implementing all path manipulation helpers, please keep
them well separated and with well defined API, so that they could be split to
a library down the road.
Also, I see that you still use lstat() and not fstatat() relative to
fd of root layer.
Is that because of the file descriptor limit problem? another reason?
I am returning to that point because it seem to me that fsck.overlay shouldn't
be bothered by the symlinks or whatnot that are parents of the layers root.
Overlayfs isn't bothered with those. Layer lookup always starts from layer root.
FWIW, I don't think that requiring more resources than usual (file descriptors)
is too much to ask when running fsck.overlay of many layers - only it need to
be documented.
For comparison, e2fsck takes a LOT of resources when there are lots of
hardlinks, just not file descriptors.
Cheers,
Amir.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 2/6] fsck.overlay: add -n -p and -y options
2017-12-28 11:40 [PATCH v3 0/6] overlay: implement fsck.overlay utility zhangyi (F)
2017-12-28 11:40 ` [PATCH v3 1/6] overlay: implement fsck utility zhangyi (F)
@ 2017-12-28 11:40 ` zhangyi (F)
2017-12-28 13:52 ` Amir Goldstein
2017-12-28 11:40 ` [PATCH v3 3/6] fsck.overlay: encapsulate underlying directories options zhangyi (F)
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: zhangyi (F) @ 2017-12-28 11:40 UTC (permalink / raw)
To: linux-unionfs, fstests
Cc: miklos, amir73il, eguan, yi.zhang, miaoxie, yangerkun
Add -n -p and -y options to make it official as fsck utilities.
-p, automatic repair (no questions)
-n, make no changes to the filesystem
-y, assume "yes" to all questions
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
README.md | 6 ++++--
fsck.c | 36 ++++++++++++++++++++++++++++++------
lib.c | 3 ++-
lib.h | 6 +++++-
4 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/README.md b/README.md
index c2acb65..458a815 100644
--- a/README.md
+++ b/README.md
@@ -44,14 +44,16 @@ Usage
2. Run fsck.overlay program:
Usage:
- fsck.overlay [-l lowerdir] [-u upperdir] [-w workdir] [-avhV]
+ fsck.overlay [-l lowerdir] [-u upperdir] [-w workdir] [-pnyvhV]
Options:
-l, --lowerdir=LOWERDIR specify lower directories of overlayfs,
multiple lower use ':' as separator.
-u, --upperdir=UPPERDIR specify upper directory of overlayfs
-w, --workdir=WORKDIR specify work directory of overlayfs
- -a, --auto repair automatically (no questions)
+ -p, automatic repair (no questions)
+ -n, make no changes to the filesystem
+ -y, assume "yes" to all questions
-v, --verbose print more messages of overlayfs
-h, --help display this usage of overlayfs
-V, --version display version information
diff --git a/fsck.c b/fsck.c
index d4e861a..44211f7 100644
--- a/fsck.c
+++ b/fsck.c
@@ -62,13 +62,15 @@ static void ovl_clean_lowerdirs(void)
static void usage(void)
{
print_info(_("Usage:\n\t%s [-l lowerdir] [-u upperdir] [-w workdir] "
- "[-avhV]\n\n"), program_name);
+ "[-pnyvhV]\n\n"), program_name);
print_info(_("Options:\n"
"-l, --lowerdir=LOWERDIR specify lower directories of overlayfs,\n"
" multiple lower use ':' as separator\n"
"-u, --upperdir=UPPERDIR specify upper directory of overlayfs\n"
"-w, --workdir=WORKDIR specify work directory of overlayfs\n"
- "-a, --auto repair automatically (no questions)\n"
+ "-p, automatic repair (no questions)\n"
+ "-n, make no changes to the filesystem\n"
+ "-y, assume \"yes\" to all questions\n"
"-v, --verbose print more messages of overlayfs\n"
"-h, --help display this usage of overlayfs\n"
"-V, --version display version information\n"));
@@ -81,19 +83,19 @@ static void parse_options(int argc, char *argv[])
int c;
int ret = 0;
bool show_usage = false;
+ bool opt_conflict = false;
struct option long_options[] = {
{"lowerdir", required_argument, NULL, 'l'},
{"upperdir", required_argument, NULL, 'u'},
{"workdir", required_argument, NULL, 'w'},
- {"auto", no_argument, NULL, 'a'},
{"verbose", no_argument, NULL, 'v'},
{"version", no_argument, NULL, 'V'},
{"help", no_argument, NULL, 'h'},
{NULL, 0, NULL, 0}
};
- while ((c = getopt_long(argc, argv, "l:u:w:avVh", long_options, NULL)) != -1) {
+ while ((c = getopt_long(argc, argv, "l:u:w:apnyvVh", long_options, NULL)) != -1) {
switch (c) {
case 'l':
lowertemp = strdup(optarg);
@@ -119,7 +121,23 @@ static void parse_options(int argc, char *argv[])
}
break;
case 'a':
- flags |= FL_AUTO;
+ case 'p':
+ if (flags & (FL_OPT_YES | FL_OPT_NO))
+ opt_conflict = true;
+ else
+ flags |= FL_OPT_AUTO;
+ break;
+ case 'n':
+ if (flags & (FL_OPT_YES | FL_OPT_AUTO))
+ opt_conflict = true;
+ else
+ flags |= FL_OPT_NO;
+ break;
+ case 'y':
+ if (flags & (FL_OPT_NO | FL_OPT_AUTO))
+ opt_conflict = true;
+ else
+ flags |= FL_OPT_YES;
break;
case 'v':
flags |= FL_VERBOSE;
@@ -143,6 +161,12 @@ static void parse_options(int argc, char *argv[])
goto err_out;
}
+ if (opt_conflict) {
+ print_info(_("Only one of the options -p/-a, -n or -y can be specified!\n\n"));
+ show_usage = true;
+ goto err_out;
+ }
+
return;
err_out:
@@ -177,7 +201,7 @@ int main(int argc, char *argv[])
/* Ensure overlay filesystem not mounted */
if ((err = ovl_check_mount(&mounted)))
goto out;
- if (mounted) {
+ if (mounted && !(flags & FL_OPT_NO)) {
set_st_abort(&status);
goto out;
}
diff --git a/lib.c b/lib.c
index 271cafd..497d357 100644
--- a/lib.c
+++ b/lib.c
@@ -59,7 +59,8 @@ static int ask_yn(const char *question, int def)
/* Ask user */
int ask_question(const char *question, int def)
{
- if (flags & FL_AUTO) {
+ if (flags & FL_OPT_MASK) {
+ def = (flags & FL_OPT_YES) ? 1 : (flags & FL_OPT_NO) ? 0 : def;
print_info(_("%s? %s\n"), question, def ? _("y") : _("n"));
return def;
}
diff --git a/lib.h b/lib.h
index 235719c..5d94836 100644
--- a/lib.h
+++ b/lib.h
@@ -37,7 +37,11 @@
#define FL_VERBOSE (1 << 0) /* verbose */
#define FL_UPPER (1 << 1) /* specify upper directory */
#define FL_WORK (1 << 2) /* specify work directory */
-#define FL_AUTO (1 << 3) /* automactically scan dirs and repair */
+#define FL_OPT_AUTO (1 << 3) /* automactically scan dirs and repair */
+#define FL_OPT_NO (1 << 4) /* no changes to the filesystem */
+#define FL_OPT_YES (1 << 5) /* yes to all questions */
+#define FL_OPT_MASK (FL_OPT_AUTO|FL_OPT_NO|FL_OPT_YES)
+
/* Scan path type */
#define OVL_UPPER 0
--
2.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 2/6] fsck.overlay: add -n -p and -y options
2017-12-28 11:40 ` [PATCH v3 2/6] fsck.overlay: add -n -p and -y options zhangyi (F)
@ 2017-12-28 13:52 ` Amir Goldstein
0 siblings, 0 replies; 18+ messages in thread
From: Amir Goldstein @ 2017-12-28 13:52 UTC (permalink / raw)
To: zhangyi (F)
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On Thu, Dec 28, 2017 at 1:40 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Add -n -p and -y options to make it official as fsck utilities.
>
> -p, automatic repair (no questions)
> -n, make no changes to the filesystem
> -y, assume "yes" to all questions
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Looks ok.
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
> README.md | 6 ++++--
> fsck.c | 36 ++++++++++++++++++++++++++++++------
> lib.c | 3 ++-
> lib.h | 6 +++++-
> 4 files changed, 41 insertions(+), 10 deletions(-)
>
> diff --git a/README.md b/README.md
> index c2acb65..458a815 100644
> --- a/README.md
> +++ b/README.md
> @@ -44,14 +44,16 @@ Usage
>
> 2. Run fsck.overlay program:
> Usage:
> - fsck.overlay [-l lowerdir] [-u upperdir] [-w workdir] [-avhV]
> + fsck.overlay [-l lowerdir] [-u upperdir] [-w workdir] [-pnyvhV]
>
> Options:
> -l, --lowerdir=LOWERDIR specify lower directories of overlayfs,
> multiple lower use ':' as separator.
> -u, --upperdir=UPPERDIR specify upper directory of overlayfs
> -w, --workdir=WORKDIR specify work directory of overlayfs
> - -a, --auto repair automatically (no questions)
> + -p, automatic repair (no questions)
> + -n, make no changes to the filesystem
> + -y, assume "yes" to all questions
> -v, --verbose print more messages of overlayfs
> -h, --help display this usage of overlayfs
> -V, --version display version information
> diff --git a/fsck.c b/fsck.c
> index d4e861a..44211f7 100644
> --- a/fsck.c
> +++ b/fsck.c
> @@ -62,13 +62,15 @@ static void ovl_clean_lowerdirs(void)
> static void usage(void)
> {
> print_info(_("Usage:\n\t%s [-l lowerdir] [-u upperdir] [-w workdir] "
> - "[-avhV]\n\n"), program_name);
> + "[-pnyvhV]\n\n"), program_name);
> print_info(_("Options:\n"
> "-l, --lowerdir=LOWERDIR specify lower directories of overlayfs,\n"
> " multiple lower use ':' as separator\n"
> "-u, --upperdir=UPPERDIR specify upper directory of overlayfs\n"
> "-w, --workdir=WORKDIR specify work directory of overlayfs\n"
> - "-a, --auto repair automatically (no questions)\n"
> + "-p, automatic repair (no questions)\n"
> + "-n, make no changes to the filesystem\n"
> + "-y, assume \"yes\" to all questions\n"
> "-v, --verbose print more messages of overlayfs\n"
> "-h, --help display this usage of overlayfs\n"
> "-V, --version display version information\n"));
> @@ -81,19 +83,19 @@ static void parse_options(int argc, char *argv[])
> int c;
> int ret = 0;
> bool show_usage = false;
> + bool opt_conflict = false;
>
> struct option long_options[] = {
> {"lowerdir", required_argument, NULL, 'l'},
> {"upperdir", required_argument, NULL, 'u'},
> {"workdir", required_argument, NULL, 'w'},
> - {"auto", no_argument, NULL, 'a'},
> {"verbose", no_argument, NULL, 'v'},
> {"version", no_argument, NULL, 'V'},
> {"help", no_argument, NULL, 'h'},
> {NULL, 0, NULL, 0}
> };
>
> - while ((c = getopt_long(argc, argv, "l:u:w:avVh", long_options, NULL)) != -1) {
> + while ((c = getopt_long(argc, argv, "l:u:w:apnyvVh", long_options, NULL)) != -1) {
> switch (c) {
> case 'l':
> lowertemp = strdup(optarg);
> @@ -119,7 +121,23 @@ static void parse_options(int argc, char *argv[])
> }
> break;
> case 'a':
> - flags |= FL_AUTO;
> + case 'p':
> + if (flags & (FL_OPT_YES | FL_OPT_NO))
> + opt_conflict = true;
> + else
> + flags |= FL_OPT_AUTO;
> + break;
> + case 'n':
> + if (flags & (FL_OPT_YES | FL_OPT_AUTO))
> + opt_conflict = true;
> + else
> + flags |= FL_OPT_NO;
> + break;
> + case 'y':
> + if (flags & (FL_OPT_NO | FL_OPT_AUTO))
> + opt_conflict = true;
> + else
> + flags |= FL_OPT_YES;
> break;
> case 'v':
> flags |= FL_VERBOSE;
> @@ -143,6 +161,12 @@ static void parse_options(int argc, char *argv[])
> goto err_out;
> }
>
> + if (opt_conflict) {
> + print_info(_("Only one of the options -p/-a, -n or -y can be specified!\n\n"));
> + show_usage = true;
> + goto err_out;
> + }
> +
> return;
>
> err_out:
> @@ -177,7 +201,7 @@ int main(int argc, char *argv[])
> /* Ensure overlay filesystem not mounted */
> if ((err = ovl_check_mount(&mounted)))
> goto out;
> - if (mounted) {
> + if (mounted && !(flags & FL_OPT_NO)) {
> set_st_abort(&status);
> goto out;
> }
> diff --git a/lib.c b/lib.c
> index 271cafd..497d357 100644
> --- a/lib.c
> +++ b/lib.c
> @@ -59,7 +59,8 @@ static int ask_yn(const char *question, int def)
> /* Ask user */
> int ask_question(const char *question, int def)
> {
> - if (flags & FL_AUTO) {
> + if (flags & FL_OPT_MASK) {
> + def = (flags & FL_OPT_YES) ? 1 : (flags & FL_OPT_NO) ? 0 : def;
> print_info(_("%s? %s\n"), question, def ? _("y") : _("n"));
> return def;
> }
> diff --git a/lib.h b/lib.h
> index 235719c..5d94836 100644
> --- a/lib.h
> +++ b/lib.h
> @@ -37,7 +37,11 @@
> #define FL_VERBOSE (1 << 0) /* verbose */
> #define FL_UPPER (1 << 1) /* specify upper directory */
> #define FL_WORK (1 << 2) /* specify work directory */
> -#define FL_AUTO (1 << 3) /* automactically scan dirs and repair */
> +#define FL_OPT_AUTO (1 << 3) /* automactically scan dirs and repair */
> +#define FL_OPT_NO (1 << 4) /* no changes to the filesystem */
> +#define FL_OPT_YES (1 << 5) /* yes to all questions */
> +#define FL_OPT_MASK (FL_OPT_AUTO|FL_OPT_NO|FL_OPT_YES)
> +
>
> /* Scan path type */
> #define OVL_UPPER 0
> --
> 2.9.5
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 3/6] fsck.overlay: encapsulate underlying directories options
2017-12-28 11:40 [PATCH v3 0/6] overlay: implement fsck.overlay utility zhangyi (F)
2017-12-28 11:40 ` [PATCH v3 1/6] overlay: implement fsck utility zhangyi (F)
2017-12-28 11:40 ` [PATCH v3 2/6] fsck.overlay: add -n -p and -y options zhangyi (F)
@ 2017-12-28 11:40 ` zhangyi (F)
2017-12-28 13:59 ` Amir Goldstein
2017-12-28 11:40 ` [PATCH v3 4/6] fsck.overlay: correct redirect xattr check zhangyi (F)
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: zhangyi (F) @ 2017-12-28 11:40 UTC (permalink / raw)
To: linux-unionfs, fstests
Cc: miklos, amir73il, eguan, yi.zhang, miaoxie, yangerkun
Encapsulate separate underlying directories options to use -o option as
for mount, see mount(8).
Old Usage:
fsck.overlay [-l lowerdir] [-u upperdir] [-w workdir]
New Usage:
fsck.overlay [-o lowerdir=<lowers>,upperdir=<upper>,workdir=<work>]
Example:
fsck.overlay -o lowerdir=lower,upperdir=upper,workdir=work
or
fsck.overlay -o lowerdir=lower -o upperdir=upper -o workdir=work
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
README.md | 11 ++--
check.c | 4 +-
fsck.c | 78 +++++++++++++--------------
mount.c | 182 +++++++++++++++++++++++++++++++++++---------------------------
mount.h | 12 ++++-
5 files changed, 157 insertions(+), 130 deletions(-)
diff --git a/README.md b/README.md
index 458a815..a8d0b1c 100644
--- a/README.md
+++ b/README.md
@@ -44,13 +44,11 @@ Usage
2. Run fsck.overlay program:
Usage:
- fsck.overlay [-l lowerdir] [-u upperdir] [-w workdir] [-pnyvhV]
+ fsck.overlay [-o lowerdir=<lowers>,upperdir=<upper>,workdir=<work>] [-pnyvhV]
Options:
- -l, --lowerdir=LOWERDIR specify lower directories of overlayfs,
- multiple lower use ':' as separator.
- -u, --upperdir=UPPERDIR specify upper directory of overlayfs
- -w, --workdir=WORKDIR specify work directory of overlayfs
+ -o, specify underlying directories of overlayfs:
+ multiple lower directories use ':' as separator
-p, automatic repair (no questions)
-n, make no changes to the filesystem
-y, assume "yes" to all questions
@@ -58,6 +56,9 @@ Usage
-h, --help display this usage of overlayfs
-V, --version display version information
+ Example:
+ fsck.overlay -o lowerdir=lower,upperdir=upper,workdir=work
+
3. Exit value:
0 No errors
1 Filesystem errors corrected
diff --git a/check.c b/check.c
index efa180f..0b5d49b 100644
--- a/check.c
+++ b/check.c
@@ -58,8 +58,8 @@ struct ovl_redirect_entry {
#define WHITEOUT_MOD 0
extern char **lowerdir;
-extern char upperdir[];
-extern char workdir[];
+extern char *upperdir;
+extern char *workdir;
extern int lower_num;
extern int flags;
extern int status;
diff --git a/fsck.c b/fsck.c
index 44211f7..5d34378 100644
--- a/fsck.c
+++ b/fsck.c
@@ -38,15 +38,16 @@
char *program_name;
+char *lowerdirs = NULL;
char **lowerdir = NULL;
-char upperdir[PATH_MAX] = {0};
-char workdir[PATH_MAX] = {0};
+char *upperdir = NULL;
+char *workdir = NULL;
int lower_num = 0;
int flags = 0; /* user input option flags */
int status = 0; /* fsck scan status */
-/* Cleanup lower directories buf */
-static void ovl_clean_lowerdirs(void)
+/* Cleanup underlying directories buffer */
+static void ovl_clean_dirs(void)
{
int i;
@@ -57,17 +58,19 @@ static void ovl_clean_lowerdirs(void)
free(lowerdir);
lowerdir = NULL;
lower_num = 0;
+ free(upperdir);
+ upperdir = NULL;
+ free(workdir);
+ workdir = NULL;
}
static void usage(void)
{
- print_info(_("Usage:\n\t%s [-l lowerdir] [-u upperdir] [-w workdir] "
+ print_info(_("Usage:\n\t%s [-o lowerdir=<lowers>,upperdir=<upper>,workdir=<work>] "
"[-pnyvhV]\n\n"), program_name);
print_info(_("Options:\n"
- "-l, --lowerdir=LOWERDIR specify lower directories of overlayfs,\n"
- " multiple lower use ':' as separator\n"
- "-u, --upperdir=UPPERDIR specify upper directory of overlayfs\n"
- "-w, --workdir=WORKDIR specify work directory of overlayfs\n"
+ "-o, specify underlying directories of overlayfs:\n"
+ " multiple lower directories use ':' as separator\n"
"-p, automatic repair (no questions)\n"
"-n, make no changes to the filesystem\n"
"-y, assume \"yes\" to all questions\n"
@@ -79,46 +82,25 @@ static void usage(void)
static void parse_options(int argc, char *argv[])
{
- char *lowertemp;
+ struct ovl_config config = {0};
+ char *ovl_opts = NULL;
int c;
int ret = 0;
- bool show_usage = false;
bool opt_conflict = false;
struct option long_options[] = {
- {"lowerdir", required_argument, NULL, 'l'},
- {"upperdir", required_argument, NULL, 'u'},
- {"workdir", required_argument, NULL, 'w'},
{"verbose", no_argument, NULL, 'v'},
{"version", no_argument, NULL, 'V'},
{"help", no_argument, NULL, 'h'},
{NULL, 0, NULL, 0}
};
- while ((c = getopt_long(argc, argv, "l:u:w:apnyvVh", long_options, NULL)) != -1) {
+ while ((c = getopt_long(argc, argv, "o:apnyvVh", long_options, NULL)) != -1) {
switch (c) {
- case 'l':
- lowertemp = strdup(optarg);
- ret = ovl_resolve_lowerdirs(lowertemp, &lowerdir, &lower_num);
- free(lowertemp);
- break;
- case 'u':
- if (realpath(optarg, upperdir)) {
- print_debug(_("Upperdir:%s\n"), upperdir);
- flags |= FL_UPPER;
- } else {
- print_err(_("Failed to resolve upperdir:%s\n"), optarg);
- ret = -1;
- }
- break;
- case 'w':
- if (realpath(optarg, workdir)) {
- print_debug(_("Workdir:%s\n"), workdir);
- flags |= FL_WORK;
- } else {
- print_err(_("Failed to resolve workdir:%s\n"), optarg);
- ret = -1;
- }
+ case 'o':
+ ovl_opts = sstrdup(optarg);
+ ovl_parse_opt(ovl_opts, &config);
+ free(ovl_opts);
break;
case 'a':
case 'p':
@@ -155,23 +137,35 @@ static void parse_options(int argc, char *argv[])
goto err_out;
}
+ /* Resolve and get each underlying directory of overlay filesystem */
+ ovl_get_dirs(&config, &lowerdir, &lower_num, &upperdir, &workdir);
+ if (upperdir)
+ flags |= FL_UPPER;
+ if (workdir)
+ flags |= FL_WORK;
+
if (!lower_num || (!(flags & FL_UPPER) && lower_num == 1)) {
print_info(_("Please specify correct lowerdirs and upperdir!\n\n"));
- show_usage = true;
+ goto err_out;
+ }
+
+ if ((flags & FL_UPPER) && !(flags & FL_WORK)) {
+ print_info(_("Please specify correct workdir!\n\n"));
goto err_out;
}
if (opt_conflict) {
print_info(_("Only one of the options -p/-a, -n or -y can be specified!\n\n"));
- show_usage = true;
goto err_out;
}
+ ovl_free_opt(&config);
return;
err_out:
- if (show_usage)
- usage();
+ ovl_free_opt(&config);
+ ovl_clean_dirs();
+ usage();
exit(1);
}
@@ -212,7 +206,7 @@ int main(int argc, char *argv[])
out:
fsck_status_check(&exit_value);
- ovl_clean_lowerdirs();
+ ovl_clean_dirs();
if (err)
exit_value |= FSCK_ERROR;
diff --git a/mount.c b/mount.c
index fba51e9..908cee0 100644
--- a/mount.c
+++ b/mount.c
@@ -38,19 +38,18 @@
#include "mount.h"
struct ovl_mnt_entry {
- char *lowers;
char **lowerdir;
int lowernum;
- char upperdir[PATH_MAX];
- char workdir[PATH_MAX];
+ char *upperdir;
+ char *workdir;
};
/* Mount buf allocate a time */
#define ALLOC_NUM 16
extern char **lowerdir;
-extern char upperdir[];
-extern char workdir[];
+extern char *upperdir;
+extern char *workdir;
extern int lower_num;
/*
@@ -78,9 +77,10 @@ static unsigned int ovl_split_lowerdirs(char *lower)
}
/* Resolve each lower directories and check the validity */
-int ovl_resolve_lowerdirs(char *loweropt, char ***lowerdir,
- int *lowernum)
+static int ovl_resolve_lowerdirs(char *loweropt, char ***lowerdir,
+ int *lowernum)
{
+ char temp[PATH_MAX] = {0};
int num;
char **dirs;
char *p;
@@ -97,12 +97,12 @@ int ovl_resolve_lowerdirs(char *loweropt, char ***lowerdir,
p = loweropt;
for (i = 0; i < num; i++) {
- dirs[i] = smalloc(PATH_MAX);
- if (!realpath(p, dirs[i])) {
+ if (!realpath(p, temp)) {
print_err(_("Failed to resolve lowerdir:%s:%s\n"),
p, strerror(errno));
goto err;
}
+ dirs[i] = sstrdup(temp);
print_debug(_("Lowerdir %u:%s\n"), i, dirs[i]);
p = strchr(p, '\0') + 1;
}
@@ -115,6 +115,7 @@ err:
for (i--; i >= 0; i--)
free(dirs[i]);
free(dirs);
+ *lowerdir = NULL;
*lowernum = 0;
return -1;
}
@@ -146,79 +147,99 @@ static char *ovl_next_opt(char **s)
return sbegin;
}
+static inline char *ovl_match_dump(const char *opt, const char *type)
+{
+ int len = strlen(opt) - strlen(type) + 1;
+
+ return sstrndup(opt+strlen(type), len);
+}
+
/*
- * Split and parse opt to each directories.
- *
- * Note: FIXME: We cannot distinguish mounted directories if overlayfs was
- * mounted use relative path, so there may have misjudgment.
+ * Resolve and get each underlying directory of overlay filesystem
+ */
+int ovl_get_dirs(struct ovl_config *config, char ***lowerdir,
+ int *lowernum, char **upperdir, char **workdir)
+{
+ char temp[PATH_MAX] = {0};
+
+ /* Resolve upperdir */
+ if (!realpath(config->upperdir, temp)) {
+ print_err(_("Faile to resolve upperdir:%s:%s\n"),
+ config->upperdir, strerror(errno));
+ goto err_out;
+ }
+ *upperdir = sstrdup(temp);
+ print_debug(_("Upperdir: %s\n"), *upperdir);
+
+ /* Resolve workdir */
+ if (!realpath(config->workdir, temp)) {
+ print_err(_("Faile to resolve workdir:%s:%s\n"),
+ config->workdir, strerror(errno));
+ goto err_work;
+ }
+ *workdir = sstrdup(temp);
+ print_debug(_("Workdir: %s\n"), *workdir);
+
+ /* Resolve lowerdir */
+ if (ovl_resolve_lowerdirs(config->lowerdir, lowerdir, lowernum))
+ goto err_lower;
+
+ return 0;
+
+err_lower:
+ free(*workdir);
+ *workdir = NULL;
+err_work:
+ free(*upperdir);
+ *upperdir = NULL;
+err_out:
+ return -1;
+}
+
+void ovl_free_opt(struct ovl_config *config)
+{
+ free(config->upperdir);
+ config->upperdir = NULL;
+ free(config->lowerdir);
+ config->lowerdir = NULL;
+ free(config->workdir);
+ config->workdir = NULL;
+}
+
+/*
+ * Split and parse opt to each underlying directories.
*/
-static int ovl_parse_opt(char *opt, struct ovl_mnt_entry *entry)
+void ovl_parse_opt(char *opt, struct ovl_config *config)
{
- char tmp[PATH_MAX] = {0};
char *p;
- int len;
- int ret;
- int i;
while ((p = ovl_next_opt(&opt)) != NULL) {
if (!*p)
continue;
if (!strncmp(p, OPT_UPPERDIR, strlen(OPT_UPPERDIR))) {
- len = strlen(p) - strlen(OPT_UPPERDIR) + 1;
- strncpy(tmp, p+strlen(OPT_UPPERDIR), len);
-
- if (!realpath(tmp, entry->upperdir)) {
- print_err(_("Faile to resolve path:%s:%s\n"),
- tmp, strerror(errno));
- ret = -1;
- goto errout;
- }
+ free(config->upperdir);
+ config->upperdir = ovl_match_dump(p, OPT_UPPERDIR);
} else if (!strncmp(p, OPT_LOWERDIR, strlen(OPT_LOWERDIR))) {
- len = strlen(p) - strlen(OPT_LOWERDIR) + 1;
- entry->lowers = smalloc(len);
- strncpy(entry->lowers, p+strlen(OPT_LOWERDIR), len);
-
- if ((ret = ovl_resolve_lowerdirs(entry->lowers,
- &entry->lowerdir, &entry->lowernum)))
- goto errout;
-
+ free(config->lowerdir);
+ config->lowerdir = ovl_match_dump(p, OPT_LOWERDIR);
} else if (!strncmp(p, OPT_WORKDIR, strlen(OPT_WORKDIR))) {
- len = strlen(p) - strlen(OPT_WORKDIR) + 1;
- strncpy(tmp, p+strlen(OPT_WORKDIR), len);
-
- if (!realpath(tmp, entry->workdir)) {
- print_err(_("Faile to resolve path:%s:%s\n"),
- tmp, strerror(errno));
- ret = -1;
- goto errout;
- }
+ free(config->workdir);
+ config->workdir = ovl_match_dump(p, OPT_WORKDIR);
}
}
-
-errout:
- if (entry->lowernum) {
- for (i = 0; i < entry->lowernum; i++)
- free(entry->lowerdir[i]);
- free(entry->lowerdir);
- entry->lowerdir = NULL;
- entry->lowernum = 0;
- }
- free(entry->lowers);
- entry->lowers = NULL;
-
- return ret;
}
/* Scan current mounted overlayfs and get used underlying directories */
static int ovl_scan_mount_init(struct ovl_mnt_entry **ovl_mnt_entries,
int *ovl_mnt_count)
{
- struct ovl_mnt_entry *mnt_entries;
- struct mntent *mnt;
+ struct ovl_mnt_entry *entries = NULL;
+ struct mntent *mnt = NULL;
+ struct ovl_config config = {0};
FILE *fp;
char *opt;
- int allocated, num = 0;
+ int allocated, i = 0;
fp = setmntent(MOUNT_TAB, "r");
if (!fp) {
@@ -227,32 +248,32 @@ static int ovl_scan_mount_init(struct ovl_mnt_entry **ovl_mnt_entries,
return -1;
}
- allocated = ALLOC_NUM;
- mnt_entries = smalloc(sizeof(struct ovl_mnt_entry) * allocated);
+ allocated = sizeof(struct ovl_mnt_entry) * ALLOC_NUM;
+ entries = smalloc(allocated);
while ((mnt = getmntent(fp))) {
- if (!strcmp(mnt->mnt_type, OVERLAY_NAME) ||
- !strcmp(mnt->mnt_type, OVERLAY_NAME_OLD)) {
+ if (strcmp(mnt->mnt_type, OVERLAY_NAME))
+ continue;
- opt = sstrdup(mnt->mnt_opts);
- if (ovl_parse_opt(opt, &mnt_entries[num])) {
- free(opt);
- continue;
- }
+ opt = sstrdup(mnt->mnt_opts);
+ ovl_parse_opt(opt, &config);
- num++;
- if (num % ALLOC_NUM == 0) {
- allocated += ALLOC_NUM;
- mnt_entries = srealloc(mnt_entries,
- sizeof(struct ovl_mnt_entry) * allocated);
+ if (!ovl_get_dirs(&config,
+ &entries[i].lowerdir, &entries[i].lowernum,
+ &entries[i].upperdir, &entries[i].workdir)) {
+ i++;
+ if (i % ALLOC_NUM == 0) {
+ allocated += sizeof(struct ovl_mnt_entry) * ALLOC_NUM;
+ entries = srealloc(entries, allocated);
}
-
- free(opt);
}
+
+ ovl_free_opt(&config);
+ free(opt);
}
- *ovl_mnt_entries = mnt_entries;
- *ovl_mnt_count = num;
+ *ovl_mnt_entries = entries;
+ *ovl_mnt_count = i;
endmntent(fp);
return 0;
@@ -267,7 +288,8 @@ static void ovl_scan_mount_exit(struct ovl_mnt_entry *ovl_mnt_entries,
for (j = 0; j < ovl_mnt_entries[i].lowernum; j++)
free(ovl_mnt_entries[i].lowerdir[j]);
free(ovl_mnt_entries[i].lowerdir);
- free(ovl_mnt_entries[i].lowers);
+ free(ovl_mnt_entries[i].upperdir);
+ free(ovl_mnt_entries[i].workdir);
}
free(ovl_mnt_entries);
}
@@ -279,6 +301,8 @@ static void ovl_scan_mount_exit(struct ovl_mnt_entry *ovl_mnt_entries,
*
* Note: fsck may modify lower layers, so even match only one directory
* is triggered as mounted.
+ * FIXME: We cannot distinguish mounted directories if overlayfs was
+ * mounted use relative path, so there may have misjudgment.
*/
int ovl_check_mount(bool *mounted)
{
diff --git a/mount.h b/mount.h
index 7eed0d0..2cf5e6a 100644
--- a/mount.h
+++ b/mount.h
@@ -19,8 +19,16 @@
#ifndef OVL_MOUNT_H
#define OVL_MOUNT_H
-int ovl_resolve_lowerdirs(char *loweropt, char ***lowerdir,
- int *lowernum);
+struct ovl_config {
+ char *lowerdir;
+ char *upperdir;
+ char *workdir;
+};
+
+void ovl_parse_opt(char *opt, struct ovl_config *config);
+void ovl_free_opt(struct ovl_config *config);
+int ovl_get_dirs(struct ovl_config *config, char ***lowerdir,
+ int *lowernum, char **upperdir, char **workdir);
int ovl_check_mount(bool *mounted);
#endif /* OVL_MOUNT_H */
--
2.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 3/6] fsck.overlay: encapsulate underlying directories options
2017-12-28 11:40 ` [PATCH v3 3/6] fsck.overlay: encapsulate underlying directories options zhangyi (F)
@ 2017-12-28 13:59 ` Amir Goldstein
0 siblings, 0 replies; 18+ messages in thread
From: Amir Goldstein @ 2017-12-28 13:59 UTC (permalink / raw)
To: zhangyi (F)
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On Thu, Dec 28, 2017 at 1:40 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> Encapsulate separate underlying directories options to use -o option as
> for mount, see mount(8).
>
> Old Usage:
> fsck.overlay [-l lowerdir] [-u upperdir] [-w workdir]
>
> New Usage:
> fsck.overlay [-o lowerdir=<lowers>,upperdir=<upper>,workdir=<work>]
>
> Example:
> fsck.overlay -o lowerdir=lower,upperdir=upper,workdir=work
> or
> fsck.overlay -o lowerdir=lower -o upperdir=upper -o workdir=work
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
Looks ok.
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
> README.md | 11 ++--
> check.c | 4 +-
> fsck.c | 78 +++++++++++++--------------
> mount.c | 182 +++++++++++++++++++++++++++++++++++---------------------------
> mount.h | 12 ++++-
> 5 files changed, 157 insertions(+), 130 deletions(-)
>
> diff --git a/README.md b/README.md
> index 458a815..a8d0b1c 100644
> --- a/README.md
> +++ b/README.md
> @@ -44,13 +44,11 @@ Usage
>
> 2. Run fsck.overlay program:
> Usage:
> - fsck.overlay [-l lowerdir] [-u upperdir] [-w workdir] [-pnyvhV]
> + fsck.overlay [-o lowerdir=<lowers>,upperdir=<upper>,workdir=<work>] [-pnyvhV]
>
> Options:
> - -l, --lowerdir=LOWERDIR specify lower directories of overlayfs,
> - multiple lower use ':' as separator.
> - -u, --upperdir=UPPERDIR specify upper directory of overlayfs
> - -w, --workdir=WORKDIR specify work directory of overlayfs
> + -o, specify underlying directories of overlayfs:
> + multiple lower directories use ':' as separator
> -p, automatic repair (no questions)
> -n, make no changes to the filesystem
> -y, assume "yes" to all questions
> @@ -58,6 +56,9 @@ Usage
> -h, --help display this usage of overlayfs
> -V, --version display version information
>
> + Example:
> + fsck.overlay -o lowerdir=lower,upperdir=upper,workdir=work
> +
> 3. Exit value:
> 0 No errors
> 1 Filesystem errors corrected
> diff --git a/check.c b/check.c
> index efa180f..0b5d49b 100644
> --- a/check.c
> +++ b/check.c
> @@ -58,8 +58,8 @@ struct ovl_redirect_entry {
> #define WHITEOUT_MOD 0
>
> extern char **lowerdir;
> -extern char upperdir[];
> -extern char workdir[];
> +extern char *upperdir;
> +extern char *workdir;
> extern int lower_num;
> extern int flags;
> extern int status;
> diff --git a/fsck.c b/fsck.c
> index 44211f7..5d34378 100644
> --- a/fsck.c
> +++ b/fsck.c
> @@ -38,15 +38,16 @@
>
> char *program_name;
>
> +char *lowerdirs = NULL;
> char **lowerdir = NULL;
> -char upperdir[PATH_MAX] = {0};
> -char workdir[PATH_MAX] = {0};
> +char *upperdir = NULL;
> +char *workdir = NULL;
> int lower_num = 0;
> int flags = 0; /* user input option flags */
> int status = 0; /* fsck scan status */
>
> -/* Cleanup lower directories buf */
> -static void ovl_clean_lowerdirs(void)
> +/* Cleanup underlying directories buffer */
> +static void ovl_clean_dirs(void)
> {
> int i;
>
> @@ -57,17 +58,19 @@ static void ovl_clean_lowerdirs(void)
> free(lowerdir);
> lowerdir = NULL;
> lower_num = 0;
> + free(upperdir);
> + upperdir = NULL;
> + free(workdir);
> + workdir = NULL;
> }
>
> static void usage(void)
> {
> - print_info(_("Usage:\n\t%s [-l lowerdir] [-u upperdir] [-w workdir] "
> + print_info(_("Usage:\n\t%s [-o lowerdir=<lowers>,upperdir=<upper>,workdir=<work>] "
> "[-pnyvhV]\n\n"), program_name);
> print_info(_("Options:\n"
> - "-l, --lowerdir=LOWERDIR specify lower directories of overlayfs,\n"
> - " multiple lower use ':' as separator\n"
> - "-u, --upperdir=UPPERDIR specify upper directory of overlayfs\n"
> - "-w, --workdir=WORKDIR specify work directory of overlayfs\n"
> + "-o, specify underlying directories of overlayfs:\n"
> + " multiple lower directories use ':' as separator\n"
> "-p, automatic repair (no questions)\n"
> "-n, make no changes to the filesystem\n"
> "-y, assume \"yes\" to all questions\n"
> @@ -79,46 +82,25 @@ static void usage(void)
>
> static void parse_options(int argc, char *argv[])
> {
> - char *lowertemp;
> + struct ovl_config config = {0};
> + char *ovl_opts = NULL;
> int c;
> int ret = 0;
> - bool show_usage = false;
> bool opt_conflict = false;
>
> struct option long_options[] = {
> - {"lowerdir", required_argument, NULL, 'l'},
> - {"upperdir", required_argument, NULL, 'u'},
> - {"workdir", required_argument, NULL, 'w'},
> {"verbose", no_argument, NULL, 'v'},
> {"version", no_argument, NULL, 'V'},
> {"help", no_argument, NULL, 'h'},
> {NULL, 0, NULL, 0}
> };
>
> - while ((c = getopt_long(argc, argv, "l:u:w:apnyvVh", long_options, NULL)) != -1) {
> + while ((c = getopt_long(argc, argv, "o:apnyvVh", long_options, NULL)) != -1) {
> switch (c) {
> - case 'l':
> - lowertemp = strdup(optarg);
> - ret = ovl_resolve_lowerdirs(lowertemp, &lowerdir, &lower_num);
> - free(lowertemp);
> - break;
> - case 'u':
> - if (realpath(optarg, upperdir)) {
> - print_debug(_("Upperdir:%s\n"), upperdir);
> - flags |= FL_UPPER;
> - } else {
> - print_err(_("Failed to resolve upperdir:%s\n"), optarg);
> - ret = -1;
> - }
> - break;
> - case 'w':
> - if (realpath(optarg, workdir)) {
> - print_debug(_("Workdir:%s\n"), workdir);
> - flags |= FL_WORK;
> - } else {
> - print_err(_("Failed to resolve workdir:%s\n"), optarg);
> - ret = -1;
> - }
> + case 'o':
> + ovl_opts = sstrdup(optarg);
> + ovl_parse_opt(ovl_opts, &config);
> + free(ovl_opts);
> break;
> case 'a':
> case 'p':
> @@ -155,23 +137,35 @@ static void parse_options(int argc, char *argv[])
> goto err_out;
> }
>
> + /* Resolve and get each underlying directory of overlay filesystem */
> + ovl_get_dirs(&config, &lowerdir, &lower_num, &upperdir, &workdir);
> + if (upperdir)
> + flags |= FL_UPPER;
> + if (workdir)
> + flags |= FL_WORK;
> +
> if (!lower_num || (!(flags & FL_UPPER) && lower_num == 1)) {
> print_info(_("Please specify correct lowerdirs and upperdir!\n\n"));
> - show_usage = true;
> + goto err_out;
> + }
> +
> + if ((flags & FL_UPPER) && !(flags & FL_WORK)) {
> + print_info(_("Please specify correct workdir!\n\n"));
> goto err_out;
> }
>
> if (opt_conflict) {
> print_info(_("Only one of the options -p/-a, -n or -y can be specified!\n\n"));
> - show_usage = true;
> goto err_out;
> }
>
> + ovl_free_opt(&config);
> return;
>
> err_out:
> - if (show_usage)
> - usage();
> + ovl_free_opt(&config);
> + ovl_clean_dirs();
> + usage();
> exit(1);
> }
>
> @@ -212,7 +206,7 @@ int main(int argc, char *argv[])
>
> out:
> fsck_status_check(&exit_value);
> - ovl_clean_lowerdirs();
> + ovl_clean_dirs();
>
> if (err)
> exit_value |= FSCK_ERROR;
> diff --git a/mount.c b/mount.c
> index fba51e9..908cee0 100644
> --- a/mount.c
> +++ b/mount.c
> @@ -38,19 +38,18 @@
> #include "mount.h"
>
> struct ovl_mnt_entry {
> - char *lowers;
> char **lowerdir;
> int lowernum;
> - char upperdir[PATH_MAX];
> - char workdir[PATH_MAX];
> + char *upperdir;
> + char *workdir;
> };
>
> /* Mount buf allocate a time */
> #define ALLOC_NUM 16
>
> extern char **lowerdir;
> -extern char upperdir[];
> -extern char workdir[];
> +extern char *upperdir;
> +extern char *workdir;
> extern int lower_num;
>
> /*
> @@ -78,9 +77,10 @@ static unsigned int ovl_split_lowerdirs(char *lower)
> }
>
> /* Resolve each lower directories and check the validity */
> -int ovl_resolve_lowerdirs(char *loweropt, char ***lowerdir,
> - int *lowernum)
> +static int ovl_resolve_lowerdirs(char *loweropt, char ***lowerdir,
> + int *lowernum)
> {
> + char temp[PATH_MAX] = {0};
> int num;
> char **dirs;
> char *p;
> @@ -97,12 +97,12 @@ int ovl_resolve_lowerdirs(char *loweropt, char ***lowerdir,
>
> p = loweropt;
> for (i = 0; i < num; i++) {
> - dirs[i] = smalloc(PATH_MAX);
> - if (!realpath(p, dirs[i])) {
> + if (!realpath(p, temp)) {
> print_err(_("Failed to resolve lowerdir:%s:%s\n"),
> p, strerror(errno));
> goto err;
> }
> + dirs[i] = sstrdup(temp);
> print_debug(_("Lowerdir %u:%s\n"), i, dirs[i]);
> p = strchr(p, '\0') + 1;
> }
> @@ -115,6 +115,7 @@ err:
> for (i--; i >= 0; i--)
> free(dirs[i]);
> free(dirs);
> + *lowerdir = NULL;
> *lowernum = 0;
> return -1;
> }
> @@ -146,79 +147,99 @@ static char *ovl_next_opt(char **s)
> return sbegin;
> }
>
> +static inline char *ovl_match_dump(const char *opt, const char *type)
> +{
> + int len = strlen(opt) - strlen(type) + 1;
> +
> + return sstrndup(opt+strlen(type), len);
> +}
> +
> /*
> - * Split and parse opt to each directories.
> - *
> - * Note: FIXME: We cannot distinguish mounted directories if overlayfs was
> - * mounted use relative path, so there may have misjudgment.
> + * Resolve and get each underlying directory of overlay filesystem
> + */
> +int ovl_get_dirs(struct ovl_config *config, char ***lowerdir,
> + int *lowernum, char **upperdir, char **workdir)
> +{
> + char temp[PATH_MAX] = {0};
> +
> + /* Resolve upperdir */
> + if (!realpath(config->upperdir, temp)) {
> + print_err(_("Faile to resolve upperdir:%s:%s\n"),
> + config->upperdir, strerror(errno));
> + goto err_out;
> + }
> + *upperdir = sstrdup(temp);
> + print_debug(_("Upperdir: %s\n"), *upperdir);
> +
> + /* Resolve workdir */
> + if (!realpath(config->workdir, temp)) {
> + print_err(_("Faile to resolve workdir:%s:%s\n"),
> + config->workdir, strerror(errno));
> + goto err_work;
> + }
> + *workdir = sstrdup(temp);
> + print_debug(_("Workdir: %s\n"), *workdir);
> +
> + /* Resolve lowerdir */
> + if (ovl_resolve_lowerdirs(config->lowerdir, lowerdir, lowernum))
> + goto err_lower;
> +
> + return 0;
> +
> +err_lower:
> + free(*workdir);
> + *workdir = NULL;
> +err_work:
> + free(*upperdir);
> + *upperdir = NULL;
> +err_out:
> + return -1;
> +}
> +
> +void ovl_free_opt(struct ovl_config *config)
> +{
> + free(config->upperdir);
> + config->upperdir = NULL;
> + free(config->lowerdir);
> + config->lowerdir = NULL;
> + free(config->workdir);
> + config->workdir = NULL;
> +}
> +
> +/*
> + * Split and parse opt to each underlying directories.
> */
> -static int ovl_parse_opt(char *opt, struct ovl_mnt_entry *entry)
> +void ovl_parse_opt(char *opt, struct ovl_config *config)
> {
> - char tmp[PATH_MAX] = {0};
> char *p;
> - int len;
> - int ret;
> - int i;
>
> while ((p = ovl_next_opt(&opt)) != NULL) {
> if (!*p)
> continue;
>
> if (!strncmp(p, OPT_UPPERDIR, strlen(OPT_UPPERDIR))) {
> - len = strlen(p) - strlen(OPT_UPPERDIR) + 1;
> - strncpy(tmp, p+strlen(OPT_UPPERDIR), len);
> -
> - if (!realpath(tmp, entry->upperdir)) {
> - print_err(_("Faile to resolve path:%s:%s\n"),
> - tmp, strerror(errno));
> - ret = -1;
> - goto errout;
> - }
> + free(config->upperdir);
> + config->upperdir = ovl_match_dump(p, OPT_UPPERDIR);
> } else if (!strncmp(p, OPT_LOWERDIR, strlen(OPT_LOWERDIR))) {
> - len = strlen(p) - strlen(OPT_LOWERDIR) + 1;
> - entry->lowers = smalloc(len);
> - strncpy(entry->lowers, p+strlen(OPT_LOWERDIR), len);
> -
> - if ((ret = ovl_resolve_lowerdirs(entry->lowers,
> - &entry->lowerdir, &entry->lowernum)))
> - goto errout;
> -
> + free(config->lowerdir);
> + config->lowerdir = ovl_match_dump(p, OPT_LOWERDIR);
> } else if (!strncmp(p, OPT_WORKDIR, strlen(OPT_WORKDIR))) {
> - len = strlen(p) - strlen(OPT_WORKDIR) + 1;
> - strncpy(tmp, p+strlen(OPT_WORKDIR), len);
> -
> - if (!realpath(tmp, entry->workdir)) {
> - print_err(_("Faile to resolve path:%s:%s\n"),
> - tmp, strerror(errno));
> - ret = -1;
> - goto errout;
> - }
> + free(config->workdir);
> + config->workdir = ovl_match_dump(p, OPT_WORKDIR);
> }
> }
> -
> -errout:
> - if (entry->lowernum) {
> - for (i = 0; i < entry->lowernum; i++)
> - free(entry->lowerdir[i]);
> - free(entry->lowerdir);
> - entry->lowerdir = NULL;
> - entry->lowernum = 0;
> - }
> - free(entry->lowers);
> - entry->lowers = NULL;
> -
> - return ret;
> }
>
> /* Scan current mounted overlayfs and get used underlying directories */
> static int ovl_scan_mount_init(struct ovl_mnt_entry **ovl_mnt_entries,
> int *ovl_mnt_count)
> {
> - struct ovl_mnt_entry *mnt_entries;
> - struct mntent *mnt;
> + struct ovl_mnt_entry *entries = NULL;
> + struct mntent *mnt = NULL;
> + struct ovl_config config = {0};
> FILE *fp;
> char *opt;
> - int allocated, num = 0;
> + int allocated, i = 0;
>
> fp = setmntent(MOUNT_TAB, "r");
> if (!fp) {
> @@ -227,32 +248,32 @@ static int ovl_scan_mount_init(struct ovl_mnt_entry **ovl_mnt_entries,
> return -1;
> }
>
> - allocated = ALLOC_NUM;
> - mnt_entries = smalloc(sizeof(struct ovl_mnt_entry) * allocated);
> + allocated = sizeof(struct ovl_mnt_entry) * ALLOC_NUM;
> + entries = smalloc(allocated);
>
> while ((mnt = getmntent(fp))) {
> - if (!strcmp(mnt->mnt_type, OVERLAY_NAME) ||
> - !strcmp(mnt->mnt_type, OVERLAY_NAME_OLD)) {
> + if (strcmp(mnt->mnt_type, OVERLAY_NAME))
> + continue;
>
> - opt = sstrdup(mnt->mnt_opts);
> - if (ovl_parse_opt(opt, &mnt_entries[num])) {
> - free(opt);
> - continue;
> - }
> + opt = sstrdup(mnt->mnt_opts);
> + ovl_parse_opt(opt, &config);
>
> - num++;
> - if (num % ALLOC_NUM == 0) {
> - allocated += ALLOC_NUM;
> - mnt_entries = srealloc(mnt_entries,
> - sizeof(struct ovl_mnt_entry) * allocated);
> + if (!ovl_get_dirs(&config,
> + &entries[i].lowerdir, &entries[i].lowernum,
> + &entries[i].upperdir, &entries[i].workdir)) {
> + i++;
> + if (i % ALLOC_NUM == 0) {
> + allocated += sizeof(struct ovl_mnt_entry) * ALLOC_NUM;
> + entries = srealloc(entries, allocated);
> }
> -
> - free(opt);
> }
> +
> + ovl_free_opt(&config);
> + free(opt);
> }
>
> - *ovl_mnt_entries = mnt_entries;
> - *ovl_mnt_count = num;
> + *ovl_mnt_entries = entries;
> + *ovl_mnt_count = i;
>
> endmntent(fp);
> return 0;
> @@ -267,7 +288,8 @@ static void ovl_scan_mount_exit(struct ovl_mnt_entry *ovl_mnt_entries,
> for (j = 0; j < ovl_mnt_entries[i].lowernum; j++)
> free(ovl_mnt_entries[i].lowerdir[j]);
> free(ovl_mnt_entries[i].lowerdir);
> - free(ovl_mnt_entries[i].lowers);
> + free(ovl_mnt_entries[i].upperdir);
> + free(ovl_mnt_entries[i].workdir);
> }
> free(ovl_mnt_entries);
> }
> @@ -279,6 +301,8 @@ static void ovl_scan_mount_exit(struct ovl_mnt_entry *ovl_mnt_entries,
> *
> * Note: fsck may modify lower layers, so even match only one directory
> * is triggered as mounted.
> + * FIXME: We cannot distinguish mounted directories if overlayfs was
> + * mounted use relative path, so there may have misjudgment.
> */
> int ovl_check_mount(bool *mounted)
> {
> diff --git a/mount.h b/mount.h
> index 7eed0d0..2cf5e6a 100644
> --- a/mount.h
> +++ b/mount.h
> @@ -19,8 +19,16 @@
> #ifndef OVL_MOUNT_H
> #define OVL_MOUNT_H
>
> -int ovl_resolve_lowerdirs(char *loweropt, char ***lowerdir,
> - int *lowernum);
> +struct ovl_config {
> + char *lowerdir;
> + char *upperdir;
> + char *workdir;
> +};
> +
> +void ovl_parse_opt(char *opt, struct ovl_config *config);
> +void ovl_free_opt(struct ovl_config *config);
> +int ovl_get_dirs(struct ovl_config *config, char ***lowerdir,
> + int *lowernum, char **upperdir, char **workdir);
> int ovl_check_mount(bool *mounted);
>
> #endif /* OVL_MOUNT_H */
> --
> 2.9.5
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 4/6] fsck.overlay: correct redirect xattr check
2017-12-28 11:40 [PATCH v3 0/6] overlay: implement fsck.overlay utility zhangyi (F)
` (2 preceding siblings ...)
2017-12-28 11:40 ` [PATCH v3 3/6] fsck.overlay: encapsulate underlying directories options zhangyi (F)
@ 2017-12-28 11:40 ` zhangyi (F)
2017-12-28 14:22 ` Amir Goldstein
2017-12-28 11:40 ` [PATCH v3 5/6] fsck.overlay: fix lower target lookup zhangyi (F)
2017-12-28 11:40 ` [PATCH v3 6/6] fsck.overlay: add impure xattr check zhangyi (F)
5 siblings, 1 reply; 18+ messages in thread
From: zhangyi (F) @ 2017-12-28 11:40 UTC (permalink / raw)
To: linux-unionfs, fstests
Cc: miklos, amir73il, eguan, yi.zhang, miaoxie, yangerkun
When we found a valid redirect xattr and the lower origin targed was
covered by a directory, it could be 1) an another redirect directory,
or 2) an opaque directory created after rename in overlayfs or 3) a
general directory created in the underlying directory when overlay is
offline. Currently, we only handled case 2, this patch cover the other
two cases.
This patch ask user if we find a covering directory whitout opaque
and another redirect xattr.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
README.md | 31 ++++++++++++++-------
check.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++-----------------
2 files changed, 90 insertions(+), 35 deletions(-)
diff --git a/README.md b/README.md
index a8d0b1c..2e6146f 100644
--- a/README.md
+++ b/README.md
@@ -25,16 +25,27 @@ If an redirect directory is found, the following must be met:
1) The directory path pointed by redirect xattr should exist in one of lower
layers.
-2) The origin directory should be redirected only once in one layer, which means
- there is only one redirect xattr point to this origin directory in the
- specified layer.
-3) A whiteout or an opaque directory with the same name to origin should exist
- in the same directory as the redirect directory.
-
-If not, 1) The redirect xattr is invalid and need to remove 2) One of the
-redirect xattr is redundant but not sure which one is, ask user or warn in
-auto mode 3) Create a whiteout device or set opaque xattr to an existing
-directory if the parent directory was meregd or remove xattr if not.
+2) The origin directory should be redirected only once in one layer, which
+ means there is only one redirect xattr point to this origin directory in
+ the specified layer.
+3) There must be something with the same name to the rename origin in its
+ origin place, maybe a whiteout/file or a(an) opaque/redirect directory
+ covering the lower target, maybe a new created directory merging with
+ lower origin directory.
+
+If not,
+1) The redirect xattr is invalid and need to remove.
+2) One of the redirect xattr is redundant but not sure which one is, ask user
+ or warn in auto mode.
+3) Create a whiteout device if there nothing exists in its place. If the lower
+ targed was covered by a directory which is not an another redirect directory,
+ it could be created after rename in overlayfs or created in underlying
+ directory when overlay is offline, not sure this directory is merged or not,
+ ask user by default.
+
+If an invalid redirect xattr was removed by fsck and there is a corresponding
+lower directory with the same name exists, not sure this directory is merged
+or not, ask user to make a decision.
Usage
=====
diff --git a/check.c b/check.c
index 0b5d49b..6db41cd 100644
--- a/check.c
+++ b/check.c
@@ -152,12 +152,29 @@ static inline int ovl_create_whiteout(const char *pathname)
return ret;
}
-static inline int ovl_ask_invalid(const char *question, const char *pathname,
- int action)
+static inline bool ovl_is_redirect(const char *pathname)
+{
+ bool exist = false;
+ ssize_t ret;
+
+ ret = get_xattr(pathname, OVL_REDIRECT_XATTR, NULL, &exist);
+ return ret ? false : exist;
+}
+
+static inline int ovl_ask_action(const char *description, const char *pathname,
+ const char *question, int action)
+{
+ print_info(_("%s: %s "), description, pathname);
+
+ return ask_question(question, action);
+}
+
+static inline int ovl_ask_question(const char *question, const char *pathname,
+ int action)
{
print_info(_("%s: %s "), question, pathname);
- return ask_question("Remove", action);
+ return ask_question("", action);
}
/*
@@ -233,7 +250,7 @@ remove:
sctx->i_whiteouts++;
/* Remove orphan whiteout directly or ask user */
- if (!ovl_ask_invalid("Orphan whiteout", pathname, 1))
+ if (!ovl_ask_action("Orphan whiteout", pathname, "Remove", 1))
return 0;
ret = unlink(pathname);
@@ -323,15 +340,21 @@ static void ovl_redirect_free(void)
* 2) Count how many directories the origin directory was redirected by.
* If more than one in the same layer, there must be some inconsistency
* but not sure which one is invalid, ask user or warn in auto mode.
- * 3) Check and fix the missing whiteout or opaque in redierct parent dir.
+ * 3) Check and fix the missing whiteout that covering the redirect lower
+ * target. If the lower targed was covered by a directory which is not
+ * an another redirect directory, it could be created after rename in
+ * overlayfs or created in underlying directory when overlay is offline,
+ * check opaque xattr and ask user by default.
+ * 4) When an invalid redirect xattr is removed, and the lower layers have
+ * directory with same name, not sure this is a merged directory, ask
+ * user by default.
*/
static int ovl_check_redirect(struct scan_ctx *sctx)
{
const char *pathname = sctx->pathname;
struct ovl_lower_check chk = {0};
- char redirect_rpath[PATH_MAX];
struct stat rst;
- char *redirect = NULL;
+ char *redirect = NULL, *cover_path = NULL;
int start;
int ret = 0;
@@ -345,8 +368,10 @@ static int ovl_check_redirect(struct scan_ctx *sctx)
sctx->t_redirects++;
/* Redirect dir in last lower dir ? */
- if (sctx->dirtype == OVL_LOWER && sctx->num == lower_num-1)
+ if (sctx->dirtype == OVL_LOWER && sctx->num == lower_num-1) {
+ start = lower_num;
goto remove;
+ }
/* Scan lower directories to check redirect dir exist or not */
start = (sctx->dirtype == OVL_LOWER) ? sctx->num + 1 : 0;
@@ -359,14 +384,14 @@ static int ovl_check_redirect(struct scan_ctx *sctx)
/* Check duplicate in same layer */
if (ovl_redirect_entry_find(chk.path, sctx->dirtype,
sctx->num, &tmp)) {
- print_info("Duplicate redirect directories found:\n");
- print_info("origin:%s current:%s last:%s\n",
- chk.path, pathname, tmp);
+ print_info("Duplicate redirect directory found:%s\n", pathname);
+ print_info("Origin:%s Previous:%s\n", chk.path, tmp);
sctx->i_redirects++;
/* Don't remove in auto mode */
- if (ovl_ask_invalid("Duplicate redirect xattr", pathname, 0))
+ if (ovl_ask_action("Duplicate redirect xattr", pathname,
+ "Remove", 0))
goto remove_d;
}
@@ -374,25 +399,32 @@ static int ovl_check_redirect(struct scan_ctx *sctx)
sctx->num, chk.path);
/* Check and fix whiteout or opaque dir */
- path_pack(redirect_rpath, sizeof(redirect_rpath),
- sctx->dirname, redirect);
- if (lstat(redirect_rpath, &rst) != 0) {
+ cover_path = path_pack(cover_path, 0, sctx->dirname, redirect);
+ if (lstat(cover_path, &rst) != 0) {
if (errno != ENOENT && errno != ENOTDIR) {
print_err(_("Cannot stat %s: %s\n"),
- redirect_rpath, strerror(errno));
+ cover_path, strerror(errno));
goto out;
}
/* Found nothing, create a whiteout */
- if ((ret = ovl_create_whiteout(redirect_rpath)))
+ if ((ret = ovl_create_whiteout(cover_path)))
goto out;
sctx->t_whiteouts++;
- } else if (is_dir(&rst) && !ovl_is_opaque(redirect_rpath)) {
- /* Found a directory but not opaqued, fix opaque xattr */
- if ((ret = ovl_set_opaque(redirect_rpath)))
- goto out;
+ } else if (is_dir(&rst) && !ovl_is_opaque(cover_path) &&
+ !ovl_is_redirect(cover_path)) {
+
+ /*
+ * Found a directory but not opaqued and not even
+ * an another redirected directory, maybe created
+ * when overlayfs is offline, ask user to add the
+ * opaque xattr if this is not a merged dir.
+ */
+ if (!ovl_ask_question("Is merged dir", cover_path, 1))
+ if ((ret = ovl_set_opaque(cover_path)))
+ goto out;
}
goto out;
@@ -402,13 +434,25 @@ remove:
sctx->i_redirects++;
/* Remove redirect xattr or ask user */
- if (!ovl_ask_invalid("Invalid redirect xattr", pathname, 1))
+ if (!ovl_ask_action("Invalid redirect xattr", pathname, "Remove", 1))
goto out;
remove_d:
- ret = ovl_remove_redirect(pathname);
- if (!ret)
- sctx->i_redirects--;
+ if ((ret = ovl_remove_redirect(pathname)))
+ goto out;
+
+ sctx->i_redirects--;
+
+ /* If lower directory exist, ask user to add opaque xattr to cover it */
+ if (start < lower_num) {
+ if ((ret = ovl_check_lower(path_pick(pathname, sctx->dirlen),
+ start, &chk, false)))
+ goto out;
+ if (chk.exist && is_dir(&chk.st) &&
+ !ovl_ask_question("Is merged dir", pathname, 1))
+ ret = ovl_set_opaque(pathname);
+ }
out:
+ free(cover_path);
free(redirect);
return ret;
}
--
2.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 4/6] fsck.overlay: correct redirect xattr check
2017-12-28 11:40 ` [PATCH v3 4/6] fsck.overlay: correct redirect xattr check zhangyi (F)
@ 2017-12-28 14:22 ` Amir Goldstein
2017-12-29 3:23 ` zhangyi (F)
0 siblings, 1 reply; 18+ messages in thread
From: Amir Goldstein @ 2017-12-28 14:22 UTC (permalink / raw)
To: zhangyi (F)
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On Thu, Dec 28, 2017 at 1:40 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> When we found a valid redirect xattr and the lower origin targed was
> covered by a directory, it could be 1) an another redirect directory,
> or 2) an opaque directory created after rename in overlayfs or 3) a
> general directory created in the underlying directory when overlay is
> offline. Currently, we only handled case 2, this patch cover the other
> two cases.
>
> This patch ask user if we find a covering directory whitout opaque
> and another redirect xattr.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
> README.md | 31 ++++++++++++++-------
> check.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++-----------------
> 2 files changed, 90 insertions(+), 35 deletions(-)
>
> diff --git a/README.md b/README.md
> index a8d0b1c..2e6146f 100644
> --- a/README.md
> +++ b/README.md
> @@ -25,16 +25,27 @@ If an redirect directory is found, the following must be met:
>
> 1) The directory path pointed by redirect xattr should exist in one of lower
> layers.
> -2) The origin directory should be redirected only once in one layer, which means
> - there is only one redirect xattr point to this origin directory in the
> - specified layer.
> -3) A whiteout or an opaque directory with the same name to origin should exist
> - in the same directory as the redirect directory.
> -
> -If not, 1) The redirect xattr is invalid and need to remove 2) One of the
> -redirect xattr is redundant but not sure which one is, ask user or warn in
> -auto mode 3) Create a whiteout device or set opaque xattr to an existing
> -directory if the parent directory was meregd or remove xattr if not.
> +2) The origin directory should be redirected only once in one layer, which
> + means there is only one redirect xattr point to this origin directory in
> + the specified layer.
> +3) There must be something with the same name to the rename origin in its
> + origin place, maybe a whiteout/file or a(an) opaque/redirect directory
> + covering the lower target, maybe a new created directory merging with
> + lower origin directory.
> +
> +If not,
> +1) The redirect xattr is invalid and need to remove.
> +2) One of the redirect xattr is redundant but not sure which one is, ask user
> + or warn in auto mode.
> +3) Create a whiteout device if there nothing exists in its place. If the lower
> + targed was covered by a directory which is not an another redirect directory,
> + it could be created after rename in overlayfs or created in underlying
> + directory when overlay is offline, not sure this directory is merged or not,
> + ask user by default.
> +
> +If an invalid redirect xattr was removed by fsck and there is a corresponding
> +lower directory with the same name exists, not sure this directory is merged
> +or not, ask user to make a decision.
>
> Usage
> =====
> diff --git a/check.c b/check.c
> index 0b5d49b..6db41cd 100644
> --- a/check.c
> +++ b/check.c
> @@ -152,12 +152,29 @@ static inline int ovl_create_whiteout(const char *pathname)
> return ret;
> }
>
> -static inline int ovl_ask_invalid(const char *question, const char *pathname,
> - int action)
> +static inline bool ovl_is_redirect(const char *pathname)
> +{
> + bool exist = false;
> + ssize_t ret;
> +
> + ret = get_xattr(pathname, OVL_REDIRECT_XATTR, NULL, &exist);
> + return ret ? false : exist;
> +}
> +
> +static inline int ovl_ask_action(const char *description, const char *pathname,
> + const char *question, int action)
> +{
> + print_info(_("%s: %s "), description, pathname);
> +
> + return ask_question(question, action);
> +}
> +
> +static inline int ovl_ask_question(const char *question, const char *pathname,
> + int action)
> {
> print_info(_("%s: %s "), question, pathname);
>
> - return ask_question("Remove", action);
> + return ask_question("", action);
> }
>
> /*
> @@ -233,7 +250,7 @@ remove:
> sctx->i_whiteouts++;
>
> /* Remove orphan whiteout directly or ask user */
> - if (!ovl_ask_invalid("Orphan whiteout", pathname, 1))
> + if (!ovl_ask_action("Orphan whiteout", pathname, "Remove", 1))
> return 0;
>
> ret = unlink(pathname);
> @@ -323,15 +340,21 @@ static void ovl_redirect_free(void)
> * 2) Count how many directories the origin directory was redirected by.
> * If more than one in the same layer, there must be some inconsistency
> * but not sure which one is invalid, ask user or warn in auto mode.
> - * 3) Check and fix the missing whiteout or opaque in redierct parent dir.
> + * 3) Check and fix the missing whiteout that covering the redirect lower
> + * target. If the lower targed was covered by a directory which is not
> + * an another redirect directory, it could be created after rename in
> + * overlayfs or created in underlying directory when overlay is offline,
> + * check opaque xattr and ask user by default.
> + * 4) When an invalid redirect xattr is removed, and the lower layers have
> + * directory with same name, not sure this is a merged directory, ask
> + * user by default.
> */
> static int ovl_check_redirect(struct scan_ctx *sctx)
> {
> const char *pathname = sctx->pathname;
> struct ovl_lower_check chk = {0};
> - char redirect_rpath[PATH_MAX];
> struct stat rst;
> - char *redirect = NULL;
> + char *redirect = NULL, *cover_path = NULL;
> int start;
> int ret = 0;
>
> @@ -345,8 +368,10 @@ static int ovl_check_redirect(struct scan_ctx *sctx)
> sctx->t_redirects++;
>
> /* Redirect dir in last lower dir ? */
> - if (sctx->dirtype == OVL_LOWER && sctx->num == lower_num-1)
> + if (sctx->dirtype == OVL_LOWER && sctx->num == lower_num-1) {
> + start = lower_num;
> goto remove;
> + }
>
> /* Scan lower directories to check redirect dir exist or not */
> start = (sctx->dirtype == OVL_LOWER) ? sctx->num + 1 : 0;
> @@ -359,14 +384,14 @@ static int ovl_check_redirect(struct scan_ctx *sctx)
> /* Check duplicate in same layer */
> if (ovl_redirect_entry_find(chk.path, sctx->dirtype,
> sctx->num, &tmp)) {
> - print_info("Duplicate redirect directories found:\n");
> - print_info("origin:%s current:%s last:%s\n",
> - chk.path, pathname, tmp);
> + print_info("Duplicate redirect directory found:%s\n", pathname);
> + print_info("Origin:%s Previous:%s\n", chk.path, tmp);
>
> sctx->i_redirects++;
>
> /* Don't remove in auto mode */
> - if (ovl_ask_invalid("Duplicate redirect xattr", pathname, 0))
> + if (ovl_ask_action("Duplicate redirect xattr", pathname,
> + "Remove", 0))
> goto remove_d;
> }
>
> @@ -374,25 +399,32 @@ static int ovl_check_redirect(struct scan_ctx *sctx)
> sctx->num, chk.path);
>
> /* Check and fix whiteout or opaque dir */
> - path_pack(redirect_rpath, sizeof(redirect_rpath),
> - sctx->dirname, redirect);
> - if (lstat(redirect_rpath, &rst) != 0) {
> + cover_path = path_pack(cover_path, 0, sctx->dirname, redirect);
> + if (lstat(cover_path, &rst) != 0) {
> if (errno != ENOENT && errno != ENOTDIR) {
> print_err(_("Cannot stat %s: %s\n"),
> - redirect_rpath, strerror(errno));
> + cover_path, strerror(errno));
> goto out;
> }
>
> /* Found nothing, create a whiteout */
> - if ((ret = ovl_create_whiteout(redirect_rpath)))
> + if ((ret = ovl_create_whiteout(cover_path)))
> goto out;
>
> sctx->t_whiteouts++;
>
> - } else if (is_dir(&rst) && !ovl_is_opaque(redirect_rpath)) {
> - /* Found a directory but not opaqued, fix opaque xattr */
> - if ((ret = ovl_set_opaque(redirect_rpath)))
> - goto out;
> + } else if (is_dir(&rst) && !ovl_is_opaque(cover_path) &&
> + !ovl_is_redirect(cover_path)) {
> +
> + /*
> + * Found a directory but not opaqued and not even
> + * an another redirected directory, maybe created
> + * when overlayfs is offline, ask user to add the
> + * opaque xattr if this is not a merged dir.
> + */
> + if (!ovl_ask_question("Is merged dir", cover_path, 1))
I am not sure what user will understand from this question,
but maybe I don't see the full flow of info/questions in my mind.
Anyway, if I am not mistaken, you seem to hold the opinion that
default behavior if user says (-y := fix it I don't care how) should be
to set the opaque xattr. If you agree, then you should ask:
if (ovl_ask_question("Should set opaque dir", cover_path, 1))
> + if ((ret = ovl_set_opaque(cover_path)))
> + goto out;
> }
>
> goto out;
> @@ -402,13 +434,25 @@ remove:
> sctx->i_redirects++;
>
> /* Remove redirect xattr or ask user */
> - if (!ovl_ask_invalid("Invalid redirect xattr", pathname, 1))
> + if (!ovl_ask_action("Invalid redirect xattr", pathname, "Remove", 1))
> goto out;
> remove_d:
> - ret = ovl_remove_redirect(pathname);
> - if (!ret)
> - sctx->i_redirects--;
> + if ((ret = ovl_remove_redirect(pathname)))
> + goto out;
> +
> + sctx->i_redirects--;
> +
> + /* If lower directory exist, ask user to add opaque xattr to cover it */
> + if (start < lower_num) {
> + if ((ret = ovl_check_lower(path_pick(pathname, sctx->dirlen),
> + start, &chk, false)))
> + goto out;
> + if (chk.exist && is_dir(&chk.st) &&
> + !ovl_ask_question("Is merged dir", pathname, 1))
> + ret = ovl_set_opaque(pathname);
Same here. Better that fsck -y will set opaque right?
So flip the question to "Should set opaque dir"
> + }
> out:
> + free(cover_path);
> free(redirect);
> return ret;
> }
> --
> 2.9.5
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v3 4/6] fsck.overlay: correct redirect xattr check
2017-12-28 14:22 ` Amir Goldstein
@ 2017-12-29 3:23 ` zhangyi (F)
2017-12-29 9:32 ` Amir Goldstein
0 siblings, 1 reply; 18+ messages in thread
From: zhangyi (F) @ 2017-12-29 3:23 UTC (permalink / raw)
To: Amir Goldstein
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On 2017/12/28 22:22, Amir Goldstein Wrote:
> On Thu, Dec 28, 2017 at 1:40 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> When we found a valid redirect xattr and the lower origin targed was
>> covered by a directory, it could be 1) an another redirect directory,
>> or 2) an opaque directory created after rename in overlayfs or 3) a
>> general directory created in the underlying directory when overlay is
>> offline. Currently, we only handled case 2, this patch cover the other
>> two cases.
>>
>> This patch ask user if we find a covering directory whitout opaque
>> and another redirect xattr.
>>
>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>> ---
>> README.md | 31 ++++++++++++++-------
>> check.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++-----------------
>> 2 files changed, 90 insertions(+), 35 deletions(-)
>>
>> diff --git a/README.md b/README.md
>> index a8d0b1c..2e6146f 100644
>> --- a/README.md
>> +++ b/README.md
>> @@ -25,16 +25,27 @@ If an redirect directory is found, the following must be met:
>>
>> 1) The directory path pointed by redirect xattr should exist in one of lower
>> layers.
>> -2) The origin directory should be redirected only once in one layer, which means
>> - there is only one redirect xattr point to this origin directory in the
>> - specified layer.
>> -3) A whiteout or an opaque directory with the same name to origin should exist
>> - in the same directory as the redirect directory.
>> -
>> -If not, 1) The redirect xattr is invalid and need to remove 2) One of the
>> -redirect xattr is redundant but not sure which one is, ask user or warn in
>> -auto mode 3) Create a whiteout device or set opaque xattr to an existing
>> -directory if the parent directory was meregd or remove xattr if not.
>> +2) The origin directory should be redirected only once in one layer, which
>> + means there is only one redirect xattr point to this origin directory in
>> + the specified layer.
>> +3) There must be something with the same name to the rename origin in its
>> + origin place, maybe a whiteout/file or a(an) opaque/redirect directory
>> + covering the lower target, maybe a new created directory merging with
>> + lower origin directory.
>> +
>> +If not,
>> +1) The redirect xattr is invalid and need to remove.
>> +2) One of the redirect xattr is redundant but not sure which one is, ask user
>> + or warn in auto mode.
>> +3) Create a whiteout device if there nothing exists in its place. If the lower
>> + targed was covered by a directory which is not an another redirect directory,
>> + it could be created after rename in overlayfs or created in underlying
>> + directory when overlay is offline, not sure this directory is merged or not,
>> + ask user by default.
>> +
>> +If an invalid redirect xattr was removed by fsck and there is a corresponding
>> +lower directory with the same name exists, not sure this directory is merged
>> +or not, ask user to make a decision.
>>
>> Usage
>> =====
>> diff --git a/check.c b/check.c
>> index 0b5d49b..6db41cd 100644
>> --- a/check.c
>> +++ b/check.c
>> @@ -152,12 +152,29 @@ static inline int ovl_create_whiteout(const char *pathname)
>> return ret;
>> }
>>
>> -static inline int ovl_ask_invalid(const char *question, const char *pathname,
>> - int action)
>> +static inline bool ovl_is_redirect(const char *pathname)
>> +{
>> + bool exist = false;
>> + ssize_t ret;
>> +
>> + ret = get_xattr(pathname, OVL_REDIRECT_XATTR, NULL, &exist);
>> + return ret ? false : exist;
>> +}
>> +
>> +static inline int ovl_ask_action(const char *description, const char *pathname,
>> + const char *question, int action)
>> +{
>> + print_info(_("%s: %s "), description, pathname);
>> +
>> + return ask_question(question, action);
>> +}
>> +
>> +static inline int ovl_ask_question(const char *question, const char *pathname,
>> + int action)
>> {
>> print_info(_("%s: %s "), question, pathname);
>>
>> - return ask_question("Remove", action);
>> + return ask_question("", action);
>> }
>>
>> /*
>> @@ -233,7 +250,7 @@ remove:
>> sctx->i_whiteouts++;
>>
>> /* Remove orphan whiteout directly or ask user */
>> - if (!ovl_ask_invalid("Orphan whiteout", pathname, 1))
>> + if (!ovl_ask_action("Orphan whiteout", pathname, "Remove", 1))
>> return 0;
>>
>> ret = unlink(pathname);
>> @@ -323,15 +340,21 @@ static void ovl_redirect_free(void)
>> * 2) Count how many directories the origin directory was redirected by.
>> * If more than one in the same layer, there must be some inconsistency
>> * but not sure which one is invalid, ask user or warn in auto mode.
>> - * 3) Check and fix the missing whiteout or opaque in redierct parent dir.
>> + * 3) Check and fix the missing whiteout that covering the redirect lower
>> + * target. If the lower targed was covered by a directory which is not
>> + * an another redirect directory, it could be created after rename in
>> + * overlayfs or created in underlying directory when overlay is offline,
>> + * check opaque xattr and ask user by default.
>> + * 4) When an invalid redirect xattr is removed, and the lower layers have
>> + * directory with same name, not sure this is a merged directory, ask
>> + * user by default.
>> */
>> static int ovl_check_redirect(struct scan_ctx *sctx)
>> {
>> const char *pathname = sctx->pathname;
>> struct ovl_lower_check chk = {0};
>> - char redirect_rpath[PATH_MAX];
>> struct stat rst;
>> - char *redirect = NULL;
>> + char *redirect = NULL, *cover_path = NULL;
>> int start;
>> int ret = 0;
>>
>> @@ -345,8 +368,10 @@ static int ovl_check_redirect(struct scan_ctx *sctx)
>> sctx->t_redirects++;
>>
>> /* Redirect dir in last lower dir ? */
>> - if (sctx->dirtype == OVL_LOWER && sctx->num == lower_num-1)
>> + if (sctx->dirtype == OVL_LOWER && sctx->num == lower_num-1) {
>> + start = lower_num;
>> goto remove;
>> + }
>>
>> /* Scan lower directories to check redirect dir exist or not */
>> start = (sctx->dirtype == OVL_LOWER) ? sctx->num + 1 : 0;
>> @@ -359,14 +384,14 @@ static int ovl_check_redirect(struct scan_ctx *sctx)
>> /* Check duplicate in same layer */
>> if (ovl_redirect_entry_find(chk.path, sctx->dirtype,
>> sctx->num, &tmp)) {
>> - print_info("Duplicate redirect directories found:\n");
>> - print_info("origin:%s current:%s last:%s\n",
>> - chk.path, pathname, tmp);
>> + print_info("Duplicate redirect directory found:%s\n", pathname);
>> + print_info("Origin:%s Previous:%s\n", chk.path, tmp);
>>
>> sctx->i_redirects++;
>>
>> /* Don't remove in auto mode */
>> - if (ovl_ask_invalid("Duplicate redirect xattr", pathname, 0))
>> + if (ovl_ask_action("Duplicate redirect xattr", pathname,
>> + "Remove", 0))
>> goto remove_d;
>> }
>>
>> @@ -374,25 +399,32 @@ static int ovl_check_redirect(struct scan_ctx *sctx)
>> sctx->num, chk.path);
>>
>> /* Check and fix whiteout or opaque dir */
>> - path_pack(redirect_rpath, sizeof(redirect_rpath),
>> - sctx->dirname, redirect);
>> - if (lstat(redirect_rpath, &rst) != 0) {
>> + cover_path = path_pack(cover_path, 0, sctx->dirname, redirect);
>> + if (lstat(cover_path, &rst) != 0) {
>> if (errno != ENOENT && errno != ENOTDIR) {
>> print_err(_("Cannot stat %s: %s\n"),
>> - redirect_rpath, strerror(errno));
>> + cover_path, strerror(errno));
>> goto out;
>> }
>>
>> /* Found nothing, create a whiteout */
>> - if ((ret = ovl_create_whiteout(redirect_rpath)))
>> + if ((ret = ovl_create_whiteout(cover_path)))
>> goto out;
>>
>> sctx->t_whiteouts++;
>>
>> - } else if (is_dir(&rst) && !ovl_is_opaque(redirect_rpath)) {
>> - /* Found a directory but not opaqued, fix opaque xattr */
>> - if ((ret = ovl_set_opaque(redirect_rpath)))
>> - goto out;
>> + } else if (is_dir(&rst) && !ovl_is_opaque(cover_path) &&
>> + !ovl_is_redirect(cover_path)) {
>> +
>> + /*
>> + * Found a directory but not opaqued and not even
>> + * an another redirected directory, maybe created
>> + * when overlayfs is offline, ask user to add the
>> + * opaque xattr if this is not a merged dir.
>> + */
>> + if (!ovl_ask_question("Is merged dir", cover_path, 1))
>
> I am not sure what user will understand from this question,
> but maybe I don't see the full flow of info/questions in my mind.
> Anyway, if I am not mistaken, you seem to hold the opinion that
> default behavior if user says (-y := fix it I don't care how) should be
> to set the opaque xattr. If you agree, then you should ask:
> if (ovl_ask_question("Should set opaque dir", cover_path, 1))
>
Sorry, the behavior of '-y' and '-p' is not set opaque xattr, because I
think it's more likely that user create a new directory than simply remove
opaque xattr when overlay is offline for this case.
>> + if ((ret = ovl_set_opaque(cover_path)))
>> + goto out;
>> }
>>
>> goto out;
>> @@ -402,13 +434,25 @@ remove:
>> sctx->i_redirects++;
>>
>> /* Remove redirect xattr or ask user */
>> - if (!ovl_ask_invalid("Invalid redirect xattr", pathname, 1))
>> + if (!ovl_ask_action("Invalid redirect xattr", pathname, "Remove", 1))
>> goto out;
>> remove_d:
>> - ret = ovl_remove_redirect(pathname);
>> - if (!ret)
>> - sctx->i_redirects--;
>> + if ((ret = ovl_remove_redirect(pathname)))
>> + goto out;
>> +
>> + sctx->i_redirects--;
>> +
>> + /* If lower directory exist, ask user to add opaque xattr to cover it */
>> + if (start < lower_num) {
>> + if ((ret = ovl_check_lower(path_pick(pathname, sctx->dirlen),
>> + start, &chk, false)))
>> + goto out;
>> + if (chk.exist && is_dir(&chk.st) &&
>> + !ovl_ask_question("Is merged dir", pathname, 1))
>> + ret = ovl_set_opaque(pathname);
>
> Same here. Better that fsck -y will set opaque right?
> So flip the question to "Should set opaque dir"
>
Here is also not set opaque xattr in '-y' and '-p' mode.
I guess general user may not care about "invalid/duplicate redirect xattr"
when they modify underlying layers (eg: call "cp -a" or remove redirect origin
directory), but they probably know directories with the same name will merge
in overlayfs by default, and they should know the distribution of directories
after modification. So I prefer to merge directories. Anyway, neither will
affect consistency. :)
Thanks,
Yi.
>> + }
>> out:
>> + free(cover_path);
>> free(redirect);
>> return ret;
>> }
>> --
>> 2.9.5
>>
>
> .
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v3 4/6] fsck.overlay: correct redirect xattr check
2017-12-29 3:23 ` zhangyi (F)
@ 2017-12-29 9:32 ` Amir Goldstein
2017-12-29 10:25 ` zhangyi (F)
0 siblings, 1 reply; 18+ messages in thread
From: Amir Goldstein @ 2017-12-29 9:32 UTC (permalink / raw)
To: zhangyi (F)
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
>>> + goto out;
>>> + if (chk.exist && is_dir(&chk.st) &&
>>> + !ovl_ask_question("Is merged dir", pathname, 1))
>>> + ret = ovl_set_opaque(pathname);
>>
>> Same here. Better that fsck -y will set opaque right?
>> So flip the question to "Should set opaque dir"
>>
>
> Here is also not set opaque xattr in '-y' and '-p' mode.
> I guess general user may not care about "invalid/duplicate redirect xattr"
> when they modify underlying layers (eg: call "cp -a" or remove redirect origin
> directory), but they probably know directories with the same name will merge
> in overlayfs by default, and they should know the distribution of directories
> after modification. So I prefer to merge directories. Anyway, neither will
> affect consistency. :)
>
OK, but the question is still wrong, because answering No changes the
file systems and didn't fsck -n guaranty NOT to change the file system?
I think you probably need a more error prone interface for changing fs
that guaranties you cannot change fs when -n was specified.
The way I see it, the only way to resolve this is run several "pass"
like e2fsck.
The first pass just marks duplicate redirects
The second pass (with -y) sets redirect dirs opaque and removes redirect
The third pass will ask if to set merge dir opaque in case user answered No
for any of the redirect dirs in second pass.
Probably easier for you at this point is to flip the question and not
implement the above.
Cheers,
Amir.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v3 4/6] fsck.overlay: correct redirect xattr check
2017-12-29 9:32 ` Amir Goldstein
@ 2017-12-29 10:25 ` zhangyi (F)
0 siblings, 0 replies; 18+ messages in thread
From: zhangyi (F) @ 2017-12-29 10:25 UTC (permalink / raw)
To: Amir Goldstein
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On 2017/12/29 17:32, Amir Goldstein Wrote:
>>>> + goto out;
>>>> + if (chk.exist && is_dir(&chk.st) &&
>>>> + !ovl_ask_question("Is merged dir", pathname, 1))
>>>> + ret = ovl_set_opaque(pathname);
>>>
>>> Same here. Better that fsck -y will set opaque right?
>>> So flip the question to "Should set opaque dir"
>>>
>>
>> Here is also not set opaque xattr in '-y' and '-p' mode.
>> I guess general user may not care about "invalid/duplicate redirect xattr"
>> when they modify underlying layers (eg: call "cp -a" or remove redirect origin
>> directory), but they probably know directories with the same name will merge
>> in overlayfs by default, and they should know the distribution of directories
>> after modification. So I prefer to merge directories. Anyway, neither will
>> affect consistency. :)
>>
>
> OK, but the question is still wrong, because answering No changes the
> file systems and didn't fsck -n guaranty NOT to change the file system?
> I think you probably need a more error prone interface for changing fs
> that guaranties you cannot change fs when -n was specified.
>
> The way I see it, the only way to resolve this is run several "pass"
> like e2fsck.
> The first pass just marks duplicate redirects
> The second pass (with -y) sets redirect dirs opaque and removes redirect
> The third pass will ask if to set merge dir opaque in case user answered No
> for any of the redirect dirs in second pass.
>
> Probably easier for you at this point is to flip the question and not
> implement the above.
>
You are right, I miss that -n option will change the file system, thanks
for pointing this out. For simplicity now, I will flip the question as
you said(set opaque xattr in with -y option). For the first choice (run
several pass), we can realize it if necessary in the future.
Thanks,
Yi.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 5/6] fsck.overlay: fix lower target lookup
2017-12-28 11:40 [PATCH v3 0/6] overlay: implement fsck.overlay utility zhangyi (F)
` (3 preceding siblings ...)
2017-12-28 11:40 ` [PATCH v3 4/6] fsck.overlay: correct redirect xattr check zhangyi (F)
@ 2017-12-28 11:40 ` zhangyi (F)
2017-12-28 11:40 ` [PATCH v3 6/6] fsck.overlay: add impure xattr check zhangyi (F)
5 siblings, 0 replies; 18+ messages in thread
From: zhangyi (F) @ 2017-12-28 11:40 UTC (permalink / raw)
To: linux-unionfs, fstests
Cc: miklos, amir73il, eguan, yi.zhang, miaoxie, yangerkun
Current lower target lookup only check pathname in each lower layer
directly, not consider the following two cases:
1) Target's parent paths were covered by a file or an opaque directory
in a middle layer which is upper than origin target.
2) Target's parent paths contains a redirect directory point to another
lower directory, changing base directory is needed while scaning
each lower layers.
Therefore, we may find the wrong target or miss the correct one. This
patch add parent path checking when scaning target in a specific layer,
following redirect if we find a redirect directory and terminate if we
find a file/whiteout or opaque directory.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
check.c | 268 +++++++++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 207 insertions(+), 61 deletions(-)
diff --git a/check.c b/check.c
index 6db41cd..033d4ba 100644
--- a/check.c
+++ b/check.c
@@ -33,13 +33,25 @@
#include "lib.h"
#include "check.h"
-/* Underlying information */
-struct ovl_lower_check {
- unsigned int type; /* check extent type */
+/* Lookup context */
+struct ovl_lookup_ctx {
+ char *path; /* absolute path for lookup */
+ size_t dirlen; /* base directory length */
+ bool last; /* in last lower layer ? */
+ bool skip; /* skip self check */
+
+ char *redirect; /* redirect path for next lookup */
+ bool stop; /* stop lookup */
+
+ struct stat st; /* target's stat(2) */
+ bool exist; /* tatget exist or not */
+};
- bool exist;
- char path[PATH_MAX]; /* exist pathname found, only valid if exist */
- struct stat st; /* only valid if exist */
+/* Underlying target information */
+struct ovl_lower_data {
+ bool exist; /* tatget exist or not */
+ char path[PATH_MAX]; /* tatget's pathname */
+ struct stat st; /* target's stat(2) */
};
/* Redirect information */
@@ -177,38 +189,179 @@ static inline int ovl_ask_question(const char *question, const char *pathname,
return ask_question("", action);
}
+static int ovl_lookup_single(const char *path, struct stat *st, bool *exist)
+{
+ int ret = 0;
+
+ if (lstat(path, st) == 0) {
+ *exist = true;
+ } else {
+ if (errno != ENOENT && errno != ENOTDIR) {
+ print_err(_("Cannot stat %s: %s\n"), path,
+ strerror(errno));
+ ret = -1;
+ }
+ *exist = false;
+ }
+
+ return ret;
+}
+
+static int ovl_lookup_layer(struct ovl_lookup_ctx *lctx)
+{
+ char *path = sstrdup(lctx->path);
+ char *p = NULL, *base = NULL;
+ int ret = 0;
+
+ if (!lctx->skip) {
+ ret = ovl_lookup_single(path, &lctx->st, &lctx->exist);
+ if (ret)
+ goto out;
+ }
+
+ if (lctx->exist || lctx->last)
+ goto out;
+
+ /* Check if we should stop or redirect for the next layer's lookup */
+
+ /* Skip current target */
+ base = path + lctx->dirlen;
+ p = path + strlen(path);
+ for (; *p != '/' && p > base; p--)
+ *p = '\0';
+ *p = '\0';
+
+ /*
+ * Iterate to the first item in the path. If a redirect dir was found,
+ * change path for the next lookup. If an opaque directory or a file
+ * was found, stop lookup.
+ */
+ while (p > base) {
+ char *next = NULL, *redirect = NULL;
+ size_t name_len = 0;
+ bool exist = false;
+ struct stat st;
+
+ for (next = p; *next != '/'; next--) {
+ if (next < base)
+ goto out;
+ }
+
+ ret = ovl_lookup_single(path, &st, &exist);
+ if (ret)
+ goto out;
+ if (!exist)
+ goto next;
+
+ if (!is_dir(&st) || ovl_is_opaque(path)) {
+ lctx->stop = true;
+ goto out;
+ }
+
+ if (ovl_is_redirect(path)) {
+ name_len = p - (next + 1);
+ ret = ovl_get_redirect(path, lctx->dirlen, name_len,
+ &redirect);
+ if (ret)
+ goto out;
+
+ free(lctx->redirect);
+ lctx->redirect =
+ path_pack(NULL, 0, redirect,
+ path_pick(lctx->path, strlen(path)));
+ free(redirect);
+ goto out;
+ }
+next:
+ *next = '\0';
+ p = next;
+ }
+out:
+ free(path);
+ return ret;
+}
+
/*
- * Scan each lower dir lower than 'start' and check type matching,
- * we stop scan if we found something.
- *
- * skip: skip whiteout.
+ * Lookup the lower layers have the same target with the specific one or not.
*
+ * Scan each lower directory start from the layer of 'start' and lookup target
+ * until find something (skip target founded in start layer), Iterate parent
+ * directories and check directory type, following redirect directory and
+ * terminate scan if there is a file or an opaque directory exists.
*/
-static int ovl_check_lower(const char *pathname, int start,
- struct ovl_lower_check *chk, bool skip)
+static int ovl_lookup_lower(const char *pathname, int dirtype, int start,
+ struct ovl_lower_data *od)
{
- struct stat st;
+ struct ovl_lookup_ctx lctx = {0};
int i;
+ int ret = 0;
- for (i = start; i < lower_num; i++) {
- path_pack(chk->path, sizeof(chk->path), lowerdir[i], pathname);
+ if (dirtype == OVL_UPPER)
+ start = 0;
- if (lstat(chk->path, &st) != 0) {
- if (errno != ENOENT && errno != ENOTDIR) {
- print_err(_("Cannot stat %s: %s\n"),
- chk->path, strerror(errno));
- return -1;
- }
- continue;
- }
- if (skip && is_whiteout(&st))
- continue;
+ if (dirtype == OVL_UPPER) {
+ lctx.path = path_pack(NULL, 0, upperdir, pathname);
+ lctx.dirlen = strlen(upperdir);
+ lctx.skip = true;
- chk->exist = true;
- chk->st = st;
- break;
+ ret = ovl_lookup_layer(&lctx);
+ if (ret)
+ goto out;
}
- return 0;
+
+ for (i = start; !lctx.stop && !lctx.exist && i < lower_num; i++) {
+ lctx.path = path_pack(NULL, 0, lowerdir[i],
+ (lctx.redirect) ? lctx.redirect : pathname);
+ lctx.dirlen = strlen(lowerdir[i]);
+ lctx.skip = (dirtype == OVL_LOWER && i == start) ? true : false;
+ lctx.last = (i == lower_num - 1) ? true : false;
+
+ ret = ovl_lookup_layer(&lctx);
+ if (ret)
+ goto out;
+ }
+
+ od->exist = lctx.exist;
+ if (od->exist) {
+ strncpy(od->path, lctx.path, sizeof(od->path));
+ od->st = lctx.st;
+ }
+out:
+ free(lctx.redirect);
+ free(lctx.path);
+ return ret;
+}
+
+/*
+ * The same as ovl_lookup_lower() except start from the layer of 'start',
+ * not skip any target founded.
+ */
+static int ovl_lookup(const char *pathname, int start, struct ovl_lower_data *od)
+{
+ struct ovl_lookup_ctx lctx = {0};
+ int i;
+ int ret = 0;
+
+ for (i = start; !lctx.stop && !lctx.exist && i < lower_num; i++) {
+ lctx.path = path_pack(NULL, 0, lowerdir[i],
+ (lctx.redirect) ? lctx.redirect : pathname);
+ lctx.dirlen = strlen(lowerdir[i]);
+ lctx.last = (i == lower_num - 1) ? true : false;
+
+ ret = ovl_lookup_layer(&lctx);
+ if (ret)
+ goto out;
+ }
+
+ od->exist = lctx.exist;
+ if (od->exist) {
+ strncpy(od->path, lctx.path, sizeof(od->path));
+ od->st = lctx.st;
+ }
+out:
+ free(lctx.redirect);
+ free(lctx.path);
+ return ret;
}
/*
@@ -220,8 +373,7 @@ static int ovl_check_whiteout(struct scan_ctx *sctx)
{
const char *pathname = sctx->pathname;
const struct stat *st = sctx->st;
- struct ovl_lower_check chk = {0};
- int start;
+ struct ovl_lower_data od = {0};
int ret = 0;
/* Is a whiteout ? */
@@ -238,12 +390,11 @@ static int ovl_check_whiteout(struct scan_ctx *sctx)
* Scan each corresponding lower directroy under this layer,
* check is there a file or dir with the same name.
*/
- start = (sctx->dirtype == OVL_LOWER) ? sctx->num + 1 : 0;
- ret = ovl_check_lower(path_pick(pathname, sctx->dirlen),
- start, &chk, true);
+ ret = ovl_lookup_lower(path_pick(pathname, sctx->dirlen),
+ sctx->dirtype, sctx->num, &od);
if (ret)
return ret;
- if (chk.exist && !is_whiteout(&chk.st))
+ if (od.exist && !is_whiteout(&od.st))
goto out;
remove:
@@ -352,8 +503,9 @@ static void ovl_redirect_free(void)
static int ovl_check_redirect(struct scan_ctx *sctx)
{
const char *pathname = sctx->pathname;
- struct ovl_lower_check chk = {0};
- struct stat rst;
+ struct ovl_lower_data od = {0};
+ struct stat cover_st;
+ bool cover_exist = false;
char *redirect = NULL, *cover_path = NULL;
int start;
int ret = 0;
@@ -368,24 +520,22 @@ static int ovl_check_redirect(struct scan_ctx *sctx)
sctx->t_redirects++;
/* Redirect dir in last lower dir ? */
- if (sctx->dirtype == OVL_LOWER && sctx->num == lower_num-1) {
- start = lower_num;
+ if (sctx->dirtype == OVL_LOWER && sctx->num == lower_num-1)
goto remove;
- }
/* Scan lower directories to check redirect dir exist or not */
start = (sctx->dirtype == OVL_LOWER) ? sctx->num + 1 : 0;
- ret = ovl_check_lower(redirect, start, &chk, false);
+ ret = ovl_lookup(redirect, start, &od);
if (ret)
goto out;
- if (chk.exist && is_dir(&chk.st)) {
+ if (od.exist && is_dir(&od.st)) {
char *tmp;
/* Check duplicate in same layer */
- if (ovl_redirect_entry_find(chk.path, sctx->dirtype,
+ if (ovl_redirect_entry_find(od.path, sctx->dirtype,
sctx->num, &tmp)) {
print_info("Duplicate redirect directory found:%s\n", pathname);
- print_info("Origin:%s Previous:%s\n", chk.path, tmp);
+ print_info("Origin:%s Previous:%s\n", od.path, tmp);
sctx->i_redirects++;
@@ -396,24 +546,21 @@ static int ovl_check_redirect(struct scan_ctx *sctx)
}
ovl_redirect_entry_add(pathname, sctx->dirtype,
- sctx->num, chk.path);
+ sctx->num, od.path);
/* Check and fix whiteout or opaque dir */
cover_path = path_pack(cover_path, 0, sctx->dirname, redirect);
- if (lstat(cover_path, &rst) != 0) {
- if (errno != ENOENT && errno != ENOTDIR) {
- print_err(_("Cannot stat %s: %s\n"),
- cover_path, strerror(errno));
- goto out;
- }
-
+ ret = ovl_lookup_single(cover_path, &cover_st, &cover_exist);
+ if (ret)
+ goto out;
+ if (!cover_exist) {
/* Found nothing, create a whiteout */
if ((ret = ovl_create_whiteout(cover_path)))
goto out;
sctx->t_whiteouts++;
- } else if (is_dir(&rst) && !ovl_is_opaque(cover_path) &&
+ } else if (is_dir(&cover_st) && !ovl_is_opaque(cover_path) &&
!ovl_is_redirect(cover_path)) {
/*
@@ -443,14 +590,13 @@ remove_d:
sctx->i_redirects--;
/* If lower directory exist, ask user to add opaque xattr to cover it */
- if (start < lower_num) {
- if ((ret = ovl_check_lower(path_pick(pathname, sctx->dirlen),
- start, &chk, false)))
- goto out;
- if (chk.exist && is_dir(&chk.st) &&
- !ovl_ask_question("Is merged dir", pathname, 1))
- ret = ovl_set_opaque(pathname);
- }
+ if ((ret = ovl_lookup_lower(path_pick(pathname, sctx->dirlen),
+ sctx->dirtype, sctx->num, &od)))
+ goto out;
+ if (od.exist && is_dir(&od.st) &&
+ !ovl_ask_question("Is merged dir", pathname, 1))
+ ret = ovl_set_opaque(pathname);
+
out:
free(cover_path);
free(redirect);
--
2.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 6/6] fsck.overlay: add impure xattr check
2017-12-28 11:40 [PATCH v3 0/6] overlay: implement fsck.overlay utility zhangyi (F)
` (4 preceding siblings ...)
2017-12-28 11:40 ` [PATCH v3 5/6] fsck.overlay: fix lower target lookup zhangyi (F)
@ 2017-12-28 11:40 ` zhangyi (F)
2017-12-28 13:18 ` Amir Goldstein
5 siblings, 1 reply; 18+ messages in thread
From: zhangyi (F) @ 2017-12-28 11:40 UTC (permalink / raw)
To: linux-unionfs, fstests
Cc: miklos, amir73il, eguan, yi.zhang, miaoxie, yangerkun
An impure directory is a directory with "trusted.overlay.impure" xattr
valued 'y', which indicate that this directory may contain copy-uped
targets from lower layers. In oredr to prevent 'd_ino' change while
copy-up (it create a new inode in upper layer) in getdents(2), impure
xattr will be set in the parent directory, letting overlay filesystem
check and get d_ino from lower origin target to ensure consistent d_ino.
There are three situations of setting impure xattr:
1) Copyup lower target in a directory.
2) Link an origined target (already copy-uped, have origin xattr) into
a directory.
3) Rename an origined target (include merged subdirectories) into a
new directory.
So, if a direcotry which contains several origined targets or redirect
directories, the impure xattr should be set. If not, fix this xattr.
Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
---
README.md | 22 +++++++++++++++-
check.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
lib.c | 63 +++++++++++++++++++++++++++++++++++-----------
lib.h | 25 +++++++++++++------
4 files changed, 168 insertions(+), 28 deletions(-)
diff --git a/README.md b/README.md
index 2e6146f..cdbd382 100644
--- a/README.md
+++ b/README.md
@@ -47,6 +47,26 @@ If an invalid redirect xattr was removed by fsck and there is a corresponding
lower directory with the same name exists, not sure this directory is merged
or not, ask user to make a decision.
+Impure directories
+------------------
+
+An impure directory is a directory with "trusted.overlay.impure" xattr valued
+'y', which indicate that this directory may contain copy-uped targets from lower
+layers. In oredr to prevent 'd_ino' change while copy-up (it create a new
+inode in upper layer) in getdents (see getdents(2)), impure xattr will be set
+in the parent directory, letting overlay filesystem check and get d_ino from
+lower origin target to ensure consistent d_ino.
+
+There are three situations of setting impure xattr:
+1) Copyup lower target in a directory.
+2) Link an origined target (already copy-uped, have origin xattr) into
+ a directory.
+3) Rename an origined target (include merged subdirectories) into a
+ new directory.
+
+So, if a direcotry which contains several origined targets or redirect
+directories, the impure xattr should be set. If not, fix this xattr.
+
Usage
=====
@@ -88,6 +108,6 @@ Todo
mounted with relative path. Should modify kernel together to support.
2. Check and fix invalid redirect xattr through origin xattr.
3. Symbolic link check.
-4. Check origin/impure/nlink xattr.
+4. Check origin/nlink xattr.
5. Check index feature consistency.
6. ...
diff --git a/check.c b/check.c
index 033d4ba..05293c5 100644
--- a/check.c
+++ b/check.c
@@ -111,9 +111,9 @@ static inline bool ovl_is_opaque(const char *pathname)
return is_dir_xattr(pathname, OVL_OPAQUE_XATTR);
}
-static inline int ovl_remove_opaque(const char *pathname)
+static inline bool ovl_is_impure(const char *pathname)
{
- return remove_xattr(pathname, OVL_OPAQUE_XATTR);
+ return is_dir_xattr(pathname, OVL_IMPURE_XATTR);
}
static inline int ovl_set_opaque(const char *pathname)
@@ -121,6 +121,11 @@ static inline int ovl_set_opaque(const char *pathname)
return set_xattr(pathname, OVL_OPAQUE_XATTR, "y", 1);
}
+static inline int ovl_set_impure(const char *pathname)
+{
+ return set_xattr(pathname, OVL_IMPURE_XATTR, "y", 1);
+}
+
static int ovl_get_redirect(const char *pathname, size_t dirlen,
size_t filelen, char **redirect)
{
@@ -166,11 +171,20 @@ static inline int ovl_create_whiteout(const char *pathname)
static inline bool ovl_is_redirect(const char *pathname)
{
- bool exist = false;
- ssize_t ret;
+ bool exist = false;
+ ssize_t ret;
+
+ ret = get_xattr(pathname, OVL_REDIRECT_XATTR, NULL, &exist);
+ return ret ? false : exist;
+}
+
+static inline bool ovl_is_origin(const char *pathname)
+{
+ bool exist = false;
+ ssize_t ret;
- ret = get_xattr(pathname, OVL_REDIRECT_XATTR, NULL, &exist);
- return ret ? false : exist;
+ ret = get_xattr(pathname, OVL_ORIGIN_XATTR, NULL, &exist);
+ return ret ? false : exist;
}
static inline int ovl_ask_action(const char *description, const char *pathname,
@@ -503,6 +517,7 @@ static void ovl_redirect_free(void)
static int ovl_check_redirect(struct scan_ctx *sctx)
{
const char *pathname = sctx->pathname;
+ struct scan_dir_data *parent_ddata = sctx->ddata;
struct ovl_lower_data od = {0};
struct stat cover_st;
bool cover_exist = false;
@@ -545,6 +560,9 @@ static int ovl_check_redirect(struct scan_ctx *sctx)
goto remove_d;
}
+ /* Now, this redirect xattr is valid */
+ if (parent_ddata)
+ parent_ddata->redirects++;
ovl_redirect_entry_add(pathname, sctx->dirtype,
sctx->num, od.path);
@@ -603,9 +621,65 @@ out:
return ret;
}
+/*
+ * Get origin xattr stored in the target. We cannot check validity for
+ * current overlay filesystem, so trust this is valid now.
+ */
+static int ovl_check_origin(struct scan_ctx *sctx)
+{
+ const char *pathname = sctx->pathname;
+ struct scan_dir_data *parent_ddata = sctx->ddata;
+
+ if (!ovl_is_origin(pathname))
+ return 0;
+ if (parent_ddata)
+ parent_ddata->origins++;
+
+ return 0;
+}
+
+/*
+ * If a directory has origined target and redirect subdirectories in it,
+ * it may contain copy-up targets. In order to avoid 'd_ino' change after
+ * lower target copyup or renameed (which will create a new inode),
+ * 'impure xattr' will be setted in the parent directory to indicate
+ * overlayfs show origin 'd_ino' when iterate directory.
+ *
+ * Missing "impure xattr" will cause wrong 'd_ino' in readdir() of impure
+ * directory, so check origined or redirected target in a directory and
+ * fix "impure xattr" if necessary.
+ */
+static int ovl_check_impure(struct scan_ctx *sctx)
+{
+ const char *pathname = sctx->pathname;
+ struct scan_dir_data *cur_ddata = sctx->ddata;
+ int ret = 0;
+
+ /*
+ * Impure xattr should be set if current directory has redirect
+ * directory or origin targets.
+ */
+ if (!cur_ddata->origins && !cur_ddata->redirects)
+ goto out;
+
+ /* Fix impure xattrs */
+ if (!ovl_is_impure(pathname)) {
+ if (ovl_ask_action("Missing impure xattr",
+ pathname, "Fix", 1)) {
+ ret = ovl_set_impure(pathname);
+ if (ret)
+ goto out;
+ }
+ }
+out:
+ return ret;
+}
+
static struct scan_operations ovl_scan_ops = {
.whiteout = ovl_check_whiteout,
.redirect = ovl_check_redirect,
+ .origin = ovl_check_origin,
+ .impure = ovl_check_impure,
};
static void ovl_scan_report(struct scan_ctx *sctx)
diff --git a/lib.c b/lib.c
index 497d357..524246d 100644
--- a/lib.c
+++ b/lib.c
@@ -165,8 +165,17 @@ const char *path_pick(const char *path, int baselen)
return (*p == '/') ? p+1 : p;
}
-static inline int __check_entry(struct scan_ctx *sctx,
- int (*do_check)(struct scan_ctx *))
+static void scan_entry_init(struct scan_ctx *sctx, FTSENT *ftsent)
+{
+ sctx->pathname = ftsent->fts_path;
+ sctx->pathlen = ftsent->fts_pathlen;
+ sctx->filename = ftsent->fts_name;
+ sctx->filelen = ftsent->fts_namelen;
+ sctx->st = ftsent->fts_statp;
+}
+
+static inline int scan_check_entry(int (*do_check)(struct scan_ctx *),
+ struct scan_ctx *sctx)
{
return do_check ? do_check(sctx) : 0;
}
@@ -187,8 +196,6 @@ int scan_dir(struct scan_ctx *sctx, struct scan_operations *sop)
}
while ((ftsent = fts_read(ftsp)) != NULL) {
- int err;
-
print_debug(_("Scan:%-3s %2d %7jd %-40s %s\n"),
(ftsent->fts_info == FTS_D) ? "d" :
(ftsent->fts_info == FTS_DNR) ? "dnr" :
@@ -202,26 +209,54 @@ int scan_dir(struct scan_ctx *sctx, struct scan_operations *sop)
ftsent->fts_path, ftsent->fts_name);
/* Fillup base context */
- sctx->pathname = ftsent->fts_path;
- sctx->pathlen = ftsent->fts_pathlen;
- sctx->filename = ftsent->fts_name;
- sctx->filelen = ftsent->fts_namelen;
- sctx->st = ftsent->fts_statp;
+ scan_entry_init(sctx, ftsent);
switch (ftsent->fts_info) {
case FTS_F:
sctx->files++;
+
+ /* Check origin */
+ ret = scan_check_entry(sop->origin, sctx);
+ if (ret)
+ goto out;
break;
case FTS_DEFAULT:
/* Check whiteouts */
- err = __check_entry(sctx, sop->whiteout);
- ret = (err) ? err : ret;
+ ret = scan_check_entry(sop->whiteout, sctx);
+ if (ret)
+ goto out;
+
+ /* Check origin */
+ ret = scan_check_entry(sop->origin, sctx);
+ if (ret)
+ goto out;
break;
case FTS_D:
- /* Check redirect xattr */
sctx->directories++;
- err = __check_entry(sctx, sop->redirect);
- ret = (err) ? err : ret;
+
+ /* Check redirect xattr */
+ ret = scan_check_entry(sop->redirect, sctx);
+ if (ret)
+ goto out;
+
+ /* Check origin */
+ ret = scan_check_entry(sop->origin, sctx);
+ if (ret)
+ goto out;
+
+ /* Save current dir data and create new one */
+ ftsent->fts_pointer = sctx->ddata;
+ sctx->ddata = smalloc(sizeof(struct scan_dir_data));
+ break;
+ case FTS_DP:
+ /* Check impure xattr */
+ ret = scan_check_entry(sop->impure, sctx);
+ if (ret)
+ goto out;
+
+ /* Restore parent's dir data */
+ free(sctx->ddata);
+ sctx->ddata = ftsent->fts_pointer;
break;
case FTS_NS:
case FTS_DNR:
diff --git a/lib.h b/lib.h
index 5d94836..5fec773 100644
--- a/lib.h
+++ b/lib.h
@@ -52,33 +52,44 @@
/* Xattr */
#define OVL_OPAQUE_XATTR "trusted.overlay.opaque"
#define OVL_REDIRECT_XATTR "trusted.overlay.redirect"
+#define OVL_ORIGIN_XATTR "trusted.overlay.origin"
+#define OVL_IMPURE_XATTR "trusted.overlay.impure"
-/* Directories scan data struct */
+/* Directories scan data structs */
+struct scan_dir_data {
+ int origins; /* origin number in this directory (no iterate) */
+ int redirects; /* redirect number in this directory (no iterate) */
+};
+
struct scan_ctx {
const char *dirname; /* upper/lower root dir */
size_t dirlen; /* strlen(dirlen) */
int dirtype; /* OVL_UPPER or OVL_LOWER */
int num; /* lower dir depth, lower type use only */
+ int files; /* total files */
+ int directories; /* total directories */
+ int t_whiteouts; /* total whiteouts */
+ int i_whiteouts; /* invalid whiteouts */
+ int t_redirects; /* total redirect dirs */
+ int i_redirects; /* invalid redirect dirs */
+
const char *pathname; /* file path from the root */
size_t pathlen; /* strlen(pathname) */
const char *filename; /* filename */
size_t filelen; /* strlen(filename) */
struct stat *st; /* file stat */
- int files;
- int directories;
- int t_whiteouts; /* total whiteouts */
- int i_whiteouts; /* invalid whiteouts */
- int t_redirects; /* total redirect dirs */
- int i_redirects; /* invalid redirect dirs */
+ struct scan_dir_data *ddata; /* parent dir data of current target */
};
/* Directories scan callback operations struct */
struct scan_operations {
int (*whiteout)(struct scan_ctx *);
int (*redirect)(struct scan_ctx *);
+ int (*origin)(struct scan_ctx *);
+ int (*impure)(struct scan_ctx *);
};
static inline void set_st_inconsistency(int *st)
--
2.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 6/6] fsck.overlay: add impure xattr check
2017-12-28 11:40 ` [PATCH v3 6/6] fsck.overlay: add impure xattr check zhangyi (F)
@ 2017-12-28 13:18 ` Amir Goldstein
2017-12-29 2:21 ` zhangyi (F)
0 siblings, 1 reply; 18+ messages in thread
From: Amir Goldstein @ 2017-12-28 13:18 UTC (permalink / raw)
To: zhangyi (F)
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On Thu, Dec 28, 2017 at 1:40 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> An impure directory is a directory with "trusted.overlay.impure" xattr
> valued 'y', which indicate that this directory may contain copy-uped
> targets from lower layers. In oredr to prevent 'd_ino' change while
> copy-up (it create a new inode in upper layer) in getdents(2), impure
> xattr will be set in the parent directory, letting overlay filesystem
> check and get d_ino from lower origin target to ensure consistent d_ino.
>
> There are three situations of setting impure xattr:
> 1) Copyup lower target in a directory.
> 2) Link an origined target (already copy-uped, have origin xattr) into
> a directory.
> 3) Rename an origined target (include merged subdirectories) into a
> new directory.
>
> So, if a direcotry which contains several origined targets or redirect
> directories, the impure xattr should be set. If not, fix this xattr.
>
> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
> ---
> README.md | 22 +++++++++++++++-
> check.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> lib.c | 63 +++++++++++++++++++++++++++++++++++-----------
> lib.h | 25 +++++++++++++------
> 4 files changed, 168 insertions(+), 28 deletions(-)
>
> diff --git a/README.md b/README.md
> index 2e6146f..cdbd382 100644
> --- a/README.md
> +++ b/README.md
> @@ -47,6 +47,26 @@ If an invalid redirect xattr was removed by fsck and there is a corresponding
> lower directory with the same name exists, not sure this directory is merged
> or not, ask user to make a decision.
>
> +Impure directories
> +------------------
> +
> +An impure directory is a directory with "trusted.overlay.impure" xattr valued
> +'y', which indicate that this directory may contain copy-uped targets from lower
"copied up"
> +layers. In oredr to prevent 'd_ino' change while copy-up (it create a new
typo: oredr/order
> +inode in upper layer) in getdents (see getdents(2)), impure xattr will be set
> +in the parent directory, letting overlay filesystem check and get d_ino from
> +lower origin target to ensure consistent d_ino.
> +
This paragraph could use a review from a native English speaker (not me)
> +There are three situations of setting impure xattr:
> +1) Copyup lower target in a directory.
I guess the only instance of "copyup" in comment in my blame.
Either "Copy up" or "Copy-up" would be better and the former is more
common in comments and documentations.
> +2) Link an origined target (already copy-uped, have origin xattr) into
"copied up"
> + a directory.
> +3) Rename an origined target (include merged subdirectories) into a
> + new directory.
> +
> +So, if a direcotry which contains several origined targets or redirect
> +directories, the impure xattr should be set. If not, fix this xattr.
> +
> Usage
> =====
>
> @@ -88,6 +108,6 @@ Todo
> mounted with relative path. Should modify kernel together to support.
> 2. Check and fix invalid redirect xattr through origin xattr.
> 3. Symbolic link check.
> -4. Check origin/impure/nlink xattr.
> +4. Check origin/nlink xattr.
> 5. Check index feature consistency.
> 6. ...
> diff --git a/check.c b/check.c
> index 033d4ba..05293c5 100644
> --- a/check.c
> +++ b/check.c
> @@ -111,9 +111,9 @@ static inline bool ovl_is_opaque(const char *pathname)
> return is_dir_xattr(pathname, OVL_OPAQUE_XATTR);
> }
>
> -static inline int ovl_remove_opaque(const char *pathname)
> +static inline bool ovl_is_impure(const char *pathname)
> {
> - return remove_xattr(pathname, OVL_OPAQUE_XATTR);
> + return is_dir_xattr(pathname, OVL_IMPURE_XATTR);
> }
>
> static inline int ovl_set_opaque(const char *pathname)
> @@ -121,6 +121,11 @@ static inline int ovl_set_opaque(const char *pathname)
> return set_xattr(pathname, OVL_OPAQUE_XATTR, "y", 1);
> }
>
> +static inline int ovl_set_impure(const char *pathname)
> +{
> + return set_xattr(pathname, OVL_IMPURE_XATTR, "y", 1);
> +}
> +
> static int ovl_get_redirect(const char *pathname, size_t dirlen,
> size_t filelen, char **redirect)
> {
> @@ -166,11 +171,20 @@ static inline int ovl_create_whiteout(const char *pathname)
>
> static inline bool ovl_is_redirect(const char *pathname)
> {
> - bool exist = false;
> - ssize_t ret;
> + bool exist = false;
> + ssize_t ret;
> +
> + ret = get_xattr(pathname, OVL_REDIRECT_XATTR, NULL, &exist);
> + return ret ? false : exist;
> +}
> +
> +static inline bool ovl_is_origin(const char *pathname)
> +{
> + bool exist = false;
> + ssize_t ret;
>
That's a strange diff... I guess it is a hint from git diff that you
could factor out
ovl_is_dir_xattr_exists()
> - ret = get_xattr(pathname, OVL_REDIRECT_XATTR, NULL, &exist);
> - return ret ? false : exist;
> + ret = get_xattr(pathname, OVL_ORIGIN_XATTR, NULL, &exist);
> + return ret ? false : exist;
> }
>
> static inline int ovl_ask_action(const char *description, const char *pathname,
> @@ -503,6 +517,7 @@ static void ovl_redirect_free(void)
> static int ovl_check_redirect(struct scan_ctx *sctx)
> {
> const char *pathname = sctx->pathname;
> + struct scan_dir_data *parent_ddata = sctx->ddata;
> struct ovl_lower_data od = {0};
> struct stat cover_st;
> bool cover_exist = false;
> @@ -545,6 +560,9 @@ static int ovl_check_redirect(struct scan_ctx *sctx)
> goto remove_d;
> }
>
> + /* Now, this redirect xattr is valid */
> + if (parent_ddata)
> + parent_ddata->redirects++;
> ovl_redirect_entry_add(pathname, sctx->dirtype,
> sctx->num, od.path);
>
> @@ -603,9 +621,65 @@ out:
> return ret;
> }
>
> +/*
> + * Get origin xattr stored in the target. We cannot check validity for
> + * current overlay filesystem, so trust this is valid now.
> + */
> +static int ovl_check_origin(struct scan_ctx *sctx)
> +{
> + const char *pathname = sctx->pathname;
> + struct scan_dir_data *parent_ddata = sctx->ddata;
> +
> + if (!ovl_is_origin(pathname))
> + return 0;
> + if (parent_ddata)
> + parent_ddata->origins++;
> +
> + return 0;
> +}
> +
> +/*
> + * If a directory has origined target and redirect subdirectories in it,
> + * it may contain copy-up targets. In order to avoid 'd_ino' change after
> + * lower target copyup or renameed (which will create a new inode),
typo renameed/renamed
> + * 'impure xattr' will be setted in the parent directory to indicate
"will be set"
> + * overlayfs show origin 'd_ino' when iterate directory.
> + *
> + * Missing "impure xattr" will cause wrong 'd_ino' in readdir() of impure
> + * directory, so check origined or redirected target in a directory and
> + * fix "impure xattr" if necessary.
> + */
> +static int ovl_check_impure(struct scan_ctx *sctx)
> +{
> + const char *pathname = sctx->pathname;
> + struct scan_dir_data *cur_ddata = sctx->ddata;
> + int ret = 0;
> +
> + /*
> + * Impure xattr should be set if current directory has redirect
> + * directory or origin targets.
> + */
> + if (!cur_ddata->origins && !cur_ddata->redirects)
> + goto out;
> +
> + /* Fix impure xattrs */
> + if (!ovl_is_impure(pathname)) {
> + if (ovl_ask_action("Missing impure xattr",
> + pathname, "Fix", 1)) {
> + ret = ovl_set_impure(pathname);
> + if (ret)
> + goto out;
> + }
> + }
> +out:
> + return ret;
> +}
> +
> static struct scan_operations ovl_scan_ops = {
> .whiteout = ovl_check_whiteout,
> .redirect = ovl_check_redirect,
> + .origin = ovl_check_origin,
> + .impure = ovl_check_impure,
> };
>
> static void ovl_scan_report(struct scan_ctx *sctx)
> diff --git a/lib.c b/lib.c
> index 497d357..524246d 100644
> --- a/lib.c
> +++ b/lib.c
> @@ -165,8 +165,17 @@ const char *path_pick(const char *path, int baselen)
> return (*p == '/') ? p+1 : p;
> }
>
> -static inline int __check_entry(struct scan_ctx *sctx,
> - int (*do_check)(struct scan_ctx *))
> +static void scan_entry_init(struct scan_ctx *sctx, FTSENT *ftsent)
> +{
> + sctx->pathname = ftsent->fts_path;
> + sctx->pathlen = ftsent->fts_pathlen;
> + sctx->filename = ftsent->fts_name;
> + sctx->filelen = ftsent->fts_namelen;
> + sctx->st = ftsent->fts_statp;
> +}
> +
> +static inline int scan_check_entry(int (*do_check)(struct scan_ctx *),
> + struct scan_ctx *sctx)
> {
> return do_check ? do_check(sctx) : 0;
> }
> @@ -187,8 +196,6 @@ int scan_dir(struct scan_ctx *sctx, struct scan_operations *sop)
> }
>
> while ((ftsent = fts_read(ftsp)) != NULL) {
> - int err;
> -
> print_debug(_("Scan:%-3s %2d %7jd %-40s %s\n"),
> (ftsent->fts_info == FTS_D) ? "d" :
> (ftsent->fts_info == FTS_DNR) ? "dnr" :
> @@ -202,26 +209,54 @@ int scan_dir(struct scan_ctx *sctx, struct scan_operations *sop)
> ftsent->fts_path, ftsent->fts_name);
>
> /* Fillup base context */
> - sctx->pathname = ftsent->fts_path;
> - sctx->pathlen = ftsent->fts_pathlen;
> - sctx->filename = ftsent->fts_name;
> - sctx->filelen = ftsent->fts_namelen;
> - sctx->st = ftsent->fts_statp;
> + scan_entry_init(sctx, ftsent);
>
> switch (ftsent->fts_info) {
> case FTS_F:
> sctx->files++;
> +
> + /* Check origin */
> + ret = scan_check_entry(sop->origin, sctx);
> + if (ret)
> + goto out;
> break;
> case FTS_DEFAULT:
> /* Check whiteouts */
> - err = __check_entry(sctx, sop->whiteout);
> - ret = (err) ? err : ret;
> + ret = scan_check_entry(sop->whiteout, sctx);
> + if (ret)
> + goto out;
> +
> + /* Check origin */
> + ret = scan_check_entry(sop->origin, sctx);
> + if (ret)
> + goto out;
All the re-factoring of scan helpers and this additional origin check
do not seem relevant to this change's subject (impure dir check).
> break;
> case FTS_D:
> - /* Check redirect xattr */
> sctx->directories++;
> - err = __check_entry(sctx, sop->redirect);
> - ret = (err) ? err : ret;
> +
> + /* Check redirect xattr */
> + ret = scan_check_entry(sop->redirect, sctx);
> + if (ret)
> + goto out;
> +
> + /* Check origin */
> + ret = scan_check_entry(sop->origin, sctx);
> + if (ret)
> + goto out;
> +
> + /* Save current dir data and create new one */
> + ftsent->fts_pointer = sctx->ddata;
> + sctx->ddata = smalloc(sizeof(struct scan_dir_data));
> + break;
> + case FTS_DP:
> + /* Check impure xattr */
> + ret = scan_check_entry(sop->impure, sctx);
> + if (ret)
> + goto out;
> +
> + /* Restore parent's dir data */
> + free(sctx->ddata);
> + sctx->ddata = ftsent->fts_pointer;
> break;
> case FTS_NS:
> case FTS_DNR:
> diff --git a/lib.h b/lib.h
> index 5d94836..5fec773 100644
> --- a/lib.h
> +++ b/lib.h
> @@ -52,33 +52,44 @@
> /* Xattr */
> #define OVL_OPAQUE_XATTR "trusted.overlay.opaque"
> #define OVL_REDIRECT_XATTR "trusted.overlay.redirect"
> +#define OVL_ORIGIN_XATTR "trusted.overlay.origin"
> +#define OVL_IMPURE_XATTR "trusted.overlay.impure"
>
>
> -/* Directories scan data struct */
> +/* Directories scan data structs */
> +struct scan_dir_data {
> + int origins; /* origin number in this directory (no iterate) */
> + int redirects; /* redirect number in this directory (no iterate) */
> +};
> +
> struct scan_ctx {
> const char *dirname; /* upper/lower root dir */
> size_t dirlen; /* strlen(dirlen) */
> int dirtype; /* OVL_UPPER or OVL_LOWER */
> int num; /* lower dir depth, lower type use only */
>
> + int files; /* total files */
> + int directories; /* total directories */
> + int t_whiteouts; /* total whiteouts */
> + int i_whiteouts; /* invalid whiteouts */
> + int t_redirects; /* total redirect dirs */
> + int i_redirects; /* invalid redirect dirs */
> +
> const char *pathname; /* file path from the root */
> size_t pathlen; /* strlen(pathname) */
> const char *filename; /* filename */
> size_t filelen; /* strlen(filename) */
> struct stat *st; /* file stat */
>
> - int files;
> - int directories;
> - int t_whiteouts; /* total whiteouts */
> - int i_whiteouts; /* invalid whiteouts */
> - int t_redirects; /* total redirect dirs */
> - int i_redirects; /* invalid redirect dirs */
> + struct scan_dir_data *ddata; /* parent dir data of current target */
> };
>
> /* Directories scan callback operations struct */
> struct scan_operations {
> int (*whiteout)(struct scan_ctx *);
> int (*redirect)(struct scan_ctx *);
> + int (*origin)(struct scan_ctx *);
> + int (*impure)(struct scan_ctx *);
> };
>
> static inline void set_st_inconsistency(int *st)
> --
> 2.9.5
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v3 6/6] fsck.overlay: add impure xattr check
2017-12-28 13:18 ` Amir Goldstein
@ 2017-12-29 2:21 ` zhangyi (F)
2017-12-29 7:03 ` Amir Goldstein
0 siblings, 1 reply; 18+ messages in thread
From: zhangyi (F) @ 2017-12-29 2:21 UTC (permalink / raw)
To: Amir Goldstein
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On 2017/12/28 21:18, Amir Goldstein Wrote:
> On Thu, Dec 28, 2017 at 1:40 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> An impure directory is a directory with "trusted.overlay.impure" xattr
>> valued 'y', which indicate that this directory may contain copy-uped
>> targets from lower layers. In oredr to prevent 'd_ino' change while
>> copy-up (it create a new inode in upper layer) in getdents(2), impure
>> xattr will be set in the parent directory, letting overlay filesystem
>> check and get d_ino from lower origin target to ensure consistent d_ino.
>>
>> There are three situations of setting impure xattr:
>> 1) Copyup lower target in a directory.
>> 2) Link an origined target (already copy-uped, have origin xattr) into
>> a directory.
>> 3) Rename an origined target (include merged subdirectories) into a
>> new directory.
>>
>> So, if a direcotry which contains several origined targets or redirect
>> directories, the impure xattr should be set. If not, fix this xattr.
>>
>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>> ---
>> README.md | 22 +++++++++++++++-
>> check.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>> lib.c | 63 +++++++++++++++++++++++++++++++++++-----------
>> lib.h | 25 +++++++++++++------
>> 4 files changed, 168 insertions(+), 28 deletions(-)
>>
>> diff --git a/README.md b/README.md
>> index 2e6146f..cdbd382 100644
>> --- a/README.md
>> +++ b/README.md
>> @@ -47,6 +47,26 @@ If an invalid redirect xattr was removed by fsck and there is a corresponding
>> lower directory with the same name exists, not sure this directory is merged
>> or not, ask user to make a decision.
>>
>> +Impure directories
>> +------------------
>> +
>> +An impure directory is a directory with "trusted.overlay.impure" xattr valued
>> +'y', which indicate that this directory may contain copy-uped targets from lower
>
> "copied up"
>
>> +layers. In oredr to prevent 'd_ino' change while copy-up (it create a new
>
> typo: oredr/order
>
>> +inode in upper layer) in getdents (see getdents(2)), impure xattr will be set
>> +in the parent directory, letting overlay filesystem check and get d_ino from
>> +lower origin target to ensure consistent d_ino.
>> +
>
> This paragraph could use a review from a native English speaker (not me)
>
Sorry, my fault, will reorganize this document and fix typo.
>> +There are three situations of setting impure xattr:
>> +1) Copyup lower target in a directory.
>
> I guess the only instance of "copyup" in comment in my blame.
> Either "Copy up" or "Copy-up" would be better and the former is more
> common in comments and documentations.
>
>> +2) Link an origined target (already copy-uped, have origin xattr) into
>
> "copied up"
>
>> + a directory.
>> +3) Rename an origined target (include merged subdirectories) into a
>> + new directory.
>> +
>> +So, if a direcotry which contains several origined targets or redirect
>> +directories, the impure xattr should be set. If not, fix this xattr.
>> +
>> Usage
>> =====
>>
>> @@ -88,6 +108,6 @@ Todo
>> mounted with relative path. Should modify kernel together to support.
>> 2. Check and fix invalid redirect xattr through origin xattr.
>> 3. Symbolic link check.
>> -4. Check origin/impure/nlink xattr.
>> +4. Check origin/nlink xattr.
>> 5. Check index feature consistency.
>> 6. ...
>> diff --git a/check.c b/check.c
>> index 033d4ba..05293c5 100644
>> --- a/check.c
>> +++ b/check.c
>> @@ -111,9 +111,9 @@ static inline bool ovl_is_opaque(const char *pathname)
>> return is_dir_xattr(pathname, OVL_OPAQUE_XATTR);
>> }
>>
>> -static inline int ovl_remove_opaque(const char *pathname)
>> +static inline bool ovl_is_impure(const char *pathname)
>> {
>> - return remove_xattr(pathname, OVL_OPAQUE_XATTR);
>> + return is_dir_xattr(pathname, OVL_IMPURE_XATTR);
>> }
>>
>> static inline int ovl_set_opaque(const char *pathname)
>> @@ -121,6 +121,11 @@ static inline int ovl_set_opaque(const char *pathname)
>> return set_xattr(pathname, OVL_OPAQUE_XATTR, "y", 1);
>> }
>>
>> +static inline int ovl_set_impure(const char *pathname)
>> +{
>> + return set_xattr(pathname, OVL_IMPURE_XATTR, "y", 1);
>> +}
>> +
>> static int ovl_get_redirect(const char *pathname, size_t dirlen,
>> size_t filelen, char **redirect)
>> {
>> @@ -166,11 +171,20 @@ static inline int ovl_create_whiteout(const char *pathname)
>>
>> static inline bool ovl_is_redirect(const char *pathname)
>> {
>> - bool exist = false;
>> - ssize_t ret;
>> + bool exist = false;
>> + ssize_t ret;
>> +
>> + ret = get_xattr(pathname, OVL_REDIRECT_XATTR, NULL, &exist);
>> + return ret ? false : exist;
>> +}
>> +
>> +static inline bool ovl_is_origin(const char *pathname)
>> +{
>> + bool exist = false;
>> + ssize_t ret;
>>
>
> That's a strange diff... I guess it is a hint from git diff that you
> could factor out
> ovl_is_dir_xattr_exists()
>
This is because I change blank to tab in ovl_is_redirect(), factor out
ovl_is_dir_xattr_exists() is also fine.
>> - ret = get_xattr(pathname, OVL_REDIRECT_XATTR, NULL, &exist);
>> - return ret ? false : exist;
>> + ret = get_xattr(pathname, OVL_ORIGIN_XATTR, NULL, &exist);
>> + return ret ? false : exist;
>> }
>>
>> static inline int ovl_ask_action(const char *description, const char *pathname,
>> @@ -503,6 +517,7 @@ static void ovl_redirect_free(void)
>> static int ovl_check_redirect(struct scan_ctx *sctx)
>> {
>> const char *pathname = sctx->pathname;
>> + struct scan_dir_data *parent_ddata = sctx->ddata;
>> struct ovl_lower_data od = {0};
>> struct stat cover_st;
>> bool cover_exist = false;
>> @@ -545,6 +560,9 @@ static int ovl_check_redirect(struct scan_ctx *sctx)
>> goto remove_d;
>> }
>>
>> + /* Now, this redirect xattr is valid */
>> + if (parent_ddata)
>> + parent_ddata->redirects++;
>> ovl_redirect_entry_add(pathname, sctx->dirtype,
>> sctx->num, od.path);
>>
>> @@ -603,9 +621,65 @@ out:
>> return ret;
>> }
>>
>> +/*
>> + * Get origin xattr stored in the target. We cannot check validity for
>> + * current overlay filesystem, so trust this is valid now.
>> + */
>> +static int ovl_check_origin(struct scan_ctx *sctx)
>> +{
>> + const char *pathname = sctx->pathname;
>> + struct scan_dir_data *parent_ddata = sctx->ddata;
>> +
>> + if (!ovl_is_origin(pathname))
>> + return 0;
>> + if (parent_ddata)
>> + parent_ddata->origins++;
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * If a directory has origined target and redirect subdirectories in it,
>> + * it may contain copy-up targets. In order to avoid 'd_ino' change after
>> + * lower target copyup or renameed (which will create a new inode),
>
> typo renameed/renamed
>
>> + * 'impure xattr' will be setted in the parent directory to indicate
>
> "will be set"
>
>> + * overlayfs show origin 'd_ino' when iterate directory.
>> + *
>> + * Missing "impure xattr" will cause wrong 'd_ino' in readdir() of impure
>> + * directory, so check origined or redirected target in a directory and
>> + * fix "impure xattr" if necessary.
>> + */
>> +static int ovl_check_impure(struct scan_ctx *sctx)
>> +{
>> + const char *pathname = sctx->pathname;
>> + struct scan_dir_data *cur_ddata = sctx->ddata;
>> + int ret = 0;
>> +
>> + /*
>> + * Impure xattr should be set if current directory has redirect
>> + * directory or origin targets.
>> + */
>> + if (!cur_ddata->origins && !cur_ddata->redirects)
>> + goto out;
>> +
>> + /* Fix impure xattrs */
>> + if (!ovl_is_impure(pathname)) {
>> + if (ovl_ask_action("Missing impure xattr",
>> + pathname, "Fix", 1)) {
>> + ret = ovl_set_impure(pathname);
>> + if (ret)
>> + goto out;
>> + }
>> + }
>> +out:
>> + return ret;
>> +}
>> +
>> static struct scan_operations ovl_scan_ops = {
>> .whiteout = ovl_check_whiteout,
>> .redirect = ovl_check_redirect,
>> + .origin = ovl_check_origin,
>> + .impure = ovl_check_impure,
>> };
>>
>> static void ovl_scan_report(struct scan_ctx *sctx)
>> diff --git a/lib.c b/lib.c
>> index 497d357..524246d 100644
>> --- a/lib.c
>> +++ b/lib.c
>> @@ -165,8 +165,17 @@ const char *path_pick(const char *path, int baselen)
>> return (*p == '/') ? p+1 : p;
>> }
>>
>> -static inline int __check_entry(struct scan_ctx *sctx,
>> - int (*do_check)(struct scan_ctx *))
>> +static void scan_entry_init(struct scan_ctx *sctx, FTSENT *ftsent)
>> +{
>> + sctx->pathname = ftsent->fts_path;
>> + sctx->pathlen = ftsent->fts_pathlen;
>> + sctx->filename = ftsent->fts_name;
>> + sctx->filelen = ftsent->fts_namelen;
>> + sctx->st = ftsent->fts_statp;
>> +}
>> +
>> +static inline int scan_check_entry(int (*do_check)(struct scan_ctx *),
>> + struct scan_ctx *sctx)
>> {
>> return do_check ? do_check(sctx) : 0;
>> }
>> @@ -187,8 +196,6 @@ int scan_dir(struct scan_ctx *sctx, struct scan_operations *sop)
>> }
>>
>> while ((ftsent = fts_read(ftsp)) != NULL) {
>> - int err;
>> -
>> print_debug(_("Scan:%-3s %2d %7jd %-40s %s\n"),
>> (ftsent->fts_info == FTS_D) ? "d" :
>> (ftsent->fts_info == FTS_DNR) ? "dnr" :
>> @@ -202,26 +209,54 @@ int scan_dir(struct scan_ctx *sctx, struct scan_operations *sop)
>> ftsent->fts_path, ftsent->fts_name);
>>
>> /* Fillup base context */
>> - sctx->pathname = ftsent->fts_path;
>> - sctx->pathlen = ftsent->fts_pathlen;
>> - sctx->filename = ftsent->fts_name;
>> - sctx->filelen = ftsent->fts_namelen;
>> - sctx->st = ftsent->fts_statp;
>> + scan_entry_init(sctx, ftsent);
>>
>> switch (ftsent->fts_info) {
>> case FTS_F:
>> sctx->files++;
>> +
>> + /* Check origin */
>> + ret = scan_check_entry(sop->origin, sctx);
>> + if (ret)
>> + goto out;
>> break;
>> case FTS_DEFAULT:
>> /* Check whiteouts */
>> - err = __check_entry(sctx, sop->whiteout);
>> - ret = (err) ? err : ret;
>> + ret = scan_check_entry(sop->whiteout, sctx);
>> + if (ret)
>> + goto out;
>> +
>> + /* Check origin */
>> + ret = scan_check_entry(sop->origin, sctx);
>> + if (ret)
>> + goto out;
>
> All the re-factoring of scan helpers and this additional origin check
> do not seem relevant to this change's subject (impure dir check).
>
Current origin check function only count origin targets in a directory
(this function can used for future check). Impure dir check use this
counts to distinguish this directory is impure or not, so this origin
check is necessary in this patch.
Thanks,
Yi.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v3 6/6] fsck.overlay: add impure xattr check
2017-12-29 2:21 ` zhangyi (F)
@ 2017-12-29 7:03 ` Amir Goldstein
2017-12-29 7:39 ` zhangyi (F)
0 siblings, 1 reply; 18+ messages in thread
From: Amir Goldstein @ 2017-12-29 7:03 UTC (permalink / raw)
To: zhangyi (F)
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On Fri, Dec 29, 2017 at 4:21 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
> On 2017/12/28 21:18, Amir Goldstein Wrote:
>> On Thu, Dec 28, 2017 at 1:40 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>>> An impure directory is a directory with "trusted.overlay.impure" xattr
>>> valued 'y', which indicate that this directory may contain copy-uped
>>> targets from lower layers. In oredr to prevent 'd_ino' change while
>>> copy-up (it create a new inode in upper layer) in getdents(2), impure
>>> xattr will be set in the parent directory, letting overlay filesystem
>>> check and get d_ino from lower origin target to ensure consistent d_ino.
>>>
>>> There are three situations of setting impure xattr:
>>> 1) Copyup lower target in a directory.
>>> 2) Link an origined target (already copy-uped, have origin xattr) into
>>> a directory.
>>> 3) Rename an origined target (include merged subdirectories) into a
>>> new directory.
>>>
>>> So, if a direcotry which contains several origined targets or redirect
>>> directories, the impure xattr should be set. If not, fix this xattr.
>>>
>>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>>> ---
[...]
>>> case FTS_DEFAULT:
>>> /* Check whiteouts */
>>> - err = __check_entry(sctx, sop->whiteout);
>>> - ret = (err) ? err : ret;
>>> + ret = scan_check_entry(sop->whiteout, sctx);
>>> + if (ret)
>>> + goto out;
>>> +
>>> + /* Check origin */
>>> + ret = scan_check_entry(sop->origin, sctx);
>>> + if (ret)
>>> + goto out;
>>
>> All the re-factoring of scan helpers and this additional origin check
>> do not seem relevant to this change's subject (impure dir check).
>>
> Current origin check function only count origin targets in a directory
> (this function can used for future check). Impure dir check use this
> counts to distinguish this directory is impure or not, so this origin
> check is necessary in this patch.
>
I see. Anyway, the cleaner the patch is, the easier it is to review it.
A re-factoring patch that does not change behavior and declares this
in commit message is easy to verify in review and a logic change
patch that declares the logical change in commit message is easy to
review. Mixing them both in a single patch without being able to declare
that this is only a logical change not that this is only re-factoring makes
reviewing harder.
This is generally speaking. This patch was easy enough to review
anyway, but other reviewers may not feel the same way.
Cheers,
Amir.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 6/6] fsck.overlay: add impure xattr check
2017-12-29 7:03 ` Amir Goldstein
@ 2017-12-29 7:39 ` zhangyi (F)
0 siblings, 0 replies; 18+ messages in thread
From: zhangyi (F) @ 2017-12-29 7:39 UTC (permalink / raw)
To: Amir Goldstein
Cc: overlayfs, fstests, Miklos Szeredi, Eryu Guan, Miao Xie,
yangerkun
On 2017/12/29 15:03, Amir Goldstein Wrote:
> On Fri, Dec 29, 2017 at 4:21 AM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>> On 2017/12/28 21:18, Amir Goldstein Wrote:
>>> On Thu, Dec 28, 2017 at 1:40 PM, zhangyi (F) <yi.zhang@huawei.com> wrote:
>>>> An impure directory is a directory with "trusted.overlay.impure" xattr
>>>> valued 'y', which indicate that this directory may contain copy-uped
>>>> targets from lower layers. In oredr to prevent 'd_ino' change while
>>>> copy-up (it create a new inode in upper layer) in getdents(2), impure
>>>> xattr will be set in the parent directory, letting overlay filesystem
>>>> check and get d_ino from lower origin target to ensure consistent d_ino.
>>>>
>>>> There are three situations of setting impure xattr:
>>>> 1) Copyup lower target in a directory.
>>>> 2) Link an origined target (already copy-uped, have origin xattr) into
>>>> a directory.
>>>> 3) Rename an origined target (include merged subdirectories) into a
>>>> new directory.
>>>>
>>>> So, if a direcotry which contains several origined targets or redirect
>>>> directories, the impure xattr should be set. If not, fix this xattr.
>>>>
>>>> Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
>>>> ---
> [...]
>>>> case FTS_DEFAULT:
>>>> /* Check whiteouts */
>>>> - err = __check_entry(sctx, sop->whiteout);
>>>> - ret = (err) ? err : ret;
>>>> + ret = scan_check_entry(sop->whiteout, sctx);
>>>> + if (ret)
>>>> + goto out;
>>>> +
>>>> + /* Check origin */
>>>> + ret = scan_check_entry(sop->origin, sctx);
>>>> + if (ret)
>>>> + goto out;
>>>
>>> All the re-factoring of scan helpers and this additional origin check
>>> do not seem relevant to this change's subject (impure dir check).
>>>
>> Current origin check function only count origin targets in a directory
>> (this function can used for future check). Impure dir check use this
>> counts to distinguish this directory is impure or not, so this origin
>> check is necessary in this patch.
>>
>
> I see. Anyway, the cleaner the patch is, the easier it is to review it.
> A re-factoring patch that does not change behavior and declares this
> in commit message is easy to verify in review and a logic change
> patch that declares the logical change in commit message is easy to
> review. Mixing them both in a single patch without being able to declare
> that this is only a logical change not that this is only re-factoring makes
> reviewing harder.
>
> This is generally speaking. This patch was easy enough to review
> anyway, but other reviewers may not feel the same way.
>
Thanks for your advice, I can make it more clear and readable for the next posting.
Thanks,
Yi.
>
> .
>
^ permalink raw reply [flat|nested] 18+ messages in thread