Vinzenz Feenstra has posted comments on this change.

Change subject: Packaging: VDSM v4.12.0 packaged on Ubuntu
......................................................................


Patch Set 3: Code-Review-1

(17 comments)

There are a lot of things to be changed still. In general it's great work, 
however there must be further split up applied. I just commented on some 
things, however I am not 100% sure if I saw everything

....................................................
Commit Message
Line 5: CommitDate: 2013-09-02 12:44:16 +0800
Line 6: 
Line 7: Packaging: VDSM v4.12.0 packaged on Ubuntu
Line 8: 
Line 9: Add files for building VDSM .deb packages on Ubuntu. The package is
split
Line 10: splitted into vdsm, vdsm-tests and various vdsm hooks.
Line 11: 
Line 12: How does it work.
Line 13: The deb packaging scripts are written according to Debian New


Line 6: 
Line 7: Packaging: VDSM v4.12.0 packaged on Ubuntu
Line 8: 
Line 9: Add files for building VDSM .deb packages on Ubuntu. The package is
Line 10: splitted into vdsm, vdsm-tests and various vdsm hooks.
Well there should be a split up of lib/vdsm (vdsm-python package on 
RHEL/Fedora), and a package for the vdsClient (vdsm-client) as well. 
Additionally we're splitting off cpopen as a separate package.

I think we should this here as well.
Line 11: 
Line 12: How does it work.
Line 13: The deb packaging scripts are written according to Debian New
Line 14: Maintainers' Guide [1]. We use dh_make, quilt and -buildpackage to do


....................................................
File debian/control
Line 96: Conflicts: apparmor
Line 97: Description: VDSM: Virtual Desktop Server Manager
Line 98:  The VDSM service is required by a Virtualization Manager to manage 
the 
Line 99:  Linux hosts. VDSM manages and monitors the host's storage, memory and 
Line 100:  networks as well as virtual machine creation, other host 
administration 
trailing whitespaces
Line 101:  tasks, statistics gathering, and log collection.
Line 102: 
Line 103: Package: vdsm-tests
Line 104: Architecture: any


....................................................
File debian/copyright
Line 13:  (at your option) any later version.
Line 14:  .
Line 15:  This package is distributed in the hope that it will be useful,
Line 16:  but WITHOUT ANY WARRANTY; without even the implied warranty of
Line 17:  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the 
trailing whitespaces
Line 18:  GNU General Public License for more details.
Line 19:  .
Line 20:  You should have received a copy of the GNU General Public License
Line 21:  along with this program. If not, see <http://www.gnu.org/licenses/>


....................................................
File debian/patches/drop-libvirt-sanlock.patch
Line 1: libvirt-sanlock provided in latest libvirt needs libaudit0, but 
policycoreutils needs libaudit1.
Line 2: In these release we drop libvirt-sanlock and related feature.
I think dropping sanlock is going to be a show stopper. @fsimonce: What do you 
say?
Line 3: --- a/lib/vdsm/tool/libvirt_configure.sh.in
Line 4: +++ b/lib/vdsm/tool/libvirt_configure.sh.in
Line 5: @@ -235,9 +235,9 @@
Line 6:      fi


Line 3: --- a/lib/vdsm/tool/libvirt_configure.sh.in
Line 4: +++ b/lib/vdsm/tool/libvirt_configure.sh.in
Line 5: @@ -235,9 +235,9 @@
Line 6:      fi
Line 7:  
trailing whitespaces
Line 8:      # Configuring sanlock
Line 9: -    set_if_default "${qconf}" lock_manager \"sanlock\"
Line 10: -    set_if_default "${qlconf}" auto_disk_leases 0
Line 11: -    set_if_default "${qlconf}" require_lease_for_disks 0


