6:49 p.m.
repo: git://git.baserock.org/baserock/baserock/morph
ref: baserock/richardipsum/write_kernel
head: 95e5845ed1b846915f196939359466a5490f93e1
Richard Ipsum (3):
Add ssh keys conf ext
More info in msg
Add option to write out kernel image
morphlib/exts/kvm.write | 3 ++-
morphlib/exts/ssh-keys.configure | 28 ++++++++++++++++++++++++++++
morphlib/writeexts.py | 9 +++++++++
3 files changed, 39 insertions(+), 1 deletion(-)
create mode 100755 morphlib/exts/ssh-keys.configure
--
1.7.10.4
6:49 p.m.
New subject: [PATCH 1/3] Add ssh keys conf ext
---
morphlib/exts/ssh-keys.configure | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
create mode 100755 morphlib/exts/ssh-keys.configure
diff --git a/morphlib/exts/ssh-keys.configure b/morphlib/exts/ssh-keys.configure
new file mode 100755
index 0000000..dc0b629
--- /dev/null
+++ b/morphlib/exts/ssh-keys.configure
@@ -0,0 +1,28 @@
+#!/bin/sh
+#
+# Copyright 2014 Codethink Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+set -e
+
+if [ "$SSHKEYS" ]
+then
+ mkdir -p "$1/root/.ssh"
+
+ for key in $SSHKEYS
+ do
+ cat "$key" >> "$1/root/.ssh/authorized_keys"
+ done
+fi
--
1.7.10.4
7:11 p.m.
New subject: [PATCH 1/3] Add ssh keys conf ext
On Thu, May 29, 2014 at 05:49:38PM +0100, Richard Ipsum wrote:
+set -e
+
+if [ "$SSHKEYS" ]
+then
+ mkdir -p "$1/root/.ssh"
+
+ for key in $SSHKEYS
+ do
+ cat "$key" >> "$1/root/.ssh/authorized_keys"
+ done
+fi
--
1.7.10.4
What this does is simple enough that I'd have preferred a more
specific name: set-root-ssh-authorized-keys.configure or such.
However, we can live with this name.
The code might have been clearer like this:
if [ -s "$SSHKEYS" ]
then
install -D -m 0700 "$1/root/.ssh"
cat $SSHKEYS >> "$1/root/.ssh/authorized_keys"
fi
However, the current code is fine, as far as I can see, though I would
like to see the mkdir replaced with install, to get the permissions
right and the setting of them to be explicit.
--
http://www.codethink.co.uk/ http://wiki.baserock.org/ http://www.baserock.com/
7:17 p.m.
New subject: [PATCH 1/3] Add ssh keys conf ext
On Thu, May 29, 2014 at 06:11:49PM +0100, Lars Wirzenius wrote:
On Thu, May 29, 2014 at 05:49:38PM +0100, Richard Ipsum wrote:
> +set -e
> +
> +if [ "$SSHKEYS" ]
> +then
> + mkdir -p "$1/root/.ssh"
> +
> + for key in $SSHKEYS
> + do
> + cat "$key" >> "$1/root/.ssh/authorized_keys"
> + done
> +fi
> --
> 1.7.10.4
What this does is simple enough that I'd have preferred a more
specific name: set-root-ssh-authorized-keys.configure or such.
However, we can live with this name.
The code might have been clearer like this:
if [ -s "$SSHKEYS" ]
then
install -D -m 0700 "$1/root/.ssh"
cat $SSHKEYS >> "$1/root/.ssh/authorized_keys"
fi
However, the current code is fine, as far as I can see, though I would
like to see the mkdir replaced with install, to get the permissions
right and the setting of them to be explicit.
I disagree, the loop isn't needed at all and I'd be ashamed to merge it.
6:49 p.m.
New subject: [PATCH 2/3] More info in msg
---
morphlib/exts/kvm.write | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/morphlib/exts/kvm.write b/morphlib/exts/kvm.write
index 94a55da..e45dc5b 100755
--- a/morphlib/exts/kvm.write
+++ b/morphlib/exts/kvm.write
@@ -89,7 +89,8 @@ class KvmPlusSshWriteExtension(morphlib.writeexts.WriteExtension):
def transfer(self, raw_disk, ssh_host, vm_path):
'''Transfer raw disk image to libvirt host.'''
- self.status(msg='Transferring disk image')
+ self.status(msg='Transferring disk image to %s on %s' %
+ (vm_path, ssh_host))
target = '%s:%s' % (ssh_host, vm_path)
with open(raw_disk, 'rb') as f:
cliapp.runcmd(['rsync', '-szS', raw_disk, target])
--
1.7.10.4
7:13 p.m.
New subject: [PATCH 2/3] More info in msg
The code seems good, but the commit message might have been clearer
and say that this makes the message about the disk image being
transferred clearer by adding source and destination paths.
--
http://www.codethink.co.uk/ http://wiki.baserock.org/ http://www.baserock.com/
6:49 p.m.
New subject: [PATCH 3/3] Add option to write out kernel image
---
morphlib/writeexts.py | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/morphlib/writeexts.py b/morphlib/writeexts.py
index b4912db..1d72554 100644
--- a/morphlib/writeexts.py
+++ b/morphlib/writeexts.py
@@ -121,6 +121,11 @@ class WriteExtension(cliapp.Application):
try:
self.create_btrfs_system_layout(
temp_root, mp, version_label='factory')
+
+ kernel_path = os.environ.get('KERNEL')
+
+ if kernel_path:
+ self.write_kernel_image(temp_root, kernel_path)
except BaseException, e:
sys.stderr.write('Error creating Btrfs system layout')
self.unmount(mp)
@@ -240,6 +245,10 @@ class WriteExtension(cliapp.Application):
self.install_syslinux_menu(mountpoint, version_root)
self.install_extlinux(mountpoint)
+ def write_kernel_image(self, temp_root, path):
+ self.status(msg='Writing kernel image to %s' % path)
+ shutil.copyfile(os.path.join(temp_root, 'boot', 'vmlinuz'),
path)
+
def create_orig(self, version_root, temp_root):
'''Create the default "factory" system.'''
--
1.7.10.4
7:14 p.m.
New subject: [PATCH 3/3] Add option to write out kernel image
This could do with a justification in the commit message for when this
is useful.
On Thu, May 29, 2014 at 05:49:40PM +0100, Richard Ipsum wrote:
+ def write_kernel_image(self, temp_root, path):
+ self.status(msg='Writing kernel image to %s' % path)
+ shutil.copyfile(os.path.join(temp_root, 'boot', 'vmlinuz'),
path)
Is the kernel always in vmlinuz form, and not in, say, uImage form?
--
http://www.codethink.co.uk/ http://wiki.baserock.org/ http://www.baserock.com/
12:09 p.m.
New subject: [PATCH 3/3] Add option to write out kernel image
On Thu, May 29, 2014 at 05:49:40PM +0100, Richard Ipsum wrote:
---
morphlib/writeexts.py | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/morphlib/writeexts.py b/morphlib/writeexts.py
index b4912db..1d72554 100644
--- a/morphlib/writeexts.py
+++ b/morphlib/writeexts.py
@@ -121,6 +121,11 @@ class WriteExtension(cliapp.Application):
try:
self.create_btrfs_system_layout(
temp_root, mp, version_label='factory')
+
+ kernel_path = os.environ.get('KERNEL')
+
+ if kernel_path:
+ self.write_kernel_image(temp_root, kernel_path)
This treats `KERNEL: ''` and an absent option as the same.
Granted, it probably wouldn't make sense to use most of those options,
but I'd rather odd inputs be treated as errors rather than the same as
being absent.
So I'd prefer:
if 'KERNEL' in os.environ:
self.write_kernel_image(temp_root, os.environ['KERNEL'])
However, it's not a big problem, and I don't mind it in its current
state too much.
7:15 p.m.
On Thu, May 29, 2014 at 05:49:37PM +0100, Richard Ipsum wrote:
repo: git://git.baserock.org/baserock/baserock/morph
ref: baserock/richardipsum/write_kernel
head: 95e5845ed1b846915f196939359466a5490f93e1
Overall, this looks OK to me, though I had some nitpicks. Those can be
fixed or not, depending on how urgent this is to merge. +1.
--
http://www.codethink.co.uk/ http://wiki.baserock.org/ http://www.baserock.com/
3479
days inactive
3480
days old
9 comments
3 participants
participants (3)
-
Lars Wirzenius -
Richard Ipsum -
Richard Maw