The free space on rootfs may be too small that it prevents emerge from working. For example, cp /tmp/100m#new /100m will fail when we have 10m left on rootfs. Note that this patch will not help if the large file is opened at the moment of unlinking. Reproducible: Always Steps to Reproduce: 1. fill rootfs with random data to make sure only few space left. 2. emerge a package that contains a large file Actual Results: emerge failed Expected Results: emerge succeeded
Sorry, but this bug report makes no sense. I would expect emerge to fail if there is insufficient space on the target filesystem.
Sorry. I just found the description is wrong. > For example, cp /tmp/100m#new /100m will fail when we have 10m left on rootfs. should be > For example, cp /tmp/100m /100m#new will fail when we have 10m left on rootfs. It's not that the device has no sufficient space. It has sufficient space. For example, if we call `mv /tmp/100m /100m` instead of `cp /tmp/100m /100m#new; mv /100m#new /100m`, it will work. IIUC, the current behavior is to make copy atomic, i.e., to make sure there won't be a state where we only copied part of the file to /100m. However, it prevents the use case as shown in the example. My patch is to 'rm /100m; cp /tmp/100m /100m' if `cp /tmp/100m /100m#new; mv /100m#new /100m` fails. The side effect is that if the later cp also fails then the original /100m will be gone but I think this is acceptable since the package is already broken now because some files are updated while some others are not. (Please correct me here if I am wrong.) Also, such a failure will fill the rootfs by not deleting /100m#new, which I believe it is not expected.
I don't understand what is failing or where. Are you saying there's an issue with the check-reqs eclass, or something else? There is also no patch attached, which would help us understand what you're referring to.
Created attachment 663286 [details, diff] patch
I have attached the patch. The issue can be reproduced by 1. emerge a package with a large file in it. 2. fill rootfs to make sure it has only few MBs left. 3. emerge the same package. precondition: the mount point for the source and the target should be different. Otherwise, emerge will just call mv.
Please send the patch to gentoo-portage-dev@lists.gentoo.org for review.
I think the reporter is saying: 1) During the qmerge portion of ebuild we copy files from ${D} to the rootfs. 2) the rootfs might not have enough space, so if you just copy the files from ${D} to the rootfs, you might fail an operation where: 2a) You open FILE for writing, and start writing... 2b) You run out of space, and get ENOSPACE on your write() call. 2c) This leaves FILE half-written with potential garbage. 2d) If FILE was an important file, its broke now. The solution to this is to merge all files adjacent to FILE (e.g. FILE.TMP), then atomically rename over FILE. My understanding of portage.util.movefile is that this is already implemented. if renamefailed: if stat.S_ISREG(sstat[stat.ST_MODE]): dest_tmp = dest + "#new" dest_tmp_bytes = _unicode_encode(dest_tmp, encoding=encoding, errors='strict') try: # For safety copy then move it over. _copyfile(src_bytes, dest_tmp_bytes) _apply_stat(sstat, dest_tmp_bytes) if xattr_enabled: try: _copyxattr(src_bytes, dest_tmp_bytes, exclude=mysettings.get("PORTAGE_XATTR_EXCLUDE", "")) except SystemExit: raise except: msg = _("Failed to copy extended attributes. " "In order to avoid this error, set " "FEATURES=\"-xattr\" in make.conf.") msg = textwrap.wrap(msg, 65) for line in msg: writemsg("!!! %s\n" % (line,), noiselevel=-1) raise _rename(dest_tmp_bytes, dest_bytes) _os.unlink(src_bytes) The patch seems to take this bit of code, call it "copy_file()" and then also call it later: writemsg("!!! %s\n" % _('copy %(src)s -> %(dest)s failed.') % {"src": src, "dest": dest}, noiselevel=-1) writemsg("!!! %s\n" % (e,), noiselevel=-1) return None # here the patch wants to delete dest, then try again..I've no # idea why this is advantageous? You are going to need to use more words to describe the problem.
(In reply to Boleyn Su from comment #5) > I have attached the patch. > > The issue can be reproduced by > > 1. emerge a package with a large file in it. > 2. fill rootfs to make sure it has only few MBs left. > 3. emerge the same package. > > precondition: the mount point for the source and the target should be > different. Otherwise, emerge will just call mv. So my reading of your report is: a) dev-example/foo is a package with 1 100MB file on it. b) My rootfs has 10MB of space remaining. You then want: 1) To only have 1 copy of the package in the rootfs. 2) Atomically replace the package in the rootfs. 3) Not use more than 1x the package space during the merge. Is that basically the gist? -A
(In reply to Alec Warner from comment #8) > (In reply to Boleyn Su from comment #5) > > I have attached the patch. > > > > The issue can be reproduced by > > > > 1. emerge a package with a large file in it. > > 2. fill rootfs to make sure it has only few MBs left. > > 3. emerge the same package. > > > > precondition: the mount point for the source and the target should be > > different. Otherwise, emerge will just call mv. > > So my reading of your report is: > a) dev-example/foo is a package with 1 100MB file on it. > b) My rootfs has 10MB of space remaining. > > You then want: > > 1) To only have 1 copy of the package in the rootfs. > 2) Atomically replace the package in the rootfs. > 3) Not use more than 1x the package space during the merge. > > Is that basically the gist? > > -A This would allow you to basically install a new copy of the package, not run out of space, and note that emerge *does* work this way when ${D} and the rootfs are on the same FS (and we can use mv to atomically replace files.) It doesn't work this way when ${D} and the rootfs are on different filesystems, because you cannot atomically replace files in that manner. Its unclear how your patch solves this problem. Its also unclear if its possible to both do atomic replacement and not save space when the filesystems differ. -A
(In reply to Alec Warner from comment #9) > Its unclear how your patch solves this problem. > Its also unclear if its possible to both do atomic replacement and not save > space when the filesystems differ. My interpretation of the patch is that it gives up on replacing the file in an atomic way if doing so fails for any reason. Instead, as a second attempt, it unlinks the file first, and then copies over the replacement. This "feels" somewhat dangerous to me, and I can't picture a real-world example where this would actually be useful in practice.
(In reply to Alec Warner from comment #8) > (In reply to Boleyn Su from comment #5) > > I have attached the patch. > > > > The issue can be reproduced by > > > > 1. emerge a package with a large file in it. > > 2. fill rootfs to make sure it has only few MBs left. > > 3. emerge the same package. > > > > precondition: the mount point for the source and the target should be > > different. Otherwise, emerge will just call mv. > > So my reading of your report is: > a) dev-example/foo is a package with 1 100MB file on it. > b) My rootfs has 10MB of space remaining. > > You then want: > > 1) To only have 1 copy of the package in the rootfs. > 2) Atomically replace the package in the rootfs. > 3) Not use more than 1x the package space during the merge. > > Is that basically the gist? > > -A Yes. This is the general idea. > Its unclear how your patch solves this problem. > Its also unclear if its possible to both do atomic replacement and not save space when the filesystems differ. If we rm /100mb first, the we will have 110mb free on rootfs. cp will succeed then (on the condition that /100mb is not opened by anyone currently).
(In reply to Mike Gilbert from comment #10) > (In reply to Alec Warner from comment #9) > > Its unclear how your patch solves this problem. > > Its also unclear if its possible to both do atomic replacement and not save > > space when the filesystems differ. > > My interpretation of the patch is that it gives up on replacing the file in > an atomic way if doing so fails for any reason. Instead, as a second > attempt, it unlinks the file first, and then copies over the replacement. > > This "feels" somewhat dangerous to me, and I can't picture a real-world > example where this would actually be useful in practice. The second try won't copy partial file, so IIUC the copy is still atomic. The only difference is that if the second try fails, the original file will be gone. However, I think this is acceptable since the package is already broken now because some files are updated while some others are not. (Please correct me here if I am wrong.)
Here's an extreme example of how this could go wrong. Let's say we are merging a critical package like sys-libs/glibc, and copying libc.so.6 fails. With the existing code, the old copy of libc.so.6 will still exist, and the user has a fighting chance of recovering the system. There might be a mix of old and new files installed, but things will likely continue working well enough to clear some disk space and attempt the merge again. With the proposed patch, libc.so.6 will be removed, and it is possible copying the new version over will fail. In this case, the user would be left with a completely broken system, and would almost certainly have to boot into some recovery environment to restore a copy of libc.so.6.
(In reply to Mike Gilbert from comment #13) > Here's an extreme example of how this could go wrong. > > Let's say we are merging a critical package like sys-libs/glibc, and copying > libc.so.6 fails. > > With the existing code, the old copy of libc.so.6 will still exist, and the > user has a fighting chance of recovering the system. There might be a mix of > old and new files installed, but things will likely continue working well > enough to clear some disk space and attempt the merge again. > > With the proposed patch, libc.so.6 will be removed, and it is possible > copying the new version over will fail. In this case, the user would be left > with a completely broken system, and would almost certainly have to boot > into some recovery environment to restore a copy of libc.so.6. Yeah, in this extreme situation, the patch can be dangerous. Mike Frysinger suggested that we can check the size of the file first and only retry if the file is really large (e.g. 100MB?). This may reduce the possibility of the dangerous outcome. However, we still cannot avoid it. Let me see if I can figure out a better solution.
How about we first check if the file is opened by some other process (there will be a race condition but we do not really care) and only do the second try if it is not. That is, we assume a file that is not *always* opened by some other process safe to be missing. (*always* is the reason why we do not care about the race condition)
Let me describe it in detail. We want to cp /tmp/100mb to /100mb. What we do is: 1. cp /tmp/100mb /100mb#new if failed goto F1 2. rename /100mb#new /100mb succeed F1. if /100mb is opened by any process fail F2. rm /100mb may fail F3. cp /tmp/100mb /100mb#new F4. mv /100mb#new /100mb In this way, the second try will only be applied for files that are not opened at the moment of F1 and we assume that such file is OK to be missing because there is a time the system is running without it.
(In reply to Boleyn Su from comment #16) > Let me describe it in detail. > > We want to cp /tmp/100mb to /100mb. What we do is: > > 1. cp /tmp/100mb /100mb#new if failed goto F1 > 2. rename /100mb#new /100mb succeed > F1. if /100mb is opened by any process fail > F2. rm /100mb may fail > F3. cp /tmp/100mb /100mb#new > F4. mv /100mb#new /100mb > > In this way, the second try will only be applied for files that are not > opened at the moment of F1 and we assume that such file is OK to be missing > because there is a time the system is running without it. So I want to tackle essentially two different questions here. (1) Why is this a problem worth solving. E.g. there is clearly additional complexity we could add in merge() to try to make it more efficient, but there are tradeoffs (as we discussed above) and its not clear they are worth it to save disk space; nominally a plentiful resource for most users. So I'm interested in more detail on why this issue is worth addressing. (2) If we decide its worth addressing, can we address it safely? Basically there are many races everywhere. We do a check-reqs to see if there is sufficient space, and if there isn't die (but this is a check-before-use race.) We can unlink the files (as you proposed) and then do a copy, but this is also racey (with other disk consumers.) The races also have dire consequences as pointed out in comment #13; if we lose a race it means we have deleted some file from the rootfs and have not replaced it. This is not acceptable in merge(), IMHO. I think what you end up with is either: HaveEnoughSpace && UnSafeMerge And again we fall back to the idea of "why don't you have enough space, and why are you happy to accept a potentially unsafe merge process?" Why is this a good tradeoff? -A
The use case here is that we run Android on Chrome OS. We want to make it possible to package Android as a normal portage package that can be emerged (it is not currently). However, Chrome OS has very limited space on rootfs, because it is expected that we will not add more packages or data to rootfs later I guess. Also, we package an Android image (around 1GB) in the android-whatever.ebuild. These two lead to the problem I describe in this bug. As for the trade-off, I believe with #comment 16, the dangerous outcome should never happen given that the assumption is correct. Thus, it may be acceptable to resolve this issue.
(In reply to Boleyn Su from comment #18) > The use case here is that we run Android on Chrome OS. We want to make it > possible to package Android as a normal portage package that can be emerged > (it is not currently). However, Chrome OS has very limited space on rootfs, > because it is expected that we will not add more packages or data to rootfs > later I guess. Also, we package an Android image (around 1GB) in the > android-whatever.ebuild. These two lead to the problem I describe in this > bug. > > As for the trade-off, I believe with #comment 16, the dangerous outcome > should never happen given that the assumption is correct. Thus, it may be > acceptable to resolve this issue. (In reply to Alec Warner from comment #17) > (In reply to Boleyn Su from comment #16) > > Let me describe it in detail. > > > > We want to cp /tmp/100mb to /100mb. What we do is: > > > > 1. cp /tmp/100mb /100mb#new if failed goto F1 > > 2. rename /100mb#new /100mb succeed > > F1. if /100mb is opened by any process fail > > F2. rm /100mb may fail > > F3. cp /tmp/100mb /100mb#new > > F4. mv /100mb#new /100mb > > > > In this way, the second try will only be applied for files that are not > > opened at the moment of F1 and we assume that such file is OK to be missing > > because there is a time the system is running without it. > > So I want to tackle essentially two different questions here. > > (1) Why is this a problem worth solving. E.g. there is clearly additional > complexity we could add in merge() to try to make it more efficient, but > there are tradeoffs (as we discussed above) and its not clear they are worth > it to save disk space; nominally a plentiful resource for most users. So I'm > interested in more detail on why this issue is worth addressing. > > (2) If we decide its worth addressing, can we address it safely? Basically > there are many races everywhere. We do a check-reqs to see if there is > sufficient space, and if there isn't die (but this is a check-before-use > race.) We can unlink the files (as you proposed) and then do a copy, but > this is also racey (with other disk consumers.) The races also have dire > consequences as pointed out in comment #13; if we lose a race it means we > have deleted some file from the rootfs and have not replaced it. This is not > acceptable in merge(), IMHO. > > I think what you end up with is either: > HaveEnoughSpace && UnSafeMerge > > And again we fall back to the idea of "why don't you have enough space, and > why are you happy to accept a potentially unsafe merge process?" Why is this > a good tradeoff? > > -A As for > The races also have dire > consequences as pointed out in comment #13; if we lose a race it means we > have deleted some file from the rootfs and have not replaced it. The assumption in comment #13, it that if a file is not always opened then it is safe to be missing. So the example here is still OK. The only situation that the file will be missing is that there is a moment during `emerge` in which that file is not opened by anyone. there is a moment during `emerge` in which that file is not opened by anyone => a file is not always opened => it is safe to be missing
(In reply to Boleyn Su from comment #19) > (In reply to Boleyn Su from comment #18) > > The use case here is that we run Android on Chrome OS. We want to make it > > possible to package Android as a normal portage package that can be emerged > > (it is not currently). However, Chrome OS has very limited space on rootfs, > > because it is expected that we will not add more packages or data to rootfs > > later I guess. Also, we package an Android image (around 1GB) in the > > android-whatever.ebuild. These two lead to the problem I describe in this > > bug. > > > > As for the trade-off, I believe with #comment 16, the dangerous outcome > > should never happen given that the assumption is correct. Thus, it may be > > acceptable to resolve this issue. > > (In reply to Alec Warner from comment #17) > > (In reply to Boleyn Su from comment #16) > > > Let me describe it in detail. > > > > > > We want to cp /tmp/100mb to /100mb. What we do is: > > > > > > 1. cp /tmp/100mb /100mb#new if failed goto F1 > > > 2. rename /100mb#new /100mb succeed > > > F1. if /100mb is opened by any process fail > > > F2. rm /100mb may fail > > > F3. cp /tmp/100mb /100mb#new > > > F4. mv /100mb#new /100mb > > > > > > In this way, the second try will only be applied for files that are not > > > opened at the moment of F1 and we assume that such file is OK to be missing > > > because there is a time the system is running without it. > > > > So I want to tackle essentially two different questions here. > > > > (1) Why is this a problem worth solving. E.g. there is clearly additional > > complexity we could add in merge() to try to make it more efficient, but > > there are tradeoffs (as we discussed above) and its not clear they are worth > > it to save disk space; nominally a plentiful resource for most users. So I'm > > interested in more detail on why this issue is worth addressing. > > > > (2) If we decide its worth addressing, can we address it safely? Basically > > there are many races everywhere. We do a check-reqs to see if there is > > sufficient space, and if there isn't die (but this is a check-before-use > > race.) We can unlink the files (as you proposed) and then do a copy, but > > this is also racey (with other disk consumers.) The races also have dire > > consequences as pointed out in comment #13; if we lose a race it means we > > have deleted some file from the rootfs and have not replaced it. This is not > > acceptable in merge(), IMHO. > > > > I think what you end up with is either: > > HaveEnoughSpace && UnSafeMerge > > > > And again we fall back to the idea of "why don't you have enough space, and > > why are you happy to accept a potentially unsafe merge process?" Why is this > > a good tradeoff? > > > > -A > > As for > > The races also have dire > > consequences as pointed out in comment #13; if we lose a race it means we > > have deleted some file from the rootfs and have not replaced it. > > The assumption in comment #13, it that if a file is not always opened then > it is safe to be missing. So the example here is still OK. So I think the struggle I have is that your example is rather constrained, but the general problem is much broader. - Usage of rootfs on chromeos is constrained; compared to usage of a rootfs on normal linux boxes, which is unconstrained. What i mean by this is that if there is little disk left on /, removing a file and replacing it on ChromeOS may be a safe operation. However in a generic linux box, it may not be safe. Other system functions may consume the free'd space and we are back to this idea that we need to atomically reserve the space. Otherwise its a yet another race :/ - Whether a file is opened or not does not determine safety. If you remove libc6, existing programs will work fine and new programs will crash on startup. THe existing programs don't have libc6 open (they have opened it, mapped it into memory, and then closed the FD.) This may be not be a problem for android-whatever; but its a problem for packages in general. > > The only situation that the file will be missing is that there is a moment > during `emerge` in which that file is not opened by anyone. > > there is a moment during `emerge` in which that file is not opened by anyone > => a file is not always opened > => it is safe to be missing I think I'm happier going with a solution where we add something like: PROPERTIES="unsafe-merge" to the package. This means that for packages like android-whatever; you just that property and say "hey I don't care about atomic merging." Most packages (including really important things like libc6) would not have that property, and would get the normal treatment. Packages that want to be unsafe can be tagged as unsafe. This also removes the implicit nature of heuristics and we keep the explicit metadata tagging for unsafe packages. So lets imagine a system where we had the unsafe-merge property for android-whatever. Would we even bother doing an atomic copy, or would we just copy directly from ${D} to the rootfs? How would those copies work? -A
Here's a workaround that requires no changes to Portage: For ebuilds where you know large files will be replaced and are not critical to system functionality, add a pkg_preinst function to remove the existing file. pkg_preinst() { # Remove the existing file to free up some disk space before merging rm -f "${EROOT}"/oldfile } The downside is that you would need to know the "old" filename that is being replaced. If the version number is part of the filename, you could used the ${REPLACING_VERSIONS} variable as a reference.
(In reply to Mike Gilbert from comment #21) > Here's a workaround that requires no changes to Portage: For ebuilds where > you know large files will be replaced and are not critical to system > functionality, add a pkg_preinst function to remove the existing file. > > pkg_preinst() { > # Remove the existing file to free up some disk space before merging > rm -f "${EROOT}"/oldfile > } > > The downside is that you would need to know the "old" filename that is being > replaced. If the version number is part of the filename, you could used the > ${REPLACING_VERSIONS} variable as a reference. We already have a workaround. `emerge -C android && emerge android` will work for our use case. However, I think it would be better to address this issue in portage itself.
(In reply to Alec Warner from comment #20) > (In reply to Boleyn Su from comment #19) > > (In reply to Boleyn Su from comment #18) > > > The use case here is that we run Android on Chrome OS. We want to make it > > > possible to package Android as a normal portage package that can be emerged > > > (it is not currently). However, Chrome OS has very limited space on rootfs, > > > because it is expected that we will not add more packages or data to rootfs > > > later I guess. Also, we package an Android image (around 1GB) in the > > > android-whatever.ebuild. These two lead to the problem I describe in this > > > bug. > > > > > > As for the trade-off, I believe with #comment 16, the dangerous outcome > > > should never happen given that the assumption is correct. Thus, it may be > > > acceptable to resolve this issue. > > > > (In reply to Alec Warner from comment #17) > > > (In reply to Boleyn Su from comment #16) > > > > Let me describe it in detail. > > > > > > > > We want to cp /tmp/100mb to /100mb. What we do is: > > > > > > > > 1. cp /tmp/100mb /100mb#new if failed goto F1 > > > > 2. rename /100mb#new /100mb succeed > > > > F1. if /100mb is opened by any process fail > > > > F2. rm /100mb may fail > > > > F3. cp /tmp/100mb /100mb#new > > > > F4. mv /100mb#new /100mb > > > > > > > > In this way, the second try will only be applied for files that are not > > > > opened at the moment of F1 and we assume that such file is OK to be missing > > > > because there is a time the system is running without it. > > > > > > So I want to tackle essentially two different questions here. > > > > > > (1) Why is this a problem worth solving. E.g. there is clearly additional > > > complexity we could add in merge() to try to make it more efficient, but > > > there are tradeoffs (as we discussed above) and its not clear they are worth > > > it to save disk space; nominally a plentiful resource for most users. So I'm > > > interested in more detail on why this issue is worth addressing. > > > > > > (2) If we decide its worth addressing, can we address it safely? Basically > > > there are many races everywhere. We do a check-reqs to see if there is > > > sufficient space, and if there isn't die (but this is a check-before-use > > > race.) We can unlink the files (as you proposed) and then do a copy, but > > > this is also racey (with other disk consumers.) The races also have dire > > > consequences as pointed out in comment #13; if we lose a race it means we > > > have deleted some file from the rootfs and have not replaced it. This is not > > > acceptable in merge(), IMHO. > > > > > > I think what you end up with is either: > > > HaveEnoughSpace && UnSafeMerge > > > > > > And again we fall back to the idea of "why don't you have enough space, and > > > why are you happy to accept a potentially unsafe merge process?" Why is this > > > a good tradeoff? > > > > > > -A > > > > As for > > > The races also have dire > > > consequences as pointed out in comment #13; if we lose a race it means we > > > have deleted some file from the rootfs and have not replaced it. > > > > The assumption in comment #13, it that if a file is not always opened then > > it is safe to be missing. So the example here is still OK. > > So I think the struggle I have is that your example is rather constrained, > but the general problem is much broader. > > - Usage of rootfs on chromeos is constrained; compared to usage of a rootfs > on normal linux boxes, which is unconstrained. What i mean by this is that > if there is little disk left on /, removing a file and replacing it on > ChromeOS may be a safe operation. However in a generic linux box, it may not > be safe. Other system functions may consume the free'd space and we are back > to this idea that we need to atomically reserve the space. Otherwise its a > yet another race :/ > - Whether a file is opened or not does not determine safety. If you remove > libc6, existing programs will work fine and new programs will crash on > startup. THe existing programs don't have libc6 open (they have opened it, > mapped it into memory, and then closed the FD.) This may be not be a problem > for android-whatever; but its a problem for packages in general. Whether it determines safety depending on how to define safety. My definition of safety is that move_file only do one of the following 1) report a failure 2) move src to dest atomicly 3) delete dest and report a failure if dest is not always opened. By this definition, safety depending on whether a file is opened or not. By this definition, if a file is opened at some moment, we can only do 1) or 2); if a file is not opened at some moment we can do 1) or 2) or 3). > > > > The only situation that the file will be missing is that there is a moment > > during `emerge` in which that file is not opened by anyone. > > > > there is a moment during `emerge` in which that file is not opened by anyone > > => a file is not always opened > > => it is safe to be missing > > I think I'm happier going with a solution where we add something like: > PROPERTIES="unsafe-merge" to the package. This means that for packages like > android-whatever; you just that property and say "hey I don't care about > atomic merging." Most packages (including really important things like > libc6) would not have that property, and would get the normal treatment. > Packages that want to be unsafe can be tagged as unsafe. This also removes > the implicit nature of heuristics and we keep the explicit metadata tagging > for unsafe packages. > > So lets imagine a system where we had the unsafe-merge property for > android-whatever. Would we even bother doing an atomic copy, or would we > just copy directly from ${D} to the rootfs? How would those copies work? > > -A
(In reply to Boleyn Su from comment #23) > (In reply to Alec Warner from comment #20) > > (In reply to Boleyn Su from comment #19) > > > (In reply to Boleyn Su from comment #18) > > > > The use case here is that we run Android on Chrome OS. We want to make it > > > > possible to package Android as a normal portage package that can be emerged > > > > (it is not currently). However, Chrome OS has very limited space on rootfs, > > > > because it is expected that we will not add more packages or data to rootfs > > > > later I guess. Also, we package an Android image (around 1GB) in the > > > > android-whatever.ebuild. These two lead to the problem I describe in this > > > > bug. > > > > > > > > As for the trade-off, I believe with #comment 16, the dangerous outcome > > > > should never happen given that the assumption is correct. Thus, it may be > > > > acceptable to resolve this issue. > > > > > > (In reply to Alec Warner from comment #17) > > > > (In reply to Boleyn Su from comment #16) > > > > > Let me describe it in detail. > > > > > > > > > > We want to cp /tmp/100mb to /100mb. What we do is: > > > > > > > > > > 1. cp /tmp/100mb /100mb#new if failed goto F1 > > > > > 2. rename /100mb#new /100mb succeed > > > > > F1. if /100mb is opened by any process fail > > > > > F2. rm /100mb may fail > > > > > F3. cp /tmp/100mb /100mb#new > > > > > F4. mv /100mb#new /100mb > > > > > > > > > > In this way, the second try will only be applied for files that are not > > > > > opened at the moment of F1 and we assume that such file is OK to be missing > > > > > because there is a time the system is running without it. > > > > > > > > So I want to tackle essentially two different questions here. > > > > > > > > (1) Why is this a problem worth solving. E.g. there is clearly additional > > > > complexity we could add in merge() to try to make it more efficient, but > > > > there are tradeoffs (as we discussed above) and its not clear they are worth > > > > it to save disk space; nominally a plentiful resource for most users. So I'm > > > > interested in more detail on why this issue is worth addressing. > > > > > > > > (2) If we decide its worth addressing, can we address it safely? Basically > > > > there are many races everywhere. We do a check-reqs to see if there is > > > > sufficient space, and if there isn't die (but this is a check-before-use > > > > race.) We can unlink the files (as you proposed) and then do a copy, but > > > > this is also racey (with other disk consumers.) The races also have dire > > > > consequences as pointed out in comment #13; if we lose a race it means we > > > > have deleted some file from the rootfs and have not replaced it. This is not > > > > acceptable in merge(), IMHO. > > > > > > > > I think what you end up with is either: > > > > HaveEnoughSpace && UnSafeMerge > > > > > > > > And again we fall back to the idea of "why don't you have enough space, and > > > > why are you happy to accept a potentially unsafe merge process?" Why is this > > > > a good tradeoff? > > > > > > > > -A > > > > > > As for > > > > The races also have dire > > > > consequences as pointed out in comment #13; if we lose a race it means we > > > > have deleted some file from the rootfs and have not replaced it. > > > > > > The assumption in comment #13, it that if a file is not always opened then > > > it is safe to be missing. So the example here is still OK. > > > > So I think the struggle I have is that your example is rather constrained, > > but the general problem is much broader. > > > > - Usage of rootfs on chromeos is constrained; compared to usage of a rootfs > > on normal linux boxes, which is unconstrained. What i mean by this is that > > if there is little disk left on /, removing a file and replacing it on > > ChromeOS may be a safe operation. However in a generic linux box, it may not > > be safe. Other system functions may consume the free'd space and we are back > > to this idea that we need to atomically reserve the space. Otherwise its a > > yet another race :/ > > - Whether a file is opened or not does not determine safety. If you remove > > libc6, existing programs will work fine and new programs will crash on > > startup. THe existing programs don't have libc6 open (they have opened it, > > mapped it into memory, and then closed the FD.) This may be not be a problem > > for android-whatever; but its a problem for packages in general. > Whether it determines safety depending on how to define safety. My > definition of safety is that move_file only do one of the following > 1) report a failure > 2) move src to dest atomicly > 3) delete dest and report a failure if dest is not always opened. > By this definition, safety depending on whether a file is opened or not. > By this definition, if a file is opened at some moment, we can only do 1) or > 2); if a file is not opened at some moment we can do 1) or 2) or 3). > > > > > > The only situation that the file will be missing is that there is a moment > > > during `emerge` in which that file is not opened by anyone. > > > > > > there is a moment during `emerge` in which that file is not opened by anyone > > > => a file is not always opened > > > => it is safe to be missing > > > > I think I'm happier going with a solution where we add something like: > > PROPERTIES="unsafe-merge" to the package. This means that for packages like > > android-whatever; you just that property and say "hey I don't care about > > atomic merging." Most packages (including really important things like > > libc6) would not have that property, and would get the normal treatment. > > Packages that want to be unsafe can be tagged as unsafe. This also removes > > the implicit nature of heuristics and we keep the explicit metadata tagging > > for unsafe packages. > > > > So lets imagine a system where we had the unsafe-merge property for > > android-whatever. Would we even bother doing an atomic copy, or would we > > just copy directly from ${D} to the rootfs? How would those copies work? > > > > -A How we define safety matters a lot. We can aggressively define safety as `emerge package` is atomic. Then the current emerge is an unsafe implementation, because it is possible that a package is half emerged.
(In reply to Alec Warner from comment #20) > (In reply to Boleyn Su from comment #19) > > (In reply to Boleyn Su from comment #18) > > > The use case here is that we run Android on Chrome OS. We want to make it > > > possible to package Android as a normal portage package that can be emerged > > > (it is not currently). However, Chrome OS has very limited space on rootfs, > > > because it is expected that we will not add more packages or data to rootfs > > > later I guess. Also, we package an Android image (around 1GB) in the > > > android-whatever.ebuild. These two lead to the problem I describe in this > > > bug. > > > > > > As for the trade-off, I believe with #comment 16, the dangerous outcome > > > should never happen given that the assumption is correct. Thus, it may be > > > acceptable to resolve this issue. > > > > (In reply to Alec Warner from comment #17) > > > (In reply to Boleyn Su from comment #16) > > > > Let me describe it in detail. > > > > > > > > We want to cp /tmp/100mb to /100mb. What we do is: > > > > > > > > 1. cp /tmp/100mb /100mb#new if failed goto F1 > > > > 2. rename /100mb#new /100mb succeed > > > > F1. if /100mb is opened by any process fail > > > > F2. rm /100mb may fail > > > > F3. cp /tmp/100mb /100mb#new > > > > F4. mv /100mb#new /100mb > > > > > > > > In this way, the second try will only be applied for files that are not > > > > opened at the moment of F1 and we assume that such file is OK to be missing > > > > because there is a time the system is running without it. > > > > > > So I want to tackle essentially two different questions here. > > > > > > (1) Why is this a problem worth solving. E.g. there is clearly additional > > > complexity we could add in merge() to try to make it more efficient, but > > > there are tradeoffs (as we discussed above) and its not clear they are worth > > > it to save disk space; nominally a plentiful resource for most users. So I'm > > > interested in more detail on why this issue is worth addressing. > > > > > > (2) If we decide its worth addressing, can we address it safely? Basically > > > there are many races everywhere. We do a check-reqs to see if there is > > > sufficient space, and if there isn't die (but this is a check-before-use > > > race.) We can unlink the files (as you proposed) and then do a copy, but > > > this is also racey (with other disk consumers.) The races also have dire > > > consequences as pointed out in comment #13; if we lose a race it means we > > > have deleted some file from the rootfs and have not replaced it. This is not > > > acceptable in merge(), IMHO. > > > > > > I think what you end up with is either: > > > HaveEnoughSpace && UnSafeMerge > > > > > > And again we fall back to the idea of "why don't you have enough space, and > > > why are you happy to accept a potentially unsafe merge process?" Why is this > > > a good tradeoff? > > > > > > -A > > > > As for > > > The races also have dire > > > consequences as pointed out in comment #13; if we lose a race it means we > > > have deleted some file from the rootfs and have not replaced it. > > > > The assumption in comment #13, it that if a file is not always opened then > > it is safe to be missing. So the example here is still OK. > > So I think the struggle I have is that your example is rather constrained, > but the general problem is much broader. > > - Usage of rootfs on chromeos is constrained; compared to usage of a rootfs > on normal linux boxes, which is unconstrained. What i mean by this is that > if there is little disk left on /, removing a file and replacing it on > ChromeOS may be a safe operation. However in a generic linux box, it may not > be safe. Other system functions may consume the free'd space and we are back > to this idea that we need to atomically reserve the space. Otherwise its a > yet another race :/ > - Whether a file is opened or not does not determine safety. If you remove > libc6, existing programs will work fine and new programs will crash on > startup. THe existing programs don't have libc6 open (they have opened it, > mapped it into memory, and then closed the FD.) This may be not be a problem > for android-whatever; but its a problem for packages in general. > > > > > The only situation that the file will be missing is that there is a moment > > during `emerge` in which that file is not opened by anyone. > > > > there is a moment during `emerge` in which that file is not opened by anyone > > => a file is not always opened > > => it is safe to be missing > > I think I'm happier going with a solution where we add something like: > PROPERTIES="unsafe-merge" to the package. This means that for packages like > android-whatever; you just that property and say "hey I don't care about > atomic merging." Most packages (including really important things like > libc6) would not have that property, and would get the normal treatment. > Packages that want to be unsafe can be tagged as unsafe. This also removes > the implicit nature of heuristics and we keep the explicit metadata tagging > for unsafe packages. > > So lets imagine a system where we had the unsafe-merge property for > android-whatever. Would we even bother doing an atomic copy, or would we > just copy directly from ${D} to the rootfs? How would those copies work? > > -A I strongly agree that having different level of safety is a good idea. For example, we may want to make emerge atomic for critical packages like kernel, glibc; the current behavior for most packages; the current behavior + the patch for packages like android-whatever.
Maybe something like this: 1) Add a PROPERTIES=delete-before setting that ebuilds can use to indicate that delete-before behavior is *probably* OK. 2) A user FEATURES=delete-before setting which indicates that delete-before behavior is acceptable. Users have explicit veto power.
The movefile function is not a safe place to implement this feature, since triggering an ENOSPC error can put the whole system into an extremely fragile state. I would suggest to simply have it unmerge the entire previous instance before attempting to merge the new instance.
Looks like the issue is much more complex than I thought and atomic emerge is even much more challenging then the issue I want to solve. I will stick to the workaround for now. However, it would still be nice to have the following to prevent temp file from filling the whole rootfs. + finally: + if _os.path.exists(dest_tmp_bytes): + _os.unlink(dest_tmp_bytes)
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/proj/portage.git/commit/?id=c446394e5b0e71a611ed5cfa0fb923e05802ef5f commit c446394e5b0e71a611ed5cfa0fb923e05802ef5f Author: Zac Medico <zmedico@gentoo.org> AuthorDate: 2020-10-06 01:21:58 +0000 Commit: Zac Medico <zmedico@gentoo.org> CommitDate: 2020-10-06 01:35:09 +0000 movefile: remove dest_tmp_bytes on failure Reported-by: Boleyn Su <boleyn.su@gmail.com> Bug: https://bugs.gentoo.org/745588 Signed-off-by: Zac Medico <zmedico@gentoo.org> lib/portage/util/movefile.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