....................................................
File debian/patches/fix-util-path.patch
Line 1: Adapt to path of the programs on Ubuntu
Line 2: --- a/configure.ac
Line 3: +++ b/configure.ac
Line 4: @@ -137,7 +137,7 @@
Line 5:  
trailing whitespaces
Line 6:  # External programs (sorted, please keep in order)
Line 7:  AC_PATH_PROG([BLKID_PATH], [blkid], [/sbin/blkid])
Line 8: -AC_PATH_PROG([BRCTL_PATH], [brctl], [/usr/sbin/brctl])
Line 9: +AC_PATH_PROG([BRCTL_PATH], [brctl], [/sbin/brctl])


....................................................
File debian/vdsm.install
Line 1: ./etc/bash_completion.d/vdsClient
part of vdsm-client
Line 2: ./etc/cron.d/vdsm-libvirt-logrotate
Line 3: ./etc/cron.hourly/vdsm-logrotate
Line 4: ./etc/cron.hourly/vdsm-reg-logrotate
Line 5: ./etc/dhcp/dhclient.d/sourceRoute.sh


Line 19: ./etc/vdsm/mom.d/01-parameters.policy
Line 20: ./etc/vdsm/mom.d/02-balloon.policy
Line 21: ./etc/vdsm/mom.d/03-ksm.policy
Line 22: ./etc/vdsm/svdsm.logger.conf
Line 23: ./usr/bin/vdsClient
vdsm-client
Line 24: ./usr/bin/vdsm-tool
Line 25: ./usr/lib/python2.7/dist-packages/cpopen/__init__.py
Line 26: ./usr/lib/python2.7/dist-packages/cpopen/cpopen.so
Line 27: ./usr/lib/python2.7/dist-packages/ovirt_config_setup/engine.py


Line 21: ./etc/vdsm/mom.d/03-ksm.policy
Line 22: ./etc/vdsm/svdsm.logger.conf
Line 23: ./usr/bin/vdsClient
Line 24: ./usr/bin/vdsm-tool
Line 25: ./usr/lib/python2.7/dist-packages/cpopen/__init__.py
cpopen should be separate
Line 26: ./usr/lib/python2.7/dist-packages/cpopen/cpopen.so
Line 27: ./usr/lib/python2.7/dist-packages/ovirt_config_setup/engine.py
Line 28: ./usr/lib/python2.7/dist-packages/sos/plugins/vdsm.py
Line 29: ./usr/lib/python2.7/dist-packages/vdsm/SecureXMLRPCServer.py


Line 23: ./usr/bin/vdsClient
Line 24: ./usr/bin/vdsm-tool
Line 25: ./usr/lib/python2.7/dist-packages/cpopen/__init__.py
Line 26: ./usr/lib/python2.7/dist-packages/cpopen/cpopen.so
Line 27: ./usr/lib/python2.7/dist-packages/ovirt_config_setup/engine.py
Not sure about this one being required to be installed here.
Line 28: ./usr/lib/python2.7/dist-packages/sos/plugins/vdsm.py
Line 29: ./usr/lib/python2.7/dist-packages/vdsm/SecureXMLRPCServer.py
Line 30: ./usr/lib/python2.7/dist-packages/vdsm/__init__.py
Line 31: ./usr/lib/python2.7/dist-packages/vdsm/config.py


Line 24: ./usr/bin/vdsm-tool
Line 25: ./usr/lib/python2.7/dist-packages/cpopen/__init__.py
Line 26: ./usr/lib/python2.7/dist-packages/cpopen/cpopen.so
Line 27: ./usr/lib/python2.7/dist-packages/ovirt_config_setup/engine.py
Line 28: ./usr/lib/python2.7/dist-packages/sos/plugins/vdsm.py
Not sure about this one either being needed to be installed. This is very 
specific to RHEL (AFAIK)
Line 29: ./usr/lib/python2.7/dist-packages/vdsm/SecureXMLRPCServer.py
Line 30: ./usr/lib/python2.7/dist-packages/vdsm/__init__.py
Line 31: ./usr/lib/python2.7/dist-packages/vdsm/config.py
Line 32: ./usr/lib/python2.7/dist-packages/vdsm/constants.py


