Bug#643585: Bug 643585
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 09/29/2011 11:21 PM, Ben Howard wrote:
> 0001-Added-Ubuntu-Cloud-package-lists.patch: - Adds the Ubuntu Cloud
> package list definitions - Package lists are the official packages
> for the Ubuntu Cloud images compatible with Maverick and later
* contains useless whitespaces at EOL
* please sort the lists: first tasks (stronger set first, then
weaker; means, e.g. standard before server), second the packages
alphabetically. and it probably would make it more readable if you
group them (means, between tasks and packages an empty line) and
add a comment '# Tasks' and '# Packages' on top of the group.
* please include the standard blurb at the beginning common to all
package lists.
* please deduplicate 'ubuntu-cloud' with the minimal package list,
using '#<include <foo>' mechanism.
* please deduplicate 'ubuntu-cloud-desktop' with the 'ubuntu-cloud'
one, using '#<include <foo>' mechanism.
otherwise good.
> 0002-Fixed-rootfs-chroot-and-added-fs-label-support.patch
this patch is too big; let me reiterate, any patch should be *one thing*
only. the rational should be: every individual commit should be
revertable on it's own.
> - Fixed an issue where mkfs.ext? failed due to missing mtab while
> formating file system in chroot
you're creating an /etc/mtab->/proc/mounts symlink, if /etc/mtab is not
already existing. which is fine, but when do you revert that change
again? the auxiliary chroot should not be tainted (gives some problems
for ensuring its sanity over multiple builds and makes testing for
regressions hard).
> - Added file system label support to rootfs for ext? file systems
but lb already supports using LB_HDD_LABEL.
> - Added /etc/fstab based on labels (allow rootfs to be used
> independent of binary wrapper)
that changes does two things: first, it creates an fstab per se, which
is not desireable in the live case. so, we need a 'generic' flag first
that we can call lb with that defines if the resulting system is a live
system or not. i'll add '--system live|normal' for that in git in a
moment, so you can check for LB_SYSTEM=normal and only then create the
fstab.
second, your sample fstab looks strange:
* you're adding an entry for /proc, but /proc (like /sys and others)
is automatically mounted through sysvinit anyway, and does not need
nor should be added to the fstab.
this might or might not be true for upstart, if it's not true for
upstart, then that entry should conditionally be added upon
LB_INITSYSTEM=upstart only.
* you're adding a write_fstab();, please remember coding style:
- function names start with capital letter
- intending is off
- use 'cat > FILE << EOF' instead of 'cat << EOF > FILE',
look at other uses of 'cat ... EOF' with respect to
empty line seperators before and after.
* the 'Generated by live-build' comment uselessly clutters the file.
* why LABEL and not UUID? why '0 0' instead of '0 1'?
* you imported a typo '-FF' in one of the mkfs calls.
otherwise good.
> 0003-Added-lb_binary_image-to-emit-standalone-rootfs-imag.patch:
too big, please break it into smaller patches.
> - Adds lb_binary_image to emit the chroot rootfs a file system image
> named <LABEL>.<FS TYPE> - Provides the ability to build images
> suitable for running - For uses where the raw file system is needed
this should go through the hdd image type (see note at the end).
> 0004-Enablement-for-foreign-bootstrap-via-qemu-static.patch: Probably
> the most controversial of my patches - Adds foreign bootstrapping
> ability via "qemu-<ARCH>-static"
* useless whitespace at EOL
* typo in comment: s/foriegn/foreign/
* unclean diff, has spurious changes in it due to mixed up
indenting of existing code.
* inconsistent indenting of newly added code
(the 'Run appropriate bootstrap [...' block)
* you're adding a new config/ports file, i'd use the
existing config/bootstrap for that. after all, this all is
about bootstrapping.
i don't really understand yet the whole foreign bootstrapping thing with
qemu, so symantically i can't yet say much, i'll have to actually
try and run the code at some later point.
> 0005-Set-default-armel-linux-flavour-for-Ubuntu.patch: - Set default
> armel kernel flavour for Ubuntu to omap
applied, thanks, with slightly modified commit message though: commit
messages are supposed to be full and proper sentences, means, they end
with a stop. and we're tryting to always use the '-ing' form, like
'Fixing/Correcting/Adding $whatever." instead of 'Fix Correct/Add
$whatever.'.
> If there is interest in supporting more complicated warm file
> systems/images, I can do some work on porting those features.
that would be awesome, i was meaning to get to do that myself during the
last ~1.5 months, but never got arround to it (see 'Providing official
virtualization images' thread on debian-devel end of july 2011).
> The concern (and perhaps it is wrong) is that live-build is focused
> on building bootable systems that use live-config, where cloud
> images generally do not have nor need live-config.
live-build is intended to build *any* system image, regardless if it's a
live one or not, live-build just doesn't announce itself that way (yet).
General note about binary images types again:
The idea of live-build is to generate different image types, such as iso
images, hdd images, or netboot tarballs.
images like you want to do are hdd images, without partition table, and
without live packages installed (the latter being addressed through
- --system live|normal). so, rather than to add an other image type, code
wise, the existing hdd image type should be extended to make yours possible.
therefore, like i said in one of my mails before, we should handle such
a new 'hdd-raw' image type as a sub-type to the hdd image type, just as
iso-hybrid to iso is handled atm. please have a look at lb_binary_iso.
now, you'll say that you want to generate multiple binary images in one
run. well, we want that too, we just didn't got arround to it (as you
see, the resp. parameter is already in the plural form: --binary-images).
the idea is that once we have support for multiple binary images types,
you can e.g. do: lb config -b 'iso iso-hybrid hdd hdd-raw net' and
you'll get 5 binary images as output, a regular iso image, a isohybrid
image, a hdd image with partitions, a raw hdd image without
partitioning, and a netboot tarball.
keeping this stuff together that way ensure that we can keep the build
time as minimal as possible by eliminating double work and not copying
stuff multiple times over and over again.
hope that makes it somewhat more clear.
Regards,
Daniel
- --
Address: Daniel Baumann, Donnerbuehlweg 3, CH-3012 Bern
Email: daniel.baumann@progress-technologies.net
Internet: http://people.progress-technologies.net/~daniel.baumann/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk6E9JcACgkQ+C5cwEsrK54MEwCghEmQ4JzWZU2vzW060l+u+vzy
x3gAoMBsMdURY1eEmmgrqIfkDeCpgM6T
=jW9I
-----END PGP SIGNATURE-----
Reply to: