Code review comment for lp:~xnox/ubuntu/quantal/lvm2/merge95

> - debian/{clvmd.ra,clvm.init}:
> - create /var/run/lvm if it doesn't exist.

debian/rules now points at /run/lvm; this should be updated to match.

--- debian/libdevmapper-dev.install 2010-12-07 08:08:45 +0000
+++ debian/libdevmapper-dev.install 2012-08-18 04:00:45 +0000
@@ -1,5 +1,2 @@
-usr/include/libdevmapper.h
-usr/include/libdevmapper-event.h
-usr/lib/libdevmapper.so
-usr/lib/pkgconfig/devmapper.pc
-usr/lib/pkgconfig/devmapper-event.pc
+usr/include/libdevmapper*
+usr/lib/*/pkgconfig/devmapper*

Hmm, good catch. I think you should call this out as a separate change in the changelog, since this isn't just a "remaining change" but a fix to multiarch support for the runtime lib package.

--- debian/lvm2.postinst 2009-10-08 18:17:43 +0000
+++ debian/lvm2.postinst 2012-08-15 17:05:32 +0000
@@ -5,7 +5,9 @@
 case "$1" in
     configure)
         vgcfgbackup >/dev/null 2>&1 || :
- invoke-rc.d lvm2 start || :
+ if [ -x /etc/init.d/lvm2 ]; then
+ invoke-rc.d lvm2 start || :
+ fi
         if [ -x /usr/sbin/update-initramfs ]; then
             update-initramfs -u
         fi

Since the lvm2.init is being dropped, this probably shouldn't be conditional - it should probably be removed entirely. In fact, when this regression was introduced during the lucid merge, the previous postinst was doing this:

    if test -f /etc/init.d/lvm2; then
        update-rc.d -f lvm2 remove >/dev/null 2>&1 || true
       rm -f /etc/init.d/lvm2
    fi

We probably want to be doing that again, possibly with better handling of modified conffiles.

BTW, perhaps it's worth checking with Kees to see if there was a reason he thought this init script should be restored when merging.

--- debian/libdevmapper-event1.02.1.symbols 2012-04-14 03:19:00 +0000
+++ debian/libdevmapper-event1.02.1.symbols 2012-08-18 04:00:45 +0000
@@ -1,11 +1,13 @@
 libdevmapper-event.so.1.02.1 libdevmapper-event1.02.1 #MINVER#
  Base@Base 2:1.02.20
- daemon_talk@Base 2:1.02.67
+ dm_event_daemon_fini_fifos@Base 2:1.02.74
+ dm_event_daemon_init_fifos@Base 2:1.02.74
+ dm_event_daemon_talk@Base 2:1.02.74

This is unfortunate, but it happens... In this case, it appears that dmeventd used the old symbol, so we want to have a Breaks against the old version of dmeventd. No other packages in Ubuntu that depend on libdevmapper-event1.02.1 appear to use that symbol, so this Breaks is the only fix needed.

- dm_event_get_version@Base 2:1.02.67
+ dm_event_get_version@Base 2:1.02.74
  dm_event_handler_create@Base 2:1.02.20
  dm_event_handler_destroy@Base 2:1.02.20
- dm_event_handler_get_dev_name@Base 2:1.02.67
+ dm_event_handler_get_dev_name@Base 2:1.02.74

Those version bumps are unnecessary in Ubuntu, but mostly harmless.

@@ -22,5 +24,3 @@
  dm_event_handler_set_uuid@Base 2:1.02.20
  dm_event_register_handler@Base 2:1.02.20
  dm_event_unregister_handler@Base 2:1.02.20
- fini_fifos@Base 2:1.02.67
- init_fifos@Base 2:1.02.67

These symbols are also only used by dmeventd. (This makes sense, as all three symbols appear to be due to a Debian-specific patch for dmeventd.)

The rest looks good to me.