In response to Jan Kratochvil's shell script I have attached a python script which replicates what she does in the bash shell script.
There is also a patch, which fixes up some regex/match in livecd-iso-to-disk and another section which assumes our loop devices have extra numbered partitions.
If you guys want to help beat mkbiarch.py Python script into shape maybe we gan get it into the tools for livecd-tools.
The way you use it, is: mkbiarch.py <x86 ISO image> <x64 ISO image> <Target Multi Arch Image name>
On Mon, Aug 16, 2010 at 7:56 PM, Jasper Hartline jasper.hartline@gmail.com wrote:
In response to Jan Kratochvil's shell script I have attached a python script which replicates what she does in the bash shell script.
There is also a patch, which fixes up some regex/match in livecd-iso-to-disk and another section which assumes our loop devices have extra numbered partitions.
If you guys want to help beat mkbiarch.py Python script into shape maybe we gan get it into the tools for livecd-tools.
The way you use it, is: mkbiarch.py <x86 ISO image> <x64 ISO image> <Target Multi Arch Image name>
I checked out the mkbiarch script and it looks good! I don't have the requisite ISO's on hand to test it though.
I'm trying to brush up on my Python after a hiatus so pardon if my comments don't make sense.
It's probably overkill but I know there's the optparse module for passing arguments and options. Not really necessary since the script requires exactly 3 arguments but it would add somewhat automagically things like --help and --version.
Is there any difference between os.stat().st_size and os.path.getsize()? Do we care if one of the argument references is a symbolic link? I think it's just personal preference in coding style but I tend to use more high level functions like os.path.isfile() versus os.stat()
It looks like as this is a largely procedural style script that there is some things done twice (once for the 32bit image and once for the 64bit image) which could be generalized in a function.
Richard
I think it could use quite a bit of comments because it took me a while to figure out what did what and I still don't see where the two images are combined but then again I'm not familiar with losetup.
On Mon, Aug 16, 2010 at 7:56 PM, Richard Shaw hobbes1069@gmail.com wrote:
It's probably overkill but I know there's the optparse module for passing arguments and options. Not really necessary since the script requires exactly 3 arguments but it would add somewhat automagically things like --help and --version.
Right, these should be added.
Is there any difference between os.stat().st_size and os.path.getsize()?
Not one I can think of, they both would require the os module. Maybe there is something better in Python 2.7
Do we care if one of the argument references is a symbolic link? I think it's just personal preference in coding style but I tend to use more high level functions like os.path.isfile() versus os.stat()
I just threw it together, so little things like that can and need to be changed.
It looks like as this is a largely procedural style script that there is some things done twice (once for the 32bit image and once for the 64bit image) which could be generalized in a function.
That is true. I will see if I can do some more hacking on it.. Any patches are welcome.
I think it could use quite a bit of comments because it took me a while to figure out what did what and I still don't see where the two images are combined but then again I'm not familiar with losetup.
Well. The most basic explanation is that by using losetup on a file, we can make an image which can be written to a disk without actually having a physical disk.
The script first makes a file which is the combined size of the x86 ISO and the x64 ISO using dd. That file is then loopbacked using losetup.
We call our existing livecd-iso-to-disk which does some stuff and isn't exactly perfect for the purpose. In fact I'm not sure how to get it to be better without doing some of it all over again, instead of how livecd-iso-to-disk does.
livecd-iso-to-disk basically will format and partition the loopback device (incorrectly I think, which is all based on it being a loopdevice, and the fact it may be initializing it with some weird geometry it doesn't like)
Then it copies all the osmin.img and squashfs.img files to the loop device (which is a file on the hard drive, remember)
We close it all up. We run livecd-iso-to-disk on the second ISO, which does the same thing without the --format or --reset-mbr options. Unfortunately I think parted is still getting called on it messing it up somehow, I am not sure yet. I need to look at that.
We create a syslinux.cfg with the ifcpu64.c32 comboot module from syslinux and close it up. The result is a target Image File to be written to disk. I would like it to be in a format that can just be dd'd.
I need to see where I am faouling up because it seems livecd-iso-to-disk is stepping on my partition table from --reset-mbr and --format command.
Take a look at this: http://autopsy.liveprojects.info/repo/SOURCES/c-section/live_generator/insta... Just Edit -> Find in this page -> "kpartx"
That is a way to use dmsetup and kpartx (device mapper) to actually make the loop device have partitions which we are more used to using.
I think my offset into the original file is off in the second loop device setup.
On Tue, Aug 17, 2010 at 1:19 AM, Jasper Hartline jasper.hartline@gmail.com wrote:
On Mon, Aug 16, 2010 at 7:56 PM, Richard Shaw hobbes1069@gmail.com wrote:
It's probably overkill but I know there's the optparse module for passing arguments and options. Not really necessary since the script requires exactly 3 arguments but it would add somewhat automagically things like --help and --version.
Right, these should be added.
I can help there (when I have time) if it's not something you're familiar with, but I would rather wait for any updates you're already thinking about to make the 'patch' easier.
Is there any difference between os.stat().st_size and os.path.getsize()?
Not one I can think of, they both would require the os module. Maybe there is something better in Python 2.7
I did some research in the python docs and there isn't any difference but it recommended the os.path method unless you're trying to get more than one attribute of a file to prevent multiple stat() calls.
Do we care if one of the argument references is a symbolic link? I think it's just personal preference in coding style but I tend to use more high level functions like os.path.isfile() versus os.stat()
I just threw it together, so little things like that can and need to be changed.
I think it needs to be changed to os.path.getsize(). I tried os.stat() on a directory and as I expected it returned true. Some other checking may also be a good idea, like is it an ISO. I'm not sure how you prove that but checking for a .iso extension would be easy.
It looks like as this is a largely procedural style script that there is some things done twice (once for the 32bit image and once for the 64bit image) which could be generalized in a function.
That is true. I will see if I can do some more hacking on it.. Any patches are welcome.
I'll see what I can do.
[SNIP]
I need to see where I am faouling up because it seems livecd-iso-to-disk is stepping on my partition table from --reset-mbr and --format command.
Take a look at this: http://autopsy.liveprojects.info/repo/SOURCES/c-section/live_generator/insta... Just Edit -> Find in this page -> "kpartx"
That is a way to use dmsetup and kpartx (device mapper) to actually make the loop device have partitions which we are more used to using.
I think my offset into the original file is off in the second loop device setup.
I checked out the link and it obviously uses a lot of shell commands. I think at some point a shell script would be better than a bunch of subprocess calls, however, I think much of it could be mitigated by using modules that allow us to get the same result without the subprocess calls. I haven't had time to check it out, but I saw a reference to pyparted which might allow us to do the 'partitioning' of the lodev natively.
Richard
On Tue, Aug 17, 2010 at 7:07 AM, Richard Shaw hobbes1069@gmail.com wrote:
On Tue, Aug 17, 2010 at 1:19 AM, Jasper Hartline
That is a way to use dmsetup and kpartx (device mapper) to actually make the loop device have partitions which we are more used to using.
I think my offset into the original file is off in the second loop device setup.
I checked out the link and it obviously uses a lot of shell commands. I think at some point a shell script would be better than a bunch of subprocess calls, however, I think much of it could be mitigated by using modules that allow us to get the same result without the subprocess calls. I haven't had time to check it out, but I saw a reference to pyparted which might allow us to do the 'partitioning' of the lodev natively.
Yeah. Pretty much I'll just use what I have as a skeleton I suppose. pyparted. I was just thinking how many Python modules.. I could use. Is it worth doing completely in Python using a few modules?
So far I've come to the conclusion livecd-iso-to-disk needs large bits hacked on it or.. I could duplicate some of what livecd-iso-to-disk does in the mkbiarch.py script.
Here's the problem with using livecd-iso-to-disk: It thinks you have a real block device It tries to add "1" to "dev" so "sda" becomes "sda1" for example. It doesn't like using device mapper mapped devices.. found that out last night. (/dev/dm-1 for example which is /dev/mapper/loop0p1)
The existing bug in livecd-iso-to-disk where checkFilesystem() is after a specific section in the code, makes it always default to mkfs.vfat regardless of partition type.
I think some other reasons why I will probably write mkbiarch.py to just do what it needs bypassing livecd-iso-to-disk.
On Tue, Aug 17, 2010 at 7:07 AM, Richard Shaw hobbes1069@gmail.com wrote:
It's probably overkill but I know there's the optparse module for passing arguments and options. Not really necessary since the script requires exactly 3 arguments but it would add somewhat automagically things like --help and --version.
Right, these should be added.
Ok. Does this look better, I have attached what I have rewritten. Needs to have the optparse stuff.
On Tue, Aug 17, 2010 at 1:06 PM, Jasper Hartline jasper.hartline@gmail.com wrote:
On Tue, Aug 17, 2010 at 7:07 AM, Richard Shaw hobbes1069@gmail.com wrote:
It's probably overkill but I know there's the optparse module for passing arguments and options. Not really necessary since the script requires exactly 3 arguments but it would add somewhat automagically things like --help and --version.
Right, these should be added.
Ok Richard. I don't know where the pyparted docs are. If you know how to partition devices, with pyparted I am more than glad to listen.
Here is what I have now, attached. In setup() After I mkimage() and do losetup() I need to partition that device, which will be a /dev/loop device.
Any suggestions?
On Tue, Aug 17, 2010 at 7:38 PM, Jasper Hartline jasper.hartline@gmail.com wrote:
On Tue, Aug 17, 2010 at 1:06 PM, Jasper Hartline jasper.hartline@gmail.com wrote:
On Tue, Aug 17, 2010 at 7:07 AM, Richard Shaw hobbes1069@gmail.com wrote:
It's probably overkill but I know there's the optparse module for passing arguments and options. Not really necessary since the script requires exactly 3 arguments but it would add somewhat automagically things like --help and --version.
Right, these should be added.
Ok Richard. I don't know where the pyparted docs are. If you know how to partition devices, with pyparted I am more than glad to listen.
Here is what I have now, attached. In setup() After I mkimage() and do losetup() I need to partition that device, which will be a /dev/loop device.
Any suggestions?
I also found the documentation to be lacking for pyparted. It may be out there, but I couldn't find it easily. I did, however find a module inspector recepie. Attached is the output for pyparted (package name) / parted (module name).I'll try to take a look at it tonight and see what I can grok.
I did some updating to the script but I'll merge it with what you just sent before posting it back to the list. (In other words, when I get a chance after the kids go to bed :) )
Richard
On Tue, Aug 17, 2010 at 5:43 PM, Richard Shaw hobbes1069@gmail.com wrote:
I also found the documentation to be lacking for pyparted. It may be out there, but I couldn't find it easily. I did, however find a module inspector recepie. Attached is the output for pyparted (package name) / parted (module name).I'll try to take a look at it tonight and see what I can grok.
I did some updating to the script but I'll merge it with what you just sent before posting it back to the list. (In other words, when I get a chance after the kids go to bed :) )
Awesome awesome.
I think that txt doc might help.
On Tue, Aug 17, 2010 at 8:17 PM, Jasper Hartline jasper.hartline@gmail.com wrote:
I did some updating to the script but I'll merge it with what you just sent before posting it back to the list. (In other words, when I get a chance after the kids go to bed :) )
I didn't get a chance to review the pyparted doc but here's the updated script. It's completely untested but the framework is there.
I put in a bunch of inline comments. I decided it was better to communicate my changes there instead of in the email. You can comment back here or there or delete them if you understand what I was trying to do and agree. I probably should have made a diff patch as well but I didn't save a copy of the original and I'm to lazy to do it now :) I don't think most of it was drastic.
Richard
On Tue, Aug 17, 2010 at 8:39 PM, Richard Shaw hobbes1069@gmail.com wrote:
I put in a bunch of inline comments. I decided it was better to communicate my changes there instead of in the email. You can comment back here or there or delete them if you understand what I was trying to do and agree. I probably should have made a diff patch as well but I didn't save a copy of the original and I'm to lazy to do it now :) I don't think most of it was drastic.
def mount(src, dst, options=None): if os.path.exists(src): # Is src the lodev device?
mount is a generic mount procedure, src can be a file, a loop device.. and ISO, a disk. Anything really.
if options: # None is the same as Null so you can test for existence instead.
I'm pretty sure if options: without making options=None will cause and IndexErroor. So I probably should just test for thing: when I set offset=None or options=None in the function.
else: # Let's make sure only one return statement get's triggered. return False # Might as well return something useful so we can test for failure.
This is sort of debateable. I mean, if the mount operation fails, we can't really do anything to correct it. In other words, it should succeed, else return nothing. I am pretty sure return will just return nothing.
else: print "Something is wrong, %s has no size." % ent return False
I think this is good, you never know when the file you are trying to use may not be the right one.
def blocks(block_size, size): # Round up to nearest block # Make sure floating math, not integer math or we don't get remainders. # Turn back into an integer for return statement. return int(math.ceil(size / float(block_size)))
I'm not sure why we would need this. Any idea?
def setup(x86, x64, multi): # Reworked the logic a bit. block_size = 2048 # Should this be a global constant instead? sz = size(x86) + size(x64) count = blocks(block_size, sz)
multi = mkimage(str(blsz), count) losetup(multi, lo())
block size is a constant, in the scope of setup() which is about the only place we would need to use it. Unless that changes. I'm using "2048"
def parse(x86, x64, multi): for file in x86, x64, multi: # Do we expect that multi exists yet? if os.path.exists(file): # Should we test for existance or isfile? pass else: parser.error("One of the images does not exist.") setup(x86, x64, multi)
multi is our end-result target file. This begins with a temporary file, that is then partitioned and formatted. Copied to, and closed up eventually, then shutil.move(tmpfile, multi)
Maybe both. if os.path.exixts() and if os.path.isfile() Then again, there shouldn't be a reason we should disallow use of hardlinked ISO image files or soft linked ones. Have any logic behind preventing use of the files if they are links and not files?
try: # Any reason this would fail? parse(args[0], args[1], args[2]) except: parser.error("Something failed. Better luck next time!") sys.exit(1)
Yes, when the files don't exist it will fail with an IndexError on sys.argv[]
I think I might also do something different with the syslinux.cfg stuff. Instead of having the huge text data as a content "" and filled. Instead read the ISOLINUX.CFG and use re.search and re.match To find what we need, then concatenate using line indexes from readlines().
Can contents = fd.readlines() be written to? Do you know? Is the content of contents static?
Still looking at the module investigator text you sent. It is hard to tell how to do simple things with that module.
All we need to do is, partition it.. type 0x83 Start cylinder 1 or 63, and end, up to the end of the file. Then, write that partition table. Format the partition. Move to mounting our ISO files and doing extraction of the UUID and other things. Copy the files to the partition.
Close it up.
On Wed, Aug 18, 2010 at 12:04 AM, Jasper Hartline jasper.hartline@gmail.com wrote:
On Tue, Aug 17, 2010 at 8:39 PM, Richard Shaw hobbes1069@gmail.com wrote:
I put in a bunch of inline comments. I decided it was better to communicate my changes there instead of in the email. You can comment back here or there or delete them if you understand what I was trying to do and agree. I probably should have made a diff patch as well but I didn't save a copy of the original and I'm to lazy to do it now :) I don't think most of it was drastic.
def mount(src, dst, options=None): if os.path.exists(src): # Is src the lodev device?
mount is a generic mount procedure, src can be a file, a loop device.. and ISO, a disk. Anything really.
Ok, so maybe later we'll need to know what type of file/device it is and we can add tests for it if necessary.
if options: # None is the same as Null so you can test for existence instead.
I'm pretty sure if options: without making options=None will cause and IndexErroor. So I probably should just test for thing: when I set offset=None or options=None in the function.
Sorry, I may have confused you with my shorthand comment. Yes, we'll still need the "None" default in the function definition. I just changed the logic so instead of testing "if not" (i.e. False) condition I'm testing for the if True condition.
I try to stick with 'if <True>' conditions if you're going to do something one way or the other. I use 'if not <True>' when if something is False then I do something and if it's True I don't do anything. Does that make sense? I'm not sure I'm explaining my logic well.
else: # Let's make sure only one return statement get's triggered. return False # Might as well return something useful so we can test for failure.
This is sort of debateable. I mean, if the mount operation fails, we can't really do anything to correct it. In other words, it should succeed, else return nothing. I am pretty sure return will just return nothing.
Yeah, I did some research last night and there are people on both sides of the "one return statement" or "multiple return statements in a function" camps. Also now that I re-read your comment I don't think you were arguing that :) Sorry, sick kid == little sleep.
To answer what I think you're commenting about, yes, I agree. So since python is going to see the same thing (None=False), I think the question is, if there's a chance of others (hopefully several others) helping with this, does the explicit False statement aid readability?
Now that I think about it, we should probably figure out what the best practice is for returning an error if the function is not successful.
else: print "Something is wrong, %s has no size." % ent return False
I think this is good, you never know when the file you are trying to use may not be the right one.
def blocks(block_size, size): # Round up to nearest block # Make sure floating math, not integer math or we don't get remainders. # Turn back into an integer for return statement. return int(math.ceil(size / float(block_size)))
I'm not sure why we would need this. Any idea?
I have some experience in this area as I'm now the discspan (check it out on freshmeat or sourceforge) maintainer. ISO 9660 also has 2k sectors so in order to properly estimate the size you have to round up to the nearest sector per file. I'm guessing we're only doing this for the whole image, but it's the same idea. No matter how small a file is it at least has to take up one sector (optical media) or one block (file systems).
Integer math will drop remainders (it doesn't even round) so for 11 / 3 the result would be 3, not 3.83 or 4. Making any one of the arguments a floating point number trips the whole thing into floating point math. Since we're stuck allocating a whole block we can't round down so that's where the ceiling function comes in.
def setup(x86, x64, multi): # Reworked the logic a bit. block_size = 2048 # Should this be a global constant instead? sz = size(x86) + size(x64) count = blocks(block_size, sz)
multi = mkimage(str(blsz), count) losetup(multi, lo())
block size is a constant, in the scope of setup() which is about the only place we would need to use it. Unless that changes. I'm using "2048"
In that case we should probably just capitalize the variable (per the PEP) so: BLOCK_SIZE = 2048...
def parse(x86, x64, multi): for file in x86, x64, multi: # Do we expect that multi exists yet? if os.path.exists(file): # Should we test for existance or isfile? pass else: parser.error("One of the images does not exist.") setup(x86, x64, multi)
multi is our end-result target file. This begins with a temporary file, that is then partitioned and formatted. Copied to, and closed up eventually, then shutil.move(tmpfile, multi)
Ok, obviously the code isn't fully flushed out but it looks like currently the test for existence happens before it's created.
Maybe both. if os.path.exixts() and if os.path.isfile() Then again, there shouldn't be a reason we should disallow use of hardlinked ISO image files or soft linked ones. Have any logic behind preventing use of the files if they are links and not files?
As long as they're usable/accessible I guess it shouldn't matter, but if a ISO is referenced we can feel pretty good that it's going to work, if it's something else, like the image mounted on /dev/sr0, does that change how we do anything? For one, the files will be already accessible unlike an ISO which we would have to mount first.
I think depends on how far we want to get in the first "release". It would be OK with me to require the sources be ISO's at this point for simplicity sake...
try: # Any reason this would fail? parse(args[0], args[1], args[2]) except: parser.error("Something failed. Better luck next time!") sys.exit(1)
Yes, when the files don't exist it will fail with an IndexError on sys.argv[]
Well I'm already testing that there are exactly 3 arguments during option parsing so parse() itself should never fail, now since it calls other functions that may fail it kinda make sense. I am concerned though that we're essentially wrapping the entire program in a 'try' statement, which I'm guessing is probably not a good programming practice. It could also mask problems we should be handling, right?
I think I might also do something different with the syslinux.cfg stuff. Instead of having the huge text data as a content "" and filled. Instead read the ISOLINUX.CFG and use re.search and re.match To find what we need, then concatenate using line indexes from readlines().
Can contents = fd.readlines() be written to? Do you know? Is the content of contents static?
I think so as it returns a list. I just made sure it doesn't return a generator.
Still looking at the module investigator text you sent. It is hard to tell how to do simple things with that module.
All we need to do is, partition it.. type 0x83 Start cylinder 1 or 63, and end, up to the end of the file. Then, write that partition table. Format the partition. Move to mounting our ISO files and doing extraction of the UUID and other things. Copy the files to the partition.
Close it up.
livecd mailing list livecd@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/livecd
On Wed, Aug 18, 2010 at 6:51 AM, Richard Shaw hobbes1069@gmail.com wrote:
def mount(src, dst, options=None): if os.path.exists(src): # Is src the lodev device?
mount is a generic mount procedure, src can be a file, a loop device.. and ISO, a disk. Anything really.
Ok, so maybe later we'll need to know what type of file/device it is and we can add tests for it if necessary.
True. If it becomes more complicated python-magic will check teh file magic.
if options: # None is the same as Null so you can test for existence instead.
I'm pretty sure if options: without making options=None will cause and IndexErroor. So I probably should just test for thing: when I set offset=None or options=None in the function.
Sorry, I may have confused you with my shorthand comment. Yes, we'll still need the "None" default in the function definition. I just changed the logic so instead of testing "if not" (i.e. False) condition I'm testing for the if True condition.
I try to stick with 'if <True>' conditions if you're going to do something one way or the other. I use 'if not <True>' when if something is False then I do something and if it's True I don't do anything. Does that make sense? I'm not sure I'm explaining my logic well.
I understand. I think I wrote it in the foresight that we would possibly only call it once with any options. Therefore, when a reader is reading it and they see if options: They expect options are expected. If I write if not options: Also this is just semantical I think. If I write the upper blocks to be code we don't use much.. I suppesa options would work too. I will go back through it once I get the base case working.
else: # Let's make sure only one return statement get's triggered. return False # Might as well return something useful so we can test for failure.
This is sort of debateable. I mean, if the mount operation fails, we can't really do anything to correct it. In other words, it should succeed, else return nothing. I am pretty sure return will just return nothing.
Yeah, I did some research last night and there are people on both sides of the "one return statement" or "multiple return statements in a function" camps. Also now that I re-read your comment I don't think you were arguing that :) Sorry, sick kid == little sleep.
test return return
Is easy to read. Python is based in it's blocks on spacing. So we tend to lean towards the indentation to read Python more or less.
To answer what I think you're commenting about, yes, I agree. So since python is going to see the same thing (None=False), I think the question is, if there's a chance of others (hopefully several others) helping with this, does the explicit False statement aid readability?
Now that I think about it, we should probably figure out what the best practice is for returning an error if the function is not successful.
I was hoping not to have to test for too many errors. The only ones I can think of would be You are not root. You can't mount. All of your loop devices are already used, can't get one. The files you provided on the command line don't exist.
So far anyhow.
def blocks(block_size, size): # Round up to nearest block # Make sure floating math, not integer math or we don't get remainders. # Turn back into an integer for return statement. return int(math.ceil(size / float(block_size)))
I'm not sure why we would need this. Any idea?
I have some experience in this area as I'm now the discspan (check it out on freshmeat or sourceforge) maintainer. ISO 9660 also has 2k sectors so in order to properly estimate the size you have to round up to the nearest sector per file.
Hm. we divide (x86size + x64size) by block_size -> 2048 to get count for passing to dd. The way I understand it, mkisofs pads the ISO up to the last 2048 divisible block. There shouldn't be a reason why a ISO file made with mkisofs is not divisible by 2048.
def parse(x86, x64, multi): for file in x86, x64, multi: # Do we expect that multi exists yet? if os.path.exists(file): # Should we test for existance or isfile? pass else: parser.error("One of the images does not exist.") setup(x86, x64, multi)
multi is our end-result target file. This begins with a temporary file, that is then partitioned and formatted. Copied to, and closed up eventually, then shutil.move(tmpfile, multi)
Ok, obviously the code isn't fully flushed out but it looks like currently the test for existence happens before it's created.
Yeah I noticed that. I want to set it, but not test for it and produce an error or usage.
Maybe both. if os.path.exixts() and if os.path.isfile() Then again, there shouldn't be a reason we should disallow use of hardlinked ISO image files or soft linked ones. Have any logic behind preventing use of the files if they are links and not files?
As long as they're usable/accessible I guess it shouldn't matter, but if a ISO is referenced we can feel pretty good that it's going to work, if it's something else, like the image mounted on /dev/sr0, does that change how we do anything? For one, the files will be already accessible unlike an ISO which we would have to mount first.
I think depends on how far we want to get in the first "release". It would be OK with me to require the sources be ISO's at this point for simplicity sake...
Yeah right now we want to keep it simpler until we get the base case working. 2 ISO files, one target which is multiarch.
try: # Any reason this would fail? parse(args[0], args[1], args[2]) except: parser.error("Something failed. Better luck next time!") sys.exit(1)
Yes, when the files don't exist it will fail with an IndexError on sys.argv[]
Well I'm already testing that there are exactly 3 arguments during option parsing so parse() itself should never fail, now since it calls other functions that may fail it kinda make sense. I am concerned though that we're essentially wrapping the entire program in a 'try' statement, which I'm guessing is probably not a good programming practice. It could also mask problems we should be handling, right?
Yes, I did this on purpose, in the foresight to be run from cron or as a nightly operation.
I think I might also do something different with the syslinux.cfg stuff. Instead of having the huge text data as a content "" and filled. Instead read the ISOLINUX.CFG and use re.search and re.match To find what we need, then concatenate using line indexes from readlines().
Can contents = fd.readlines() be written to? Do you know? Is the content of contents static?
I think so as it returns a list. I just made sure it doesn't return a generator.
Hm. Right. so no string replace operations on a list, bummer. But, we could create a new list out of the old.
Read data we want from the old list, create a new list, with the new inputs and write it to the file.
As far as keeping it simpler for now, you can see I've done that by saying we need to partition the lodevivce type 0x83 We can expand this later.
On Wed, Aug 18, 2010 at 5:14 PM, Jasper Hartline jasper.hartline@gmail.com wrote:
On Wed, Aug 18, 2010 at 6:51 AM, Richard Shaw hobbes1069@gmail.com wrote:
def blocks(block_size, size): # Round up to nearest block # Make sure floating math, not integer math or we don't get remainders. # Turn back into an integer for return statement. return int(math.ceil(size / float(block_size)))
I'm not sure why we would need this. Any idea?
I have some experience in this area as I'm now the discspan (check it out on freshmeat or sourceforge) maintainer. ISO 9660 also has 2k sectors so in order to properly estimate the size you have to round up to the nearest sector per file.
Hm. we divide (x86size + x64size) by block_size -> 2048 to get count for passing to dd. The way I understand it, mkisofs pads the ISO up to the last 2048 divisible block. There shouldn't be a reason why a ISO file made with mkisofs is not divisible by 2048.
Ok, I guess I'm confused here. If mkisofs pads the data, why do we have to calculate the number of 2048k blocks at all? If we do need to know the number of 2048k blocks then we need floating point math since integer math will drop the remainder.
Maybe once the code is flushed out more I'll see exactly what's going on.
def parse(x86, x64, multi): for file in x86, x64, multi: # Do we expect that multi exists yet? if os.path.exists(file): # Should we test for existance or isfile? pass else: parser.error("One of the images does not exist.") setup(x86, x64, multi)
multi is our end-result target file. This begins with a temporary file, that is then partitioned and formatted. Copied to, and closed up eventually, then shutil.move(tmpfile, multi)
Ok, obviously the code isn't fully flushed out but it looks like currently the test for existence happens before it's created.
Yeah I noticed that. I want to set it, but not test for it and produce an error or usage.
In that case we only need to test for existence of the two source images which could be done in an if statement:
if os.path.exists(x86) and os.path.exists(x64): ...
Maybe both. if os.path.exixts() and if os.path.isfile() Then again, there shouldn't be a reason we should disallow use of hardlinked ISO image files or soft linked ones. Have any logic behind preventing use of the files if they are links and not files?
As long as they're usable/accessible I guess it shouldn't matter, but if a ISO is referenced we can feel pretty good that it's going to work, if it's something else, like the image mounted on /dev/sr0, does that change how we do anything? For one, the files will be already accessible unlike an ISO which we would have to mount first.
I think depends on how far we want to get in the first "release". It would be OK with me to require the sources be ISO's at this point for simplicity sake...
Yeah right now we want to keep it simpler until we get the base case working. 2 ISO files, one target which is multiarch.
try: # Any reason this would fail? parse(args[0], args[1], args[2]) except: parser.error("Something failed. Better luck next time!") sys.exit(1)
Yes, when the files don't exist it will fail with an IndexError on sys.argv[]
Well I'm already testing that there are exactly 3 arguments during option parsing so parse() itself should never fail, now since it calls other functions that may fail it kinda make sense. I am concerned though that we're essentially wrapping the entire program in a 'try' statement, which I'm guessing is probably not a good programming practice. It could also mask problems we should be handling, right?
Yes, I did this on purpose, in the foresight to be run from cron or as a nightly operation.
It probably works as a good bandaid for now, but we could probably create a "check_requirements" or some other named function that would make sure all the bash commands are available and the arguments as passed are acceptable.
I think I might also do something different with the syslinux.cfg stuff. Instead of having the huge text data as a content "" and filled. Instead read the ISOLINUX.CFG and use re.search and re.match To find what we need, then concatenate using line indexes from readlines().
Can contents = fd.readlines() be written to? Do you know? Is the content of contents static?
I think so as it returns a list. I just made sure it doesn't return a generator.
Hm. Right. so no string replace operations on a list, bummer. But, we could create a new list out of the old.
Read data we want from the old list, create a new list, with the new inputs and write it to the file.
Either that or have a separate source file that we can substitute the proper variables into before writing. I guess it depends if we want it to be the same as one of the source cfg files except what we need to be different or if we'd rather specify what it looks like.
As far as keeping it simpler for now, you can see I've done that by saying we need to partition the lodevivce type 0x83 We can expand this later.
I guess I need clarification on what the objective is here. I can see 2 or 3 possible paths.
One would be a script that takes two live CD ready images (ISOs) and creates a multiarch ISO out of it. This would not necessarily care about partitioning since the live-cd-iso-to-disk should be able to handle it.
Another would be a script that only cared about creating a bootable partition (regardless of whether it's a hard disk or USB partition).
The third would be a script that would handle both situations natively without relying on the livecd-iso-to-disk script.
What's your thoughts there?
Richard
On Wed, Aug 18, 2010 at 7:15 PM, Richard Shaw hobbes1069@gmail.com wrote:
Hm. we divide (x86size + x64size) by block_size -> 2048 to get count for passing to dd. The way I understand it, mkisofs pads the ISO up to the last 2048 divisible block. There shouldn't be a reason why a ISO file made with mkisofs is not divisible by 2048.
Ok, I guess I'm confused here. If mkisofs pads the data, why do we have to calculate the number of 2048k blocks at all? If we do need to know the number of 2048k blocks then we need floating point math since integer math will drop the remainder.
Just so we can pass a block size of 2048 to dd, and have the count we need of the number of blocks instead of using bs=1 count=sizeof ISO files. The file dd creates will need to at least be as large as the two ISO files combined.
Ok, obviously the code isn't fully flushed out but it looks like currently the test for existence happens before it's created.
Yeah I noticed that. I want to set it, but not test for it and produce an error or usage.
In that case we only need to test for existence of the two source images which could be done in an if statement:
if os.path.exists(x86) and os.path.exists(x64):
Just changed that earlier:
def parse(x86, x64, multi): for file in x86, x64: if os.path.exists(file): pass else: usage() if not multi: usage() setup(x86, x64, multi)
usage() is also not all we want to do here, but for now we exit.. so yeah it isn't done yet.
It probably works as a good bandaid for now, but we could probably create a "check_requirements" or some other named function that would make sure all the bash commands are available and the arguments as passed are acceptable.
Exactly, some people may be running these scripts from git, from cron. Who knows.
Hm. Right. so no string replace operations on a list, bummer. But, we could create a new list out of the old.
Read data we want from the old list, create a new list, with the new inputs and write it to the file.
Either that or have a separate source file that we can substitute the proper variables into before writing. I guess it depends if we want it to be the same as one of the source cfg files except what we need to be different or if we'd rather specify what it looks like.
I thought about that too. Which may work for the immediate case but creating a new list will make it more elegant, and keep the number of files required to run the operation to 1.
As far as keeping it simpler for now, you can see I've done that by saying we need to partition the lodevivce type 0x83 We can expand this later.
I guess I need clarification on what the objective is here. I can see 2 or 3 possible paths.
One would be a script that takes two live CD ready images (ISOs) and creates a multiarch ISO out of it. This would not necessarily care about partitioning since the live-cd-iso-to-disk should be able to handle it.
No. I won't be using livecd-iso-to-disk because it was written to do some things in a way I don't feel like patching to correct for using on a kpartx/device-mapper loopback file to get something simple done.
Instead I can dd an image file, loopback it, partition it format it at an offset starting on partition 1. losetup with an offset , mount ISO files, copy files, write syslinux.cfg and install syslinux to it. Close it up and you have the resulting target.
I was looking at livecd-iso-to-disk long enough to say it wasn't worth the trouble to patch it, just to move along with development on this one.
livecd-iso-to-disk has some problems, which can be fixed, but I'm not sure how soon. Most of the patches to livecd-iso-to-disk sent by Frederick Grose are in the original posting of this thread.
After those are merged and the support for syslinux on Ext4, and btrfs which requires syslinux 4.02 (F14) there is also a bug which is fixed by calling checkFilesystem() at the correct time, to make livecd-creator not default to using vfat and claiming your ext2/ext3 partitioned disk needs to be vfat or ext2/3 when it already is.
https://admin.fedoraproject.org/pkgdb/acls/bugs/livecd-tools?_csrf_token=e07...
Another would be a script that only cared about creating a bootable partition (regardless of whether it's a hard disk or USB partition).
I don't know what this means. Grub2 can boot ISO files directly, which is neat, but also isn't syslinux.
The third would be a script that would handle both situations natively without relying on the livecd-iso-to-disk script.
The option last is the one we want for this mkbiarch.py script. As I said, the earlier script was for to be used as a skeleton. As a side effect I have decided to drop using livecd-iso-to-disk. It needs it's holes plugged.. it was written to be used to write this ISO to a real disk, and since that is the case, it just doesn't work like it should for what we are doing here in mkbiarch.py which won't write anything to any disk, only make the image file.. so far.
On Wed, Aug 18, 2010 at 11:00 PM, Jasper Hartline jasper.hartline@gmail.com wrote:
On Wed, Aug 18, 2010 at 7:15 PM, Richard Shaw hobbes1069@gmail.com wrote:
Hm. we divide (x86size + x64size) by block_size -> 2048 to get count for passing to dd. The way I understand it, mkisofs pads the ISO up to the last 2048 divisible block. There shouldn't be a reason why a ISO file made with mkisofs is not divisible by 2048.
Ok, I guess I'm confused here. If mkisofs pads the data, why do we have to calculate the number of 2048k blocks at all? If we do need to know the number of 2048k blocks then we need floating point math since integer math will drop the remainder.
Just so we can pass a block size of 2048 to dd, and have the count we need of the number of blocks instead of using bs=1 count=sizeof ISO files. The file dd creates will need to at least be as large as the two ISO files combined.
Ok, I understand what you're doing now, but I still think I'm right. If you use straight integer math, it does not round up for the last block. It does not round at all, it just drops the remainder. The easiest solution probably is to just add 1 to whatever size / BLOCK_SIZE returns.
Instead I can dd an image file, loopback it, partition it format it at an offset starting on partition 1. losetup with an offset , mount ISO files, copy files, write syslinux.cfg and install syslinux to it. Close it up and you have the resulting target.
Actually, I think we can partition it first and maybe even create the file system, then loop mount it to write the files. I've had some partial success with using pyparted (see below).
--- I used this[1] as a partial guide.
I created a disk image from the shell: # dd if=/dev/zero of=disk_test.img bs=1 count=1 seek=100M 1+0 records in 1+0 records out 1 byte (1 B) copied, 4.6739e-05 s, 21.4 kB/s
Then brought up python: # python Python 2.6.4 (r264:75706, Jun 4 2010, 18:20:31) [GCC 4.4.4 20100503 (Red Hat 4.4.4-2)] on linux2 Type "help", "copyright", "credits" or "license" for more information.
import parted parted.getDevice(r'/root/disk_test.img')
<parted.device.Device object at 0x2568490>
disk = parted.getDevice(r'/root/disk_test.img') print disk
parted.Device instance -- model: path: /root/disk_test.img type: 5 sectorSize: 512 physicalSectorSize: 512 length: 204800 openCount: 0 readOnly: False externalMode: False dirty: False bootDirty: False host: 0 did: 0 busy: False hardwareGeometry: (1600, 4, 32) biosGeometry: (1600, 4, 32) PedDevice: <_ped.Device object at 0x24ad950>
type(disk)
<class 'parted.device.Device'>
disk.getSize()
100.0
disk1 = parted.Disk(disk)
Traceback (most recent call last): File "<stdin>", line 1, in <module> File "<string>", line 2, in __init__ File "/usr/lib64/python2.6/site-packages/parted/decorators.py", line 31, in localeC ret = fn(*args, **kwds) File "/usr/lib64/python2.6/site-packages/parted/disk.py", line 51, in __init__ self.__disk = _ped.Disk(device.getPedDevice()) _ped.DiskLabelException: /root/disk_test.img: unrecognised disk label
Now I haven't found the method to create a disk label. There is a pyparted mailing list so we may have to go there for help.
Richard
[1] http://www.infohit.net/blog/post/virtual-disk-image-tools.html
Ok, here's where I am with pyparted:
# python Python 2.6.4 (r264:75706, Jun 4 2010, 18:20:31) [GCC 4.4.4 20100503 (Red Hat 4.4.4-2)] on linux2 Type "help", "copyright", "credits" or "license" for more information.
import parted device = parted.Device("/root/disk_test.img") disk = parted.freshDisk(device, 'msdos')
There's no reference to the "freshDisk" method that I could find the the parted module. I found it by downloading the source for anaconda and digging through the code.
Now we have a disk image with a blank msdos partition table. Now we need to add the primary partition.
Richard
On Thu, Aug 19, 2010 at 3:04 PM, Richard Shaw hobbes1069@gmail.com wrote:
Ok, here's where I am with pyparted:
# python Python 2.6.4 (r264:75706, Jun 4 2010, 18:20:31) [GCC 4.4.4 20100503 (Red Hat 4.4.4-2)] on linux2 Type "help", "copyright", "credits" or "license" for more information.
import parted device = parted.Device("/root/disk_test.img") disk = parted.freshDisk(device, 'msdos')
There's no reference to the "freshDisk" method that I could find the the parted module. I found it by downloading the source for anaconda and digging through the code.
Now we have a disk image with a blank msdos partition table. Now we need to add the primary partition.
Right. Well. We need to somehow get the maximum length a partition can be in the file. If we do get this information, I suspect start, end here are sector numbers.. we can do that rather easily. Note that in the file below, sdb is being used.. not a file yet. It is just my local testing thing which will add a partition properly. Unfortunately we need to make start and end with the right values. I've seen Gemetry.getMaxGeometry I think in the text you sent.. but after trying to get some geometry or a maximum start and end sector a partition can be I have still not been able to come up with the right function. The code below will add a partition though.
#!/usr/bin/python
import os import sys import parted
device = parted.Device(path="/dev/sdb") disk = parted.Disk(device=device)
start = 1 end =
new_geom = parted.Geometry(device=device, start=start, end=end)
partition = parted.Partition(disk=disk, type=82, geometry=new_geom)
constraint = parted.Constraint(exactGeom=new_geom)
disk.addPartition(partition=partition, constraint=constraint)
disk.commit()
On Thu, Aug 19, 2010 at 3:20 PM, Jasper Hartline jasper.hartline@gmail.com wrote:
On Thu, Aug 19, 2010 at 3:04 PM, Richard Shaw hobbes1069@gmail.com wrote:
Ok, here's where I am with pyparted:
# python Python 2.6.4 (r264:75706, Jun 4 2010, 18:20:31) [GCC 4.4.4 20100503 (Red Hat 4.4.4-2)] on linux2 Type "help", "copyright", "credits" or "license" for more information.
import parted device = parted.Device("/root/disk_test.img") disk = parted.freshDisk(device, 'msdos')
There's no reference to the "freshDisk" method that I could find the the parted module. I found it by downloading the source for anaconda and digging through the code.
Well, this looks like it is going to do it: [root@localhost tmp]# fdisk -l /dev/loop0
Disk /dev/loop0: 102 MB, 102400000 bytes 4 heads, 32 sectors/track, 1562 cylinders Units = cylinders of 128 * 512 = 65536 bytes Sector size (logical/physical): 512 bytes / 512 bytes I/O size (minimum/optimal): 512 bytes / 512 bytes Disk identifier: 0x0000804e
Device Boot Start End Blocks Id System /dev/loop0p1 1 1563 99999+ 83 Linux [root@localhost tmp]#
Here is how I decide to get the number of sectors: #!/usr/bin/python
import os import sys import parted
size = os.stat("/tmp/file").st_size sectors = size / 512 - 1
device = parted.Device(path="/dev/loop0") disk = parted.freshDisk(device, 'msdos')
start = 1 end = sectors
new_geom = parted.Geometry(device=device, start=start, end=end)
partition = parted.Partition(disk=disk, type=82, geometry=new_geom)
constraint = parted.Constraint(exactGeom=new_geom)
disk.addPartition(partition=partition, constraint=constraint)
disk.commit()
I'll keep workking with this and update you with the latest, as soon as I get more done.
On Thu, Aug 19, 2010 at 11:15 PM, Jasper Hartline jasper.hartline@gmail.com wrote:
On Thu, Aug 19, 2010 at 3:20 PM, Jasper Hartline
I'll keep workking with this and update you with the latest, as soon as I get more done.
Well. I have failed to decrypt the FileSystem class for pyparted. It seems you create a format object and then pass that to Partition, like this.. below however, upon looking at the loop device and using hexdump, and dumpe2fs and trying to mount it, etc. It seems it is corrupt or is not a valid filesystem.
So I'm not sure what is going on. Also, to get the size we can just use size divided by 512. That gives us the maximum number of sectors minus 1 for the MBR.
#!/usr/bin/python
import os import sys import parted
size = os.stat("/tmp/file").st_size sectors = size / 512 - 1
device = parted.Device(path="/dev/loop0") disk = parted.freshDisk(device, 'msdos')
start = 2 end = sectors
new_geom = parted.Geometry(device=device, start=start, end=end)
format = parted.FileSystem(type="ext2", geometry=new_geom).create()
partition = parted.Partition(disk=disk, fs=format, type=82, geometry=new_geom)
constraint = parted.Constraint(exactGeom=new_geom)
disk.addPartition(partition=partition, constraint=constraint)
disk.commit()
So here attached is what I have so far, it is what I would like to continue to work on and fox the format routing when we figure out how to use pyparted properly.
So far I have it setting up correctly and the master boot record is proper. /dev/loop0 on /tmp/tmprjsmh3 type ext3 (rw) /dev/loop1 on /tmp/tmpKTCs75 type iso9660 (rw) /dev/loop2 on /tmp/tmpLi76ER type iso9660 (rw) [root@localhost tmp]# ls -l /tmp/tmprjsmh3 total 28 drwx------ 2 root root 16384 Aug 20 03:54 lost+found drwxr-xr-x 2 root root 4096 Aug 20 03:54 syslinux drwxr-xr-x 2 root root 4096 Aug 20 03:54 x64 drwxr-xr-x 2 root root 4096 Aug 20 03:54 x86 [root@localhost tmp]#
On Thu, Aug 19, 2010 at 13:14:21 -0500, Richard Shaw hobbes1069@gmail.com wrote:
Ok, I understand what you're doing now, but I still think I'm right. If you use straight integer math, it does not round up for the last block. It does not round at all, it just drops the remainder. The easiest solution probably is to just add 1 to whatever size / BLOCK_SIZE returns.
The normal sematic for this kind of thing with integer division is: (size + BLOCKSIZE - 1) / BLOCK_SIZE
(Unless integer overflows are possible.)
Otherwise if size is a multiple of BLOCK_SIZE the result would be 1 too large.
On Tue, Aug 17, 2010 at 8:39 PM, Richard Shaw hobbes1069@gmail.com wrote:
I didn't get a chance to review the pyparted doc but here's the updated script. It's completely untested but the framework is there.
I'm not sure I'm understanding this module well. Says device = parted.Device(path="/dev/sdb") has no attribute getPedPartition()
I wonder what getPedPartition is supposed to be.
[root@localhost tmp]# ./partition.py Traceback (most recent call last): File "./partition.py", line 14, in <module> part = d.addPartition(disk, device) File "<string>", line 2, in addPartition File "/usr/lib/python2.6/site-packages/parted/decorators.py", line 31, in localeC ret = fn(*args, **kwds) File "/usr/lib/python2.6/site-packages/parted/disk.py", line 250, in addPartition result = self.__disk.add_partition(partition.getPedPartition()) AttributeError: 'Device' object has no attribute 'getPedPartition' [root@localhost tmp]# cat partition.py #!/usr/bin/python
import os import sys import parted
device = parted.Device(path="/dev/sdb") disk = parted.Disk(device=device)
d = parted.Disk part = d.addPartition(disk, device) disk.commit()
[root@localhost tmp]#
On Tue, Aug 17, 2010 at 22:39:14 -0500, Richard Shaw hobbes1069@gmail.com wrote:
I put in a bunch of inline comments. I decided it was better to communicate my changes there instead of in the email. You can comment back here or there or delete them if you understand what I was trying to do and agree. I probably should have made a diff patch as well but I didn't save a copy of the original and I'm to lazy to do it now :) I don't think most of it was drastic.
I committed this, so that we have a good way to do shared work on it.
It won't be used by any existing stuff, so shouldn't cause problems.
livecd@lists.fedoraproject.org
-
Bruno Wolff III -
Jasper Hartline -
Richard Shaw