On Thu, Feb 17, 2011 at 05:52:07PM -0800, Andrew Evans wrote: > Commit d66880ee attempted to revert the original XenServer-shipped versions of > scripts replaced by OVS during an RPM upgrade, but the logic was incorrect. It > assumed that %postun of the package being replaced was run before the %post of > the new version replacing it. The reverse is actually true. > > Make upgrade and erase cases both work correctly by simply checking whether > any > of the files ever replaced in any OVS version are dangling symlinks, and if > so, > attempt to copy the saved XenServer original back to its rightful place. In > the > upgrade case, if the newly-installed version of OVS lacks any of the scripts > in > the previous version, those will be reverted. In the erase case, none of the > OVS replacements will exist, so they will all be dangling symlinks and will > all > be reverted. > > Furthermore, replace any dangling symlink from /usr/sbin/xen-bugtool to the > now-nonexistent OVS replacement (caused by commit 92dbd5c9). > > Lastly, prevent accidental reversion of files replaced by OVS with XenServer > originals during rpm -U (also caused by commit 92dbd5c9). > > Bug #4696.
This looks good to me, with a few comments. In references to commit numbers, please also give the subject of the commit (the first line of the log message). > +# Deliberately break %postun in broken OVS builds that revert original > +# XenServer scripts during rpm -U by moving the directory where it thinks > +# they are saved. > +[ -d /usr/lib/openvswitch/xs-original ] && \ > + mv -f /usr/lib/openvswitch/xs-original /usr/lib/openvswitch/xs-saved > 2>/dev/null I see two possible issues here. If /usr/lib/openvswitch/xs-original and /usr/lib/openvswitch/xs-saved both exist, then I think that we will end up with a /usr/lib/openvswitch/xs-saved/xs-original directory. So I think that we should add a [ ! -e /usr/lib/openvswitch/xs-saved ] condition here too. Second, the '-f' and '2>/dev/null' indicate that you expect that there could be errors that should be suppress. What errors do you expect, and why should they be suppressed? > + saved="/usr/lib/openvswitch/xs-saved/$(basename "$orig")" There's no word splitting on the right side of a shell assignment, so you can omit the outermost double quotes here. It sure is a shame that rpm doesn't have anything equivalent to dpkg's "diversions" mechanism. Just look how much time and trouble it could have saved us over the years. > + # Only revert dangling symlinks. > + if [ -h "$f" ] && ! readlink -e "$f"; then Won't this readlink invocation print the link name on the console? Should we redirect to /dev/null? But I think that [ ! -e "$f" ] would work just as well, since "test" generally follows symlinks. Thanks for cleaning up and fixing all of this. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