Line 25: ./usr/lib/python2.7/dist-packages/cpopen/__init__.py
Line 26: ./usr/lib/python2.7/dist-packages/cpopen/cpopen.so
Line 27: ./usr/lib/python2.7/dist-packages/ovirt_config_setup/engine.py
Line 28: ./usr/lib/python2.7/dist-packages/sos/plugins/vdsm.py
Line 29: ./usr/lib/python2.7/dist-packages/vdsm/SecureXMLRPCServer.py
the vdsm python module should be separate
Line 30: ./usr/lib/python2.7/dist-packages/vdsm/__init__.py
Line 31: ./usr/lib/python2.7/dist-packages/vdsm/config.py
Line 32: ./usr/lib/python2.7/dist-packages/vdsm/constants.py
Line 33: ./usr/lib/python2.7/dist-packages/vdsm/define.py


Line 49: ./usr/lib/python2.7/dist-packages/vdsm/tool/vdsm-id.py
Line 50: ./usr/lib/python2.7/dist-packages/vdsm/utils.py
Line 51: ./usr/lib/python2.7/dist-packages/vdsm/vdscli.py
Line 52: ./usr/lib/python2.7/dist-packages/vdsmapi.py
Line 53: ./usr/lib/python2.7/dist-packages/yajsonrpc/__init__.py
yajsonrpc should be separate
Line 54: ./usr/lib/python2.7/dist-packages/yajsonrpc/asyncoreReactor.py
Line 55: ./usr/lib/python2.7/dist-packages/yajsonrpc/client.py
Line 56: ./usr/lib/python2.7/dist-packages/yajsonrpc/protonReactor.py
Line 57: ./usr/libexec/vdsm/curl-img-wrap


Line 66: ./usr/libexec/vdsm/vdsmd_init_common.sh
Line 67: ./usr/share/man/man1/vdsClient.1
Line 68: ./usr/share/man/man8/vdsm-reg.8
Line 69: ./usr/share/man/man8/vdsmd.8
Line 70: ./usr/share/vdsm-bootstrap/deployUtil.py
vdsm-bootstrap should not be installed
Line 71: ./usr/share/vdsm-bootstrap/interface-2/deployUtil.py
Line 72: ./usr/share/vdsm-bootstrap/interface-2/miniyum.py
Line 73: ./usr/share/vdsm-bootstrap/interface-2/setup
Line 74: ./usr/share/vdsm-bootstrap/interface-2/vds_bootstrap.py


Line 74: ./usr/share/vdsm-bootstrap/interface-2/vds_bootstrap.py
Line 75: ./usr/share/vdsm-bootstrap/interface-2/vds_bootstrap_complete.py
Line 76: ./usr/share/vdsm-bootstrap/vds_bootstrap.py
Line 77: ./usr/share/vdsm-bootstrap/vds_bootstrap_complete.py
Line 78: ./usr/share/vdsm-reg/config.py
AFAIK the same like bootstrap. Alon?
Line 79: ./usr/share/vdsm-reg/createDaemon.py
Line 80: ./usr/share/vdsm-reg/deployUtil.py
Line 81: ./usr/share/vdsm-reg/register-to-engine.py
Line 82: ./usr/share/vdsm-reg/save-config


Line 100: ./usr/share/vdsm/dmidecodeUtil.py
Line 101: ./usr/share/vdsm/dsaversion.py
Line 102: ./usr/share/vdsm/dumpStorageTable.py
Line 103: ./usr/share/vdsm/get-conf-item
Line 104: ./usr/share/vdsm/gluster/__init__.py
this should be in a separate package as well (vdsm-gluster)
Line 105: ./usr/share/vdsm/gluster/api.py
Line 106: ./usr/share/vdsm/gluster/cli.py
Line 107: ./usr/share/vdsm/gluster/exception.py
Line 108: ./usr/share/vdsm/gluster/hooks.py


-- 
To view, visit http://gerrit.ovirt.org/18443
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iab62aeadfa65baef3f0f4e6da2592fa0ff451c6f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Zhou Zheng Sheng <zhshz...@linux.vnet.ibm.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshz...@linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to