A patch to enable setting the build directory.
This adds more flexibility for those who don't have as much space in /var/tmp.
Kind regards,
Jeroen van Meeuwen -kanarip
On Sun, 2007-08-05 at 20:24 +0200, Jeroen van Meeuwen wrote:
A patch to enable setting the build directory.
This adds more flexibility for those who don't have as much space in /var/tmp.
If you don't have space in /var/tmp, just use --tmpdir=/some/other/dir/with/space. Setting the build directory just sets people up for either a) deleting things they don't intend to or b) getting stale data in their build
Jeremy
Jeremy Katz wrote:
On Sun, 2007-08-05 at 20:24 +0200, Jeroen van Meeuwen wrote:
A patch to enable setting the build directory.
This adds more flexibility for those who don't have as much space in /var/tmp.
If you don't have space in /var/tmp, just use --tmpdir=/some/other/dir/with/space. Setting the build directory just sets people up for either a) deleting things they don't intend to or b) getting stale data in their build
Right, tmpdir does evade the i-do-not-have-enough-space-in-/var/tmp issue. Push comes to shovel, I really don't need to set the build_dir to anything I can think of, rather then just use the InstallationTarget.build_dir for any reference after I import livecd-creator and create the extended InstallationTarget instance.
I'm not sure though what you mean by a) and b) though. AFAICT these would never just happen because build_dir === build_dir, regardless of what you set it to, or how you set it.
Kind regards,
Jeroen van Meeuwen -kanarip
On Mon, 2007-08-06 at 23:21 +0200, Jeroen van Meeuwen wrote:
Jeremy Katz wrote:
On Sun, 2007-08-05 at 20:24 +0200, Jeroen van Meeuwen wrote:
A patch to enable setting the build directory.
This adds more flexibility for those who don't have as much space in /var/tmp.
If you don't have space in /var/tmp, just use --tmpdir=/some/other/dir/with/space. Setting the build directory just sets people up for either a) deleting things they don't intend to or b) getting stale data in their build
Right, tmpdir does evade the i-do-not-have-enough-space-in-/var/tmp issue. Push comes to shovel, I really don't need to set the build_dir to anything I can think of, rather then just use the InstallationTarget.build_dir for any reference after I import livecd-creator and create the extended InstallationTarget instance.
This sounds a lot better to me...
I'm not sure though what you mean by a) and b) though. AFAICT these would never just happen because build_dir === build_dir, regardless of what you set it to, or how you set it.
Okay, we don't actually do any removal, so you won't hit that. But if you set a build_dir that already exists, then you're setting up for some "interesting" conditions. By ensuring that it's a new mkdtemp'd dir, we can be certain that we own it, that it's unique, and that no one has done anything with it first
Jeremy
Jeremy Katz wrote:
On Mon, 2007-08-06 at 23:21 +0200, Jeroen van Meeuwen wrote:
Jeremy Katz wrote:
On Sun, 2007-08-05 at 20:24 +0200, Jeroen van Meeuwen wrote:
A patch to enable setting the build directory.
This adds more flexibility for those who don't have as much space in /var/tmp.
If you don't have space in /var/tmp, just use --tmpdir=/some/other/dir/with/space. Setting the build directory just sets people up for either a) deleting things they don't intend to or b) getting stale data in their build
Right, tmpdir does evade the i-do-not-have-enough-space-in-/var/tmp issue. Push comes to shovel, I really don't need to set the build_dir to anything I can think of, rather then just use the InstallationTarget.build_dir for any reference after I import livecd-creator and create the extended InstallationTarget instance.
This sounds a lot better to me...
I'm not sure though what you mean by a) and b) though. AFAICT these would never just happen because build_dir === build_dir, regardless of what you set it to, or how you set it.
Okay, we don't actually do any removal, so you won't hit that. But if you set a build_dir that already exists, then you're setting up for some "interesting" conditions. By ensuring that it's a new mkdtemp'd dir, we can be certain that we own it, that it's unique, and that no one has done anything with it first
So again if we remove the opts, livecd-tools is safe, and Revisor gets to do whatever it wants. Would that be an acceptable patch?
Kind regards,
Jeroen van Meeuwen -kanarip
Jeroen van Meeuwen wrote:
Jeremy Katz wrote:
On Mon, 2007-08-06 at 23:21 +0200, Jeroen van Meeuwen wrote:
Jeremy Katz wrote:
On Sun, 2007-08-05 at 20:24 +0200, Jeroen van Meeuwen wrote:
A patch to enable setting the build directory.
This adds more flexibility for those who don't have as much space in /var/tmp.
If you don't have space in /var/tmp, just use --tmpdir=/some/other/dir/with/space. Setting the build directory just sets people up for either a) deleting things they don't intend to or b) getting stale data in their build
Right, tmpdir does evade the i-do-not-have-enough-space-in-/var/tmp issue. Push comes to shovel, I really don't need to set the build_dir to anything I can think of, rather then just use the InstallationTarget.build_dir for any reference after I import livecd-creator and create the extended InstallationTarget instance.
This sounds a lot better to me...
I'm not sure though what you mean by a) and b) though. AFAICT these would never just happen because build_dir === build_dir, regardless of what you set it to, or how you set it.
Okay, we don't actually do any removal, so you won't hit that. But if you set a build_dir that already exists, then you're setting up for some "interesting" conditions. By ensuring that it's a new mkdtemp'd dir, we can be certain that we own it, that it's unique, and that no one has done anything with it first
Are you actually certain that you don't do any removal?
def teardown(self): if self.build_dir: self.unmount() shutil.rmtree(self.build_dir, ignore_errors = True)
-dmc
On Tue, 2007-08-07 at 03:43 -0500, Douglas McClendon wrote:
Jeroen van Meeuwen wrote:
Jeremy Katz wrote:
On Mon, 2007-08-06 at 23:21 +0200, Jeroen van Meeuwen wrote:
Jeremy Katz wrote:
On Sun, 2007-08-05 at 20:24 +0200, Jeroen van Meeuwen wrote:
A patch to enable setting the build directory.
This adds more flexibility for those who don't have as much space in /var/tmp.
If you don't have space in /var/tmp, just use --tmpdir=/some/other/dir/with/space. Setting the build directory just sets people up for either a) deleting things they don't intend to or b) getting stale data in their build
Right, tmpdir does evade the i-do-not-have-enough-space-in-/var/tmp issue. Push comes to shovel, I really don't need to set the build_dir to anything I can think of, rather then just use the InstallationTarget.build_dir for any reference after I import livecd-creator and create the extended InstallationTarget instance.
This sounds a lot better to me...
I'm not sure though what you mean by a) and b) though. AFAICT these would never just happen because build_dir === build_dir, regardless of what you set it to, or how you set it.
Okay, we don't actually do any removal, so you won't hit that. But if you set a build_dir that already exists, then you're setting up for some "interesting" conditions. By ensuring that it's a new mkdtemp'd dir, we can be certain that we own it, that it's unique, and that no one has done anything with it first
Are you actually certain that you don't do any removal?
Indeed there is -- I thought there was, but then my quick look yesterday didn't see it. That really makes this a bit (okay, a lot) scary as it's basically a quick path to nuking your system which is something that we try pretty hard to be careful to not do.
And since you can just make use of the build dir after instantiating, I think the risk is more than the gain
Jeremy
Douglas McClendon wrote:
Jeroen van Meeuwen wrote:
Jeremy Katz wrote:
On Mon, 2007-08-06 at 23:21 +0200, Jeroen van Meeuwen wrote:
Jeremy Katz wrote:
On Sun, 2007-08-05 at 20:24 +0200, Jeroen van Meeuwen wrote:
A patch to enable setting the build directory.
This adds more flexibility for those who don't have as much space in /var/tmp.
If you don't have space in /var/tmp, just use --tmpdir=/some/other/dir/with/space. Setting the build directory just sets people up for either a) deleting things they don't intend to or b) getting stale data in their build
Right, tmpdir does evade the i-do-not-have-enough-space-in-/var/tmp issue. Push comes to shovel, I really don't need to set the build_dir to anything I can think of, rather then just use the InstallationTarget.build_dir for any reference after I import livecd-creator and create the extended InstallationTarget instance.
This sounds a lot better to me...
I'm not sure though what you mean by a) and b) though. AFAICT these would never just happen because build_dir === build_dir, regardless of what you set it to, or how you set it.
Okay, we don't actually do any removal, so you won't hit that. But if you set a build_dir that already exists, then you're setting up for some "interesting" conditions. By ensuring that it's a new mkdtemp'd dir, we can be certain that we own it, that it's unique, and that no one has done anything with it first
Are you actually certain that you don't do any removal?
def teardown(self): if self.build_dir: self.unmount() shutil.rmtree(self.build_dir, ignore_errors = True)
Right on.
There's an addition problem with having tempfile.mkdtemp(), and that is that mounts can not easily be unmounted in a next run, if the creation of live media fails in one way or the other. Having a "static" build_dir, it can be used to check for existance, and be unmounted. You'd not be running out of loop devices anymore. If the directory already exists, the app complains and exits appropriately, or allows the user to act upon it and save his data.
Kind regards,
Jeroen van Meeuwen -kanarip
On Tue, 2007-08-07 at 23:28 +0200, Jeroen van Meeuwen wrote:
There's an addition problem with having tempfile.mkdtemp(), and that is that mounts can not easily be unmounted in a next run, if the creation of live media fails in one way or the other. Having a "static" build_dir, it can be used to check for existance, and be unmounted. You'd not be running out of loop devices anymore. If the directory already exists, the app complains and exits appropriately, or allows the user to act upon it and save his data.
What/how are you expecting the user to do on the failure? Why not teardown immediately on failure (or at least, before you throw away the object you had)? Outside of cases where we're currently traceback'ing and not catching it[1], livecd-creator should be cleaning up after itself on error pretty much all the time and thus not running out of loop devices
Jeremy
[1] Hopefully a dwindling number... it seems to be, but if there are other cases people hit, let me know either by filing a bug or sending a patch!
Jeremy Katz wrote:
On Tue, 2007-08-07 at 23:28 +0200, Jeroen van Meeuwen wrote:
There's an addition problem with having tempfile.mkdtemp(), and that is that mounts can not easily be unmounted in a next run, if the creation of live media fails in one way or the other. Having a "static" build_dir, it can be used to check for existance, and be unmounted. You'd not be running out of loop devices anymore. If the directory already exists, the app complains and exits appropriately, or allows the user to act upon it and save his data.
What/how are you expecting the user to do on the failure? Why not teardown immediately on failure (or at least, before you throw away the object you had)? Outside of cases where we're currently traceback'ing and not catching it[1], livecd-creator should be cleaning up after itself on error pretty much all the time and thus not running out of loop devices
True. On the other hand one may need/want to inspect the tree, in our case we usually do. If everything is tried and excepted, teardown on failure should be optional, or requested feedback upon in the form of a dialog:
"We failed and now want to clean up. Do you want to inspect the tree first? Navigate there and there and knock yourself out."
"Now, do you want to keep this tree for further investigation? [y/N]: "
Kind regards,
Jeroen van Meeuwen -kanarip
On Wed, 2007-08-08 at 12:59 +0200, Jeroen van Meeuwen wrote:
Jeremy Katz wrote:
On Tue, 2007-08-07 at 23:28 +0200, Jeroen van Meeuwen wrote:
There's an addition problem with having tempfile.mkdtemp(), and that is that mounts can not easily be unmounted in a next run, if the creation of live media fails in one way or the other. Having a "static" build_dir, it can be used to check for existance, and be unmounted. You'd not be running out of loop devices anymore. If the directory already exists, the app complains and exits appropriately, or allows the user to act upon it and save his data.
What/how are you expecting the user to do on the failure? Why not teardown immediately on failure (or at least, before you throw away the object you had)? Outside of cases where we're currently traceback'ing and not catching it[1], livecd-creator should be cleaning up after itself on error pretty much all the time and thus not running out of loop devices
True. On the other hand one may need/want to inspect the tree, in our case we usually do. If everything is tried and excepted, teardown on failure should be optional, or requested feedback upon in the form of a dialog:
"We failed and now want to clean up. Do you want to inspect the tree first? Navigate there and there and knock yourself out."
"Now, do you want to keep this tree for further investigation? [y/N]: "
And in this case, you just say "the tree is at /path/to/build_dir. be sure to clean up afterwards by running the following". Since you're going to have to say where it is _anyway_. And if they say they don't want to inspect it, then you can still have the object around to do the cleanup.
Given the scariness of removing directories that are important to the system and the fact that there are other ways around the clean up case, I'm really having a hard time justifying this to myself.
Jeremy
livecd@lists.fedoraproject.org
-
Douglas McClendon -
Jeremy Katz -
Jeroen van Meeuwen