On 2019/10/15 22:55, Brian Foster wrote:
> On Fri, Oct 11, 2019 at 03:56:01PM +0800, kaixuxia wrote:
>> Support the renameat2 syscall in fsstress.
>>
>> Signed-off-by: kaixuxia <kaixuxia@xxxxxxxxxxx>
>> ---
>> ltp/fsstress.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 79 insertions(+), 11 deletions(-)
>>
>> diff --git a/ltp/fsstress.c b/ltp/fsstress.c
>> index 51976f5..21529a2 100644
>> --- a/ltp/fsstress.c
>> +++ b/ltp/fsstress.c
>> @@ -44,6 +44,16 @@ io_context_t io_ctx;
>> #define IOV_MAX 1024
>> #endif
>>
>> +#ifndef RENAME_NOREPLACE
>> +#define RENAME_NOREPLACE (1 << 0) /* Don't overwrite target */
>> +#endif
>> +#ifndef RENAME_EXCHANGE
>> +#define RENAME_EXCHANGE (1 << 1) /* Exchange source and dest */
>> +#endif
>> +#ifndef RENAME_WHITEOUT
>> +#define RENAME_WHITEOUT (1 << 2) /* Whiteout source */
>> +#endif
>> +
>> #define FILELEN_MAX (32*4096)
>>
>> typedef enum {
>> @@ -85,6 +95,9 @@ typedef enum {
>> OP_READV,
>> OP_REMOVEFATTR,
>> OP_RENAME,
>> + OP_RNOREPLACE,
>> + OP_REXCHANGE,
>> + OP_RWHITEOUT,
>> OP_RESVSP,
>> OP_RMDIR,
>> OP_SETATTR,
>> @@ -203,6 +216,9 @@ void readlink_f(int, long);
>> void readv_f(int, long);
>> void removefattr_f(int, long);
>> void rename_f(int, long);
>> +void rnoreplace_f(int, long);
>> +void rexchange_f(int, long);
>> +void rwhiteout_f(int, long);
>> void resvsp_f(int, long);
>> void rmdir_f(int, long);
>> void setattr_f(int, long);
>> @@ -262,6 +278,9 @@ opdesc_t ops[] = {
>> /* remove (delete) extended attribute */
>> { OP_REMOVEFATTR, "removefattr", removefattr_f, 1, 1 },
>> { OP_RENAME, "rename", rename_f, 2, 1 },
>> + { OP_RNOREPLACE, "rnoreplace", rnoreplace_f, 2, 1 },
>> + { OP_REXCHANGE, "rexchange", rexchange_f, 2, 1 },
>> + { OP_RWHITEOUT, "rwhiteout", rwhiteout_f, 2, 1 },
>> { OP_RESVSP, "resvsp", resvsp_f, 1, 1 },
>> { OP_RMDIR, "rmdir", rmdir_f, 1, 1 },
>> /* set attribute flag (FS_IOC_SETFLAGS ioctl) */
>> @@ -354,7 +373,7 @@ int open_path(pathname_t *, int);
>> DIR *opendir_path(pathname_t *);
>> void process_freq(char *);
>> int readlink_path(pathname_t *, char *, size_t);
>> -int rename_path(pathname_t *, pathname_t *);
>> +int rename_path(pathname_t *, pathname_t *, int);
>> int rmdir_path(pathname_t *);
>> void separate_pathname(pathname_t *, char *, pathname_t *);
>> void show_ops(int, char *);
>> @@ -1519,7 +1538,7 @@ readlink_path(pathname_t *name, char *lbuf, size_t lbufsiz)
>> }
>>
>> int
>> -rename_path(pathname_t *name1, pathname_t *name2)
>> +rename_path(pathname_t *name1, pathname_t *name2, int mode)
>> {
>> char buf1[NAME_MAX + 1];
>> char buf2[NAME_MAX + 1];
>> @@ -1528,14 +1547,14 @@ rename_path(pathname_t *name1, pathname_t *name2)
>> pathname_t newname2;
>> int rval;
>>
>> - rval = rename(name1->path, name2->path);
>> + rval = syscall(__NR_renameat2, AT_FDCWD, name1->path, AT_FDCWD, name2->path, mode);
>
> Perhaps we should preserve the rename() call with an ifdef or something
> (similar to src/renameat2.c) in the event that renameat2 is not
> available. In fact, I suppose we'd want to compile out the new renameat2
> based ops as well so the user doesn't have to disable them where they
> aren't supported.
Right, will fix it in V2.
>
>> if (rval >= 0 || errno != ENAMETOOLONG)
>> return rval;
>> separate_pathname(name1, buf1, &newname1);
>> separate_pathname(name2, buf2, &newname2);
>> if (strcmp(buf1, buf2) == 0) {
>> if (chdir(buf1) == 0) {
>> - rval = rename_path(&newname1, &newname2);
>> + rval = rename_path(&newname1, &newname2, mode);
>> assert(chdir("..") == 0);
>> }
>> } else {
>> @@ -1555,7 +1574,7 @@ rename_path(pathname_t *name1, pathname_t *name2)
>> append_pathname(&newname2, "../");
>> append_pathname(&newname2, name2->path);
>> if (chdir(buf1) == 0) {
>> - rval = rename_path(&newname1, &newname2);
>> + rval = rename_path(&newname1, &newname2, mode);
>> assert(chdir("..") == 0);
>> }
>> } else {
>> @@ -1563,7 +1582,7 @@ rename_path(pathname_t *name1, pathname_t *name2)
>> append_pathname(&newname1, "../");
>> append_pathname(&newname1, name1->path);
>> if (chdir(buf2) == 0) {
>> - rval = rename_path(&newname1, &newname2);
>> + rval = rename_path(&newname1, &newname2, mode);
>> assert(chdir("..") == 0);
>> }
>> }
>> @@ -4215,8 +4234,18 @@ out:
>> free_pathname(&f);
>> }
>>
>> +struct print_flags renameat2_flags [] = {
>> + { RENAME_NOREPLACE, "NOREPLACE"},
>> + { RENAME_EXCHANGE, "EXCHANGE"},
>> + { RENAME_WHITEOUT, "WHITEOUT"},
>> + { -1, NULL}
>> +};
>> +
>> +#define translate_renameat2_flags(mode) \
>> + ({translate_flags(mode, "|", renameat2_flags);})
>> +
>> void
>> -rename_f(int opno, long r)
>> +do_renameat2(int opno, long r, int mode)
>> {
>> fent_t *dfep;
>> int e;
>> @@ -4229,6 +4258,7 @@ rename_f(int opno, long r)
>> int parid;
>> int v;
>> int v1;
>> + int fd;
>>
>> /* get an existing path for the source of the rename */
>> init_pathname(&f);
>> @@ -4260,7 +4290,21 @@ rename_f(int opno, long r)
>> free_pathname(&f);
>> return;
>> }
>> - e = rename_path(&f, &newf) < 0 ? errno : 0;
>> + /* Both pathnames must exist for the RENAME_EXCHANGE */
>> + if (mode == RENAME_EXCHANGE) {
>> + fd = creat_path(&newf, 0666);
>> + e = fd < 0 ? errno : 0;
>> + check_cwd();
>> + if (fd < 0) {
>> + if (v)
>> + printf("%d/%d: renameat2 - creat %s failed %d\n",
>> + procid, opno, newf.path, e);
>> + free_pathname(&newf);
>> + free_pathname(&f);
>> + return;
>> + }
>> + }
>> + e = rename_path(&f, &newf, mode) < 0 ? errno : 0;
>
> It looks like the existing code looks up a source entry and a target
> directory and generates a new name to serve as the rename target entry.
> Here we create the entry so we can perform an exchange. Can we check
> mode earlier and instead look up another existing entry if this is an
> exchange, else run the existing code to generate a name?
Yeah, it is more reasonable, will fix it in V2.
>
> Other than those couple things this looks pretty good to me.
>
> Brian
>
>
>> check_cwd();
>> if (e == 0) {
>> int xattr_counter = fep->xattr_counter;
>> @@ -4273,12 +4317,13 @@ rename_f(int opno, long r)
>> add_to_flist(flp - flist, id, parid, xattr_counter);
>> }
>> if (v) {
>> - printf("%d/%d: rename %s to %s %d\n", procid, opno, f.path,
>> + printf("%d/%d: rename(%s) %s to %s %d\n", procid,
>> + opno, translate_renameat2_flags(mode), f.path,
>> newf.path, e);
>> if (e == 0) {
>> - printf("%d/%d: rename del entry: id=%d,parent=%d\n",
>> + printf("%d/%d: rename source entry: id=%d,parent=%d\n",
>> procid, opno, fep->id, fep->parent);
>> - printf("%d/%d: rename add entry: id=%d,parent=%d\n",
>> + printf("%d/%d: rename target entry: id=%d,parent=%d\n",
>> procid, opno, id, parid);
>> }
>> }
>> @@ -4287,6 +4332,29 @@ rename_f(int opno, long r)
>> }
>>
>> void
>> +rename_f(int opno, long r)
>> +{
>> + do_renameat2(opno, r, 0);
>> +}
>> +void
>> +rnoreplace_f(int opno, long r)
>> +{
>> + do_renameat2(opno, r, RENAME_NOREPLACE);
>> +}
>> +
>> +void
>> +rexchange_f(int opno, long r)
>> +{
>> + do_renameat2(opno, r, RENAME_EXCHANGE);
>> +}
>> +
>> +void
>> +rwhiteout_f(int opno, long r)
>> +{
>> + do_renameat2(opno, r, RENAME_WHITEOUT);
>> +}
>> +
>> +void
>> resvsp_f(int opno, long r)
>> {
>> int e;
>> --
>> 1.8.3.1
>>
>> --
>> kaixuxia
--
kaixuxia
