From fd8ffffced543e248847d63e9375fb95584f998d Mon Sep 17 00:00:00 2001 From: chantra Date: Fri, 11 Feb 2022 18:09:47 -0800 Subject: [PATCH 22/30] [reflink] fix support for hardlinks a `suffix` made of ";${tid}" is now used until the changes are commited. As such, when linking the file, we should link to the suffixed file. The `suffix` is currently internal only to rpmPackageFilesInstall and there is no obvious way to use data from struct `filedata_s` without exposing it to a bunch of other places such as rpmplugin*. We already keep track of the first file index. We used to use this to fetch the file name, but due to suffix, the file is not named correctly, e.g it has the name we will want at the end, once the transaction is commited, not the temporary name. Instead of re-implementing a local suffix that may change over time as the implementation evolves, we can store the name of the file we should link to in the hash instead of the index, which we were then using to fetch the file name. When hardlinking, we can then retrieve the name of the target file instead of the index, like we previously did, and hardlink to that without any issues. Add a test to validate that reflink can handle hardlinking. Originally, the test added would fail with: ``` error: reflink: Unable to hard link /foo/hello -> /foo/hello-bar;6207219c due to No such file or directory error: Plugin reflink: hook fsm_file_install failed error: unpacking of archive failed on file /foo/hello-bar;6207219c: cpio: (error 0x2) error: hlinktest-1.0-1.noarch: install failed ./rpm2extents.at:114: exit code was 1, expected 0 499. rpm2extents.at:112: 499. reflink hardlink package (rpm2extents.at:112): FAILED (rpm2extents.at:114) ``` After this change, the test passes. --- plugins/reflink.c | 19 ++++++++----------- tests/rpm2extents.at | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/plugins/reflink.c b/plugins/reflink.c index d5e6db27a..4fc1d74d1 100644 --- a/plugins/reflink.c +++ b/plugins/reflink.c @@ -29,7 +29,7 @@ #undef HTDATATYPE #define HASHTYPE inodeIndexHash #define HTKEYTYPE rpm_ino_t -#define HTDATATYPE int +#define HTDATATYPE const char * #include "lib/rpmhash.H" #include "lib/rpmhash.C" @@ -163,7 +163,7 @@ static rpmRC reflink_psm_pre(rpmPlugin plugin, rpmte te) { return RPMRC_FAIL; } state->inodeIndexes = inodeIndexHashCreate( - state->keys, inodeId, inodeCmp, NULL, NULL + state->keys, inodeId, inodeCmp, NULL, (inodeIndexHashFreeData)rfree ); } @@ -226,7 +226,7 @@ static rpmRC reflink_fsm_file_install(rpmPlugin plugin, rpmfi fi, const char* pa struct file_clone_range fcr; rpm_loff_t size; int dst, rc; - int *hlix; + const char **hl_target = NULL; reflink_state state = rpmPluginGetData(plugin); if (state->table == NULL) { @@ -243,18 +243,15 @@ static rpmRC reflink_fsm_file_install(rpmPlugin plugin, rpmfi fi, const char* pa /* check for hard link entry in table. GetEntry overwrites hlix with * the address of the first match. */ - if (inodeIndexHashGetEntry(state->inodeIndexes, inode, &hlix, NULL, - NULL)) { + if (inodeIndexHashGetEntry(state->inodeIndexes, inode, &hl_target, + NULL, NULL)) { /* entry is in table, use hard link */ - char *fn = rpmfilesFN(state->files, hlix[0]); - if (link(fn, path) != 0) { + if (link(hl_target[0], path) != 0) { rpmlog(RPMLOG_ERR, _("reflink: Unable to hard link %s -> %s due to %s\n"), - fn, path, strerror(errno)); - free(fn); + hl_target[0], path, strerror(errno)); return RPMRC_FAIL; } - free(fn); return RPMRC_PLUGIN_CONTENTS; } /* if we didn't hard link, then we'll track this inode as being @@ -262,7 +259,7 @@ static rpmRC reflink_fsm_file_install(rpmPlugin plugin, rpmfi fi, const char* pa */ if (rpmfiFNlink(fi) > 1) { /* minor optimization: only store files with more than one link */ - inodeIndexHashAddEntry(state->inodeIndexes, inode, rpmfiFX(fi)); + inodeIndexHashAddEntry(state->inodeIndexes, inode, rstrdup(path)); } /* derived from wfd_open in fsm.c */ mode_t old_umask = umask(0577); diff --git a/tests/rpm2extents.at b/tests/rpm2extents.at index fa124ac03..5c66de7f6 100644 --- a/tests/rpm2extents.at +++ b/tests/rpm2extents.at @@ -123,3 +123,18 @@ test -f ${RPMTEST}/usr/bin/hello [], []) AT_CLEANUP + +AT_SETUP([reflink hardlink package]) +AT_KEYWORDS([reflink hardlink]) +AT_SKIP_IF([$REFLINK_DISABLED]) +AT_CHECK([ +RPMDB_INIT + +PKG=hlinktest-1.0-1.noarch.rpm +runroot_other cat /data/RPMS/${PKG} | runroot_other rpm2extents SHA256 > ${RPMTEST}/tmp/${PKG} 2> /dev/null +runroot_plugins rpm -i --nodigest --nodeps --undefine=%__transaction_dbus_announce /tmp/${PKG} +], +[0], +[], +[]) +AT_CLEANUP -- 2.35.1