Change in vdsm[master]: Supervdsm as external service

danken at redhat.com danken at redhat.com
Sun May 26 19:14:31 UTC 2013


Dan Kenigsberg has posted comments on this change.

Change subject: Supervdsm as external service
......................................................................


Patch Set 11: I would prefer that you didn't submit this

(7 inline comments)

mostly minor comments. I like the spirit.

....................................................
File vdsm/sudoers.vdsm.in
Line 2:     @DMIDECODE_PATH@, \
Line 3:     @VDSMDIR@/mk_sysprep_floppy, \
Line 4:     @SERVICE_PATH@ ksmtuned *, \
Line 5:     @SERVICE_PATH@ ksm *, \
Line 6:     @SERVICE_PATH@ vdsmd *, \
why have you moved this? I think that it is still used only by the storage subsystem.
Line 7:     @VDSMDIR@/prepare-vmchannel
Line 8: Cmnd_Alias VDSM_STORAGE = @MOUNT_PATH@, @UMOUNT_PATH@, \
Line 9:     @FSCK_PATH@ -p *, \
Line 10:     @TUNE2FS_PATH@ -j *, \


....................................................
File vdsm/supervdsmd.service
Line 1: [Unit]
Line 2: Description=Super Vdsm Service
maybe a less dramatic and more informative description would be better:

"Auxiliary Vdsm service for running helper functions as the super user"

or something like that.
Line 3: 
Line 4: [Service]
Line 5: Type=simple
Line 6: ExecStart=/usr/share/vdsm/supervdsmServer.py /var/run/vdsm/svdsm.sock


Line 2: Description=Super Vdsm Service
Line 3: 
Line 4: [Service]
Line 5: Type=simple
Line 6: ExecStart=/usr/share/vdsm/supervdsmServer.py /var/run/vdsm/svdsm.sock
use @PYTHON@ to avoid ovirt-node surprises; use @VDSMRUNDIR@ to be nice to Ubuntu.


....................................................
File vdsm/supervdsm.py
Line 75:             self._log.warn("Connect to svdsm failed %s", ex)
Line 76:             raise
Line 77:         self._svdsm = self._manager.instance()
Line 78: 
Line 79:     def _launch(self):
we no longer launch supervdsm here, so this is a misnomer.
Line 80:         try:
Line 81:             utils.retry(self._connect, Exception, timeout=60, tries=3)
Line 82:         except:
Line 83:             utils.panic("Couldn't connect to supervdsm service")


Line 115
Line 116
Line 117
Line 118
Line 119
I love the smell of code burning!


....................................................
File vdsm/supervdsmServer.py
Line 1: #!/usr/bin/python
why was this added? not that you still cannot just execute this script, since ovirt-node blacklists all *.py. Keep using

  python supervdsmServer.pyc

please.
Line 2: #
Line 3: # Copyright 2011 Red Hat, Inc.
Line 4: #
Line 5: # This program is free software; you can redistribute it and/or modify


....................................................
File vdsm/vdsmd.service
Line 1: [Unit]
Line 2: Description=Virtual Desktop Server Manager
Line 3: Requires=multipathd.service libvirtd.service time-sync.target iscsid.service supervdsmd.service
do you know if this line can be broken?
Line 4: Conflicts=libvirt-guests.service ksmtuned.service
Line 5: 
Line 6: [Service]
Line 7: Type=forking


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I290a584f38129406cd390fdd1d3d1aad9f829a60
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Adam Litke <agl at us.ibm.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Mei Liu <liumbj at linux.vnet.ibm.com>
Gerrit-Reviewer: Ohad Basan <obasan at redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce at linux.vnet.ibm.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list