On 03/07/2013 03:59 PM, Reshetova, Elena wrote:
On 03/06/2013 05:01 PM, Reshetova, Elena wrote:
What's missing is that another call to the hook is needed in
fsmMkdirs() for the unowned directories, and there we should perhaps pass
in the "unowned"
aspect somehow. Having a separate argument for that seems like an
overkill though... maybe we could pass that piece of info in the
action argument instead. One possibility could be adding a new
action, eg FA_CREATEUNOWNED that could be used for unowned
directories (and files, but there aren't any currently). Or we could
define the action as a partial bitfield: leave the existing actions
alone but reserve the "upper" byte for special bits, such as
"unknown". I had some other use-case for turning it into (partial) bitfield
but can't remember what it was right now.
I think bitfield would be better in this case: if one start introduce
actions FA_CREATEUNOWNED, then why not have FA_CREATELINK and etc. I
will add hook calling for mkdirs, but I think it is better that you do
a change of actions in a separate commit.
Sure, and I agree bitfield seems like the better option as it'll allow
cramming a whole lot more information in there. There's a whole lot of
redundancy in the current actions (all the skip cases for example) and some
of the values are totally unused as well.
As the file actions aren't really exported in the API in a way that somebody
could actually be using them, we might be able to get away with just
redefining the whole thing as a bitfield and add the old symbols as defines
or'ed together from the bits. But I'll need to think about it some more, in
any case such >a total conversion is not required in order to add a handful
of bits right now.
Ok, I think if you can add an "unowned" bit to it for a start, this can be a
good beginning and then even the existing pre/post hooks can get one less
argument, which is great.
Sure, will do.
Continue on bit field idea, it would be great for plugins to get the
basic info about the file acted upon from action also: so , if we are
adding unowned bit, should the basic bits indicating hard or sym link
be also supplied in action? Like action could point that it is
creation of symlink or removal of hard link. Is it too bad idea?
Hmm, but we already pass the mode (and planning to pass the whole stat
struct) around, so you can tell whether its a symlink, directory or something
else. Hardlinks are the more special case but you can tell them apart from
others by st_nlinks from the stat struct. Except that you dont necessarily
know which one is "real" file without extra tracking... unless st_nlinks is 1
for real >files (including the first
hardlink) and more for the actual links. I'm not sure if that's the case even
already, but should be possible to arrange at least.
I am quite sure you can't tell it from stat struct which is real file and
which is hard link: the value for st_nlinks is stored in inode and not in
dentry in my understanding, that's why it isn't really easy for plugin to
detect the hard links, so indicator would be a plus...
A file is a hard-link if (S_ISREG(st->st_mode) && st->st_nlink > 1) is
true. When erasing, we get this info from filesystem so that remains
accurate (the last one would be seen as the "real" file). On
installation the stat struct of a file is made up by rpm, so we can pass
whatever we want in there. Currently st_nlink for hardlinks equals the
total number of links a file will have, but we can easily change that to
the number of *current* links so that it better matches reality. Ie the
first one will have st_nlink == 1 so it will be seen as the real file,
the rest will st_nlink++ each. See attached patch for a quick
implementation of this.
- Panu -
diff --git a/lib/fsm.c b/lib/fsm.c
index f28010c..5bd5bfa 100644
--- a/lib/fsm.c
+++ b/lib/fsm.c
@@ -1052,7 +1052,7 @@ static int fsmMakeLinks(FSM_t fsm, hardLink_t li)
return ec;
}
-static int fsmCommit(FSM_t fsm, int ix, int setmeta);
+static int fsmCommit(FSM_t fsm, int ix, const struct stat * st);
/** \ingroup payload
* Commit hard linked file set atomically.
@@ -1065,8 +1065,7 @@ static int fsmCommitLinks(FSM_t fsm)
const char * nsuffix = fsm->nsuffix;
struct stat * st = &fsm->sb;
int rc = 0;
- int setmeta = 1;
- nlink_t i;
+ nlink_t i, link_count = 1;
hardLink_t li;
fsm->path = NULL;
@@ -1081,10 +1080,10 @@ static int fsmCommitLinks(FSM_t fsm)
if (li->filex[i] < 0) continue;
rc = fsmMapPath(fsm, li->filex[i]);
if (!XFA_SKIPPING(fsm->action)) {
- rc = fsmCommit(fsm, li->filex[i], setmeta);
- /* only the first created link needs permissions etc to be set */
+ st->st_nlink = link_count;
+ rc = fsmCommit(fsm, li->filex[i], st);
if (!rc)
- setmeta = 0;
+ link_count++;
}
fsm->path = _free(fsm->path);
li->filex[i] = -1;
@@ -1542,10 +1541,9 @@ static int fsmBackup(FSM_t fsm)
return rc;
}
-static int fsmCommit(FSM_t fsm, int ix, int setmeta)
+static int fsmCommit(FSM_t fsm, int ix, const struct stat *st)
{
int rc = 0;
- struct stat * st = &fsm->sb;
/* XXX Special case /dev/log, which shouldn't be packaged anyways */
if (S_ISSOCK(st->st_mode) && IS_DEV_LOG(fsm->path))
@@ -1568,7 +1566,8 @@ static int fsmCommit(FSM_t fsm, int ix, int setmeta)
fsm->path = npath;
}
- if (setmeta) {
+ /* only the first created link needs permissions etc to be set */
+ if (!(S_ISREG(st->st_mode) && st->st_nlink > 1)) {
/* Set file security context (if enabled) */
if (!rc && !getuid()) {
rc = fsmSetSELabel(fsm->sehandle, fsm->path, fsm->sb.st_mode);
@@ -1790,7 +1789,7 @@ int rpmPackageFilesInstall(rpmts ts, rpmte te, rpmfi fi, FD_t cfd,
if (!fsm->postpone) {
rc = ((S_ISREG(st->st_mode) && st->st_nlink > 1)
- ? fsmCommitLinks(fsm) : fsmCommit(fsm, fsm->ix, 1));
+ ? fsmCommitLinks(fsm) : fsmCommit(fsm, fsm->ix, st));
}
}
_______________________________________________
Rpm-maint mailing list
[email protected]
http://lists.rpm.org/mailman/listinfo/rpm-maint
