Code review comment for lp:~openstack-ubuntu-packagers/ubuntu/natty/nova/ubuntu-natty-packaging-changes

On 18-11-2010 17:42, Chuck Short wrote:
> Chuck Short has proposed merging lp:~openstack-ubuntu-packagers/ubuntu/natty/nova/ubuntu-natty-packaging-changes into lp:~openstack-ubuntu-packagers/ubuntu/natty/nova/ubuntu.
>
> Requested reviews:
> OpenStack Ubuntu packagers (openstack-ubuntu-packagers)
>
>
> natty debian changes

Please be more verbose about the changes you're making. This is the
Natty packaging branch. If you want to change it, it's rather obvious
that they're natty changes. :)

> === modified file 'debian/changelog'
> --- debian/changelog 2010-11-18 15:35:00 +0000
> +++ debian/changelog 2010-11-18 16:42:00 +0000
> @@ -1,10 +1,22 @@
> nova (2011.1~bzr403-0ubuntu1) UNRELEASED; urgency=low
>
> + [Soren Hansen]
> * New upstream snapshot.
> * Added the new tarballs page to debian/watch.
> * Clean out patch-branches (everything is upstream now).
> * Remove redis-server as a build-depends and don't start it for tests
> anymore.
> +
> + [Chuck Short]
> + * debian/control:
> + - Add option to use user-mode-linux

"Add UML templates to flagfile" would be better.

> + - Add python-greenlet as a build dependency.

Why?

> + - Update maintainer according to spec.

a) The spec deals with packages that we import from Debian.
b) If the Maintainer already is an @ubuntu.com address, you don't have
to change it.
It's ok that you did, it's just the log entry that's off.

> + - Add dependency to python-rrdtool so that nova-instancemonitor doesnt complain about missing python modules
> + when starting.
> + * debian/nova-common.install: Add missing templates.

> + * debian/nova-*.conf: Update configs due to changes to nova-api.

Not all the changes you made to the conf files are due to change in
nova-api. They're due to all sorts of things.

> - python-sqlalchemy, python-eventlet, python-routes
> + python-sqlalchemy, python-eventlet, python-routes, python-greenlet

Why?

> === modified file 'debian/nova-api.conf'
> --- debian/nova-api.conf 2010-11-18 15:16:24 +0000
> +++ debian/nova-api.conf 2010-11-18 16:42:00 +0000
> @@ -6,3 +6,7 @@
> --logfile=/var/log/nova/nova-api.log
> --sql_connection=sqlite:////var/lib/nova/nova.sqlite
> --FAKE_subdomain=ec2
> +--libvirt_xml_template=/usr/share/nova/libvirt.qemu.xml.template
> +--libvirt_uml_xml_template=/usr/share/nova/libvirt.uml.xml.template
> +--libvirt_rescue_xml_template=/usr/share/nova/libvirt.rescue.qemu.xml.template
> +--libvirt_rescue_uml_xml_template=/usr/share/nova/libvirt.rescue.uml.xml.template

This is fine, but soon won't be needed anymore, just FYI. The API server
doesn't use them, so it's a bug that it needs them there.

> === removed file 'debian/start-redis'
> --- debian/start-redis 2010-11-18 15:16:24 +0000
> +++ debian/start-redis 1970-01-01 00:00:00 +0000
> @@ -1,21 +0,0 @@
> -#!/bin/sh
> -
> -port=6379
> -maxport=6400
> -while netstat -ln | grep -q :$port.*LISTEN
> -do
> - if [ $port -eq $maxport ]
> - then
> - exit 1
> - fi
> - port=$(($port + 1))
> -done
> -
> -sed -e 's!^save!#&!g' \
> - -e 's!^dir .*!dir '"$PWD"'!g' \
> - -e 's!^pidfile .*!pidfile '"$PWD"'/redis.pid!g' \
> - -e 's!^logfile .*!logfile /dev/stdout!g' \
> - -e 's!^port .*!port '$port'!g' \
> - /etc/redis/redis.conf | redis-server -
> -echo $port
> -exit 0
>

Good catch.

  vote needsfixing

--
Soren Hansen
Ubuntu Developer http://www.ubuntu.com/
OpenStack Developer http://www.openstack.org/