[libvirt] [PATCH RFC] spec: move hypvisor-specific files to hypervisor driver subpackages
The libvirt-daemon package contains several hypervisor-specific files, directories, and script, which can be problematic when building the package with multiple hypervisor support, e.g. both QEMU and Xen. E.g. installing a QEMU+Xen enabled libvirt-daemon on a Xen-only system will result in the creation of qemu and kvm groups and a qemu user. Move the hypervisor-specific files, directories, and script to the respective hypervisor driver subpackages. --- libvirt.spec.in | 117 ++++++++++++++++++++++---------------------------------- 1 file changed, 46 insertions(+), 71 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index cfd8890..76c21ec 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1522,23 +1522,6 @@ then fi %if %{with_libvirtd} -%pre daemon - %if 0%{?fedora} >= 12 || 0%{?rhel} >= 6 -# We want soft static allocation of well-known ids, as disk images -# are commonly shared across NFS mounts by id rather than name; see -# https://fedoraproject.org/wiki/Packaging:UsersAndGroups -getent group kvm >/dev/null || groupadd -f -g 36 -r kvm -getent group qemu >/dev/null || groupadd -f -g 107 -r qemu -if ! getent passwd qemu >/dev/null; then - if ! getent passwd 107 >/dev/null; then - useradd -r -u 107 -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu - else - useradd -r -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu - fi -fi -exit 0 - %endif - %post daemon %if %{with_network} @@ -1647,6 +1630,25 @@ fi %endif %endif + %if %{with_qemu} +%pre daemon-driver-qemu + %if 0%{?fedora} >= 12 || 0%{?rhel} >= 6 +# We want soft static allocation of well-known ids, as disk images +# are commonly shared across NFS mounts by id rather than name; see +# https://fedoraproject.org/wiki/Packaging:UsersAndGroups +getent group kvm >/dev/null || groupadd -f -g 36 -r kvm +getent group qemu >/dev/null || groupadd -f -g 107 -r qemu +if ! getent passwd qemu >/dev/null; then + if ! getent passwd 107 >/dev/null; then + useradd -r -u 107 -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu + else + useradd -r -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu + fi +fi +exit 0 + %endif + %endif + %if %{with_network} %post daemon-config-network if test $1 -eq 1 && test ! -f %{_sysconfdir}/libvirt/qemu/networks/default.xml ; then @@ -1770,26 +1772,8 @@ exit 0 %if 0%{?fedora} >= 14 || 0%{?rhel} >= 6 %config(noreplace) %{_prefix}/lib/sysctl.d/libvirtd.conf %endif -%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/qemu/ -%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/lxc/ -%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/uml/ - %if %{with_libxl} -%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/libxl/ - %endif %config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd - %if %{with_qemu} -%config(noreplace) %{_sysconfdir}/libvirt/qemu.conf -%config(noreplace) %{_sysconfdir}/libvirt/qemu-lockd.conf -%config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.qemu - %endif - %if %{with_lxc} -%config(noreplace) %{_sysconfdir}/libvirt/lxc.conf -%config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.lxc - %endif - %if %{with_uml} -%config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.uml - %endif %dir %{_datadir}/libvirt/ @@ -1805,28 +1789,6 @@ exit 0 %dir %attr(0711, root, root) %{_localstatedir}/lib/libvirt/boot/ %dir %attr(0711, root, root) %{_localstatedir}/cache/libvirt/ - %if %{with_qemu} -%ghost %dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/ -%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/ -%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/ -%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/target/ -%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/cache/libvirt/qemu/ - %endif - %if %{with_lxc} -%ghost %dir %{_localstatedir}/run/libvirt/lxc/ -%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/lxc/ - %endif - %if %{with_uml} -%ghost %dir %{_localstatedir}/run/libvirt/uml/ -%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/uml/ - %endif - %if %{with_libxl} -%ghost %dir %{_localstatedir}/run/libvirt/libxl/ -%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/libxl/ - %endif - %if %{with_xen} -%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/xen/ - %endif %if %{with_network} %ghost %dir %{_localstatedir}/run/libvirt/network/ %dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/network/ @@ -1836,16 +1798,6 @@ exit 0 %dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver %attr(0755, root, root) %{_libdir}/libvirt/lock-driver/lockd.so - %if %{with_qemu} -%{_datadir}/augeas/lenses/libvirtd_qemu.aug -%{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug - %endif - - %if %{with_lxc} -%{_datadir}/augeas/lenses/libvirtd_lxc.aug -%{_datadir}/augeas/lenses/tests/test_libvirtd_lxc.aug - %endif - %{_datadir}/augeas/lenses/libvirtd.aug %{_datadir}/augeas/lenses/tests/test_libvirtd.aug %{_datadir}/augeas/lenses/virtlockd.aug @@ -1864,10 +1816,6 @@ exit 0 %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/ - %if %{with_lxc} -%attr(0755, root, root) %{_libexecdir}/libvirt_lxc - %endif - %if %{with_storage_disk} %attr(0755, root, root) %{_libexecdir}/libvirt_parthelper %endif @@ -1933,30 +1881,57 @@ exit 0 %if %{with_qemu} %files daemon-driver-qemu %defattr(-, root, root) +%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/qemu/ +%config(noreplace) %{_sysconfdir}/libvirt/qemu.conf +%config(noreplace) %{_sysconfdir}/libvirt/qemu-lockd.conf +%config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.qemu +%ghost %dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/ +%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/ +%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/ +%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/target/ +%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/cache/libvirt/qemu/ +%{_datadir}/augeas/lenses/libvirtd_qemu.aug +%{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug %{_libdir}/%{name}/connection-driver/libvirt_driver_qemu.so %endif %if %{with_lxc} %files daemon-driver-lxc %defattr(-, root, root) +%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/lxc/ +%config(noreplace) %{_sysconfdir}/libvirt/lxc.conf +%config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.lxc +%ghost %dir %{_localstatedir}/run/libvirt/lxc/ +%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/lxc/ +%{_datadir}/augeas/lenses/libvirtd_lxc.aug +%{_datadir}/augeas/lenses/tests/test_libvirtd_lxc.aug +%attr(0755, root, root) %{_libexecdir}/libvirt_lxc %{_libdir}/%{name}/connection-driver/libvirt_driver_lxc.so %endif %if %{with_uml} %files daemon-driver-uml %defattr(-, root, root) +%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/uml/ +%config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.uml +%ghost %dir %{_localstatedir}/run/libvirt/uml/ +%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/uml/ %{_libdir}/%{name}/connection-driver/libvirt_driver_uml.so %endif %if %{with_xen} %files daemon-driver-xen %defattr(-, root, root) +%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/xen/ %{_libdir}/%{name}/connection-driver/libvirt_driver_xen.so %endif %if %{with_libxl} %files daemon-driver-libxl %defattr(-, root, root) +%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/libxl/ +%ghost %dir %{_localstatedir}/run/libvirt/libxl/ +%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/libxl/ %{_libdir}/%{name}/connection-driver/libvirt_driver_libxl.so %endif -- 1.8.0.1
Jim Fehlig wrote:
The libvirt-daemon package contains several hypervisor-specific files, directories, and script, which can be problematic when building the package with multiple hypervisor support, e.g. both QEMU and Xen.
E.g. installing a QEMU+Xen enabled libvirt-daemon on a Xen-only system will result in the creation of qemu and kvm groups and a qemu user.
Move the hypervisor-specific files, directories, and script to the respective hypervisor driver subpackages.
Any thoughts on moving these hypervisor-specific items to their subpackages? Regards, Jim
--- libvirt.spec.in | 117 ++++++++++++++++++++++---------------------------------- 1 file changed, 46 insertions(+), 71 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in index cfd8890..76c21ec 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1522,23 +1522,6 @@ then fi
%if %{with_libvirtd} -%pre daemon - %if 0%{?fedora} >= 12 || 0%{?rhel} >= 6 -# We want soft static allocation of well-known ids, as disk images -# are commonly shared across NFS mounts by id rather than name; see -# https://fedoraproject.org/wiki/Packaging:UsersAndGroups -getent group kvm >/dev/null || groupadd -f -g 36 -r kvm -getent group qemu >/dev/null || groupadd -f -g 107 -r qemu -if ! getent passwd qemu >/dev/null; then - if ! getent passwd 107 >/dev/null; then - useradd -r -u 107 -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu - else - useradd -r -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu - fi -fi -exit 0 - %endif - %post daemon
%if %{with_network} @@ -1647,6 +1630,25 @@ fi %endif %endif
+ %if %{with_qemu} +%pre daemon-driver-qemu + %if 0%{?fedora} >= 12 || 0%{?rhel} >= 6 +# We want soft static allocation of well-known ids, as disk images +# are commonly shared across NFS mounts by id rather than name; see +# https://fedoraproject.org/wiki/Packaging:UsersAndGroups +getent group kvm >/dev/null || groupadd -f -g 36 -r kvm +getent group qemu >/dev/null || groupadd -f -g 107 -r qemu +if ! getent passwd qemu >/dev/null; then + if ! getent passwd 107 >/dev/null; then + useradd -r -u 107 -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu + else + useradd -r -g qemu -G kvm -d / -s /sbin/nologin -c "qemu user" qemu + fi +fi +exit 0 + %endif + %endif + %if %{with_network} %post daemon-config-network if test $1 -eq 1 && test ! -f %{_sysconfdir}/libvirt/qemu/networks/default.xml ; then @@ -1770,26 +1772,8 @@ exit 0 %if 0%{?fedora} >= 14 || 0%{?rhel} >= 6 %config(noreplace) %{_prefix}/lib/sysctl.d/libvirtd.conf %endif -%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/qemu/ -%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/lxc/ -%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/uml/ - %if %{with_libxl} -%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/libxl/ - %endif
%config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd - %if %{with_qemu} -%config(noreplace) %{_sysconfdir}/libvirt/qemu.conf -%config(noreplace) %{_sysconfdir}/libvirt/qemu-lockd.conf -%config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.qemu - %endif - %if %{with_lxc} -%config(noreplace) %{_sysconfdir}/libvirt/lxc.conf -%config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.lxc - %endif - %if %{with_uml} -%config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.uml - %endif
%dir %{_datadir}/libvirt/
@@ -1805,28 +1789,6 @@ exit 0 %dir %attr(0711, root, root) %{_localstatedir}/lib/libvirt/boot/ %dir %attr(0711, root, root) %{_localstatedir}/cache/libvirt/
- %if %{with_qemu} -%ghost %dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/ -%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/ -%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/ -%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/target/ -%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/cache/libvirt/qemu/ - %endif - %if %{with_lxc} -%ghost %dir %{_localstatedir}/run/libvirt/lxc/ -%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/lxc/ - %endif - %if %{with_uml} -%ghost %dir %{_localstatedir}/run/libvirt/uml/ -%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/uml/ - %endif - %if %{with_libxl} -%ghost %dir %{_localstatedir}/run/libvirt/libxl/ -%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/libxl/ - %endif - %if %{with_xen} -%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/xen/ - %endif %if %{with_network} %ghost %dir %{_localstatedir}/run/libvirt/network/ %dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/network/ @@ -1836,16 +1798,6 @@ exit 0 %dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver %attr(0755, root, root) %{_libdir}/libvirt/lock-driver/lockd.so
- %if %{with_qemu} -%{_datadir}/augeas/lenses/libvirtd_qemu.aug -%{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug - %endif - - %if %{with_lxc} -%{_datadir}/augeas/lenses/libvirtd_lxc.aug -%{_datadir}/augeas/lenses/tests/test_libvirtd_lxc.aug - %endif - %{_datadir}/augeas/lenses/libvirtd.aug %{_datadir}/augeas/lenses/tests/test_libvirtd.aug %{_datadir}/augeas/lenses/virtlockd.aug @@ -1864,10 +1816,6 @@ exit 0
%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/
- %if %{with_lxc} -%attr(0755, root, root) %{_libexecdir}/libvirt_lxc - %endif - %if %{with_storage_disk} %attr(0755, root, root) %{_libexecdir}/libvirt_parthelper %endif @@ -1933,30 +1881,57 @@ exit 0 %if %{with_qemu} %files daemon-driver-qemu %defattr(-, root, root) +%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/qemu/ +%config(noreplace) %{_sysconfdir}/libvirt/qemu.conf +%config(noreplace) %{_sysconfdir}/libvirt/qemu-lockd.conf +%config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.qemu +%ghost %dir %attr(0700, root, root) %{_localstatedir}/run/libvirt/qemu/ +%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/ +%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/ +%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/target/ +%dir %attr(0750, %{qemu_user}, %{qemu_group}) %{_localstatedir}/cache/libvirt/qemu/ +%{_datadir}/augeas/lenses/libvirtd_qemu.aug +%{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug %{_libdir}/%{name}/connection-driver/libvirt_driver_qemu.so %endif
%if %{with_lxc} %files daemon-driver-lxc %defattr(-, root, root) +%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/lxc/ +%config(noreplace) %{_sysconfdir}/libvirt/lxc.conf +%config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.lxc +%ghost %dir %{_localstatedir}/run/libvirt/lxc/ +%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/lxc/ +%{_datadir}/augeas/lenses/libvirtd_lxc.aug +%{_datadir}/augeas/lenses/tests/test_libvirtd_lxc.aug +%attr(0755, root, root) %{_libexecdir}/libvirt_lxc %{_libdir}/%{name}/connection-driver/libvirt_driver_lxc.so %endif
%if %{with_uml} %files daemon-driver-uml %defattr(-, root, root) +%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/uml/ +%config(noreplace) %{_sysconfdir}/logrotate.d/libvirtd.uml +%ghost %dir %{_localstatedir}/run/libvirt/uml/ +%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/uml/ %{_libdir}/%{name}/connection-driver/libvirt_driver_uml.so %endif
%if %{with_xen} %files daemon-driver-xen %defattr(-, root, root) +%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/xen/ %{_libdir}/%{name}/connection-driver/libvirt_driver_xen.so %endif
%if %{with_libxl} %files daemon-driver-libxl %defattr(-, root, root) +%dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/libxl/ +%ghost %dir %{_localstatedir}/run/libvirt/libxl/ +%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/libxl/ %{_libdir}/%{name}/connection-driver/libvirt_driver_libxl.so %endif
On Thu, Dec 5, 2013 at 9:03 AM, Jim Fehlig <jfehlig@suse.com> wrote:
Jim Fehlig wrote:
The libvirt-daemon package contains several hypervisor-specific files, directories, and script, which can be problematic when building the package with multiple hypervisor support, e.g. both QEMU and Xen.
E.g. installing a QEMU+Xen enabled libvirt-daemon on a Xen-only system will result in the creation of qemu and kvm groups and a qemu user.
Move the hypervisor-specific files, directories, and script to the respective hypervisor driver subpackages.
Any thoughts on moving these hypervisor-specific items to their sub packages?
Not related to the code or the work at all but there is a typo in the commit subject that probably should be fixed before pushing. -- Doug
On Tue, Dec 03, 2013 at 03:30:08PM -0700, Jim Fehlig wrote:
The libvirt-daemon package contains several hypervisor-specific files, directories, and script, which can be problematic when building the package with multiple hypervisor support, e.g. both QEMU and Xen.
E.g. installing a QEMU+Xen enabled libvirt-daemon on a Xen-only system will result in the creation of qemu and kvm groups and a qemu user.
Move the hypervisor-specific files, directories, and script to the respective hypervisor driver subpackages. --- libvirt.spec.in | 117 ++++++++++++++++++++++----------------------------------
Mostly looks good, but how about the dirs related to the libvirt-daemon-driver-network RPM - they're still in the main RPM and could be moved too. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Daniel P. Berrange wrote:
On Tue, Dec 03, 2013 at 03:30:08PM -0700, Jim Fehlig wrote:
The libvirt-daemon package contains several hypervisor-specific files, directories, and script, which can be problematic when building the package with multiple hypervisor support, e.g. both QEMU and Xen.
E.g. installing a QEMU+Xen enabled libvirt-daemon on a Xen-only system will result in the creation of qemu and kvm groups and a qemu user.
Move the hypervisor-specific files, directories, and script to the respective hypervisor driver subpackages. --- libvirt.spec.in | 117 ++++++++++++++++++++++----------------------------------
Mostly looks good, but how about the dirs related to the libvirt-daemon-driver-network RPM - they're still in the main RPM and could be moved too.
While doing that, noticed that all the libvirt-daemon-driver-* are conditional on 'with_driver_modules'. So it seems these files, directories, etc are needed in the main RPM when not building driver modules, but in the subpackages when building driver modules? Hmm, not sure if that is correct... Regards, Jim
On Thu, Dec 05, 2013 at 09:15:15AM -0700, Jim Fehlig wrote:
Daniel P. Berrange wrote:
On Tue, Dec 03, 2013 at 03:30:08PM -0700, Jim Fehlig wrote:
The libvirt-daemon package contains several hypervisor-specific files, directories, and script, which can be problematic when building the package with multiple hypervisor support, e.g. both QEMU and Xen.
E.g. installing a QEMU+Xen enabled libvirt-daemon on a Xen-only system will result in the creation of qemu and kvm groups and a qemu user.
Move the hypervisor-specific files, directories, and script to the respective hypervisor driver subpackages. --- libvirt.spec.in | 117 ++++++++++++++++++++++----------------------------------
Mostly looks good, but how about the dirs related to the libvirt-daemon-driver-network RPM - they're still in the main RPM and could be moved too.
While doing that, noticed that all the libvirt-daemon-driver-* are conditional on 'with_driver_modules'. So it seems these files, directories, etc are needed in the main RPM when not building driver modules, but in the subpackages when building driver modules? Hmm, not sure if that is correct...
Oh, of course. That'll be why i left them in the main libvirt-daemon RPM. Guess we'll need to make the this change conditional on the with_driver_modules flag. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange -
Doug Goldstein -
Jim Fehlig