Zhou Zheng Sheng has uploaded a new change for review.
Change subject: fix crash when sync with libvirt network ......................................................................
fix crash when sync with libvirt network
When sync with libvirt network, vdsm finds out networks in libvirt whose corresponding bridge is not existed, then vdsm deletes those networks. vdsm creates a dir for storing the deleted network config as backups. It chown the dir to vdsm:root, but the process runs as vdsm:kvm, so this leads to an Exception. The sync is run in clientIF.__init__, so the Exception causes vdsmd crashing on start.
So this patch chown the dir to vdsm:kvm, then vdsmd starts OK.
Change-Id: I4a3ea0303656cba87bf17b14929fe59bf0d46337 Signed-off-by: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com --- M vdsm/configNetwork.py 1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/56/7156/1
diff --git a/vdsm/configNetwork.py b/vdsm/configNetwork.py index 9dffcdd..5ce3cf6 100755 --- a/vdsm/configNetwork.py +++ b/vdsm/configNetwork.py @@ -212,12 +212,14 @@ # original copy already backed up return
- vdsm_uid = pwd.getpwnam('vdsm').pw_uid + pwdEntry = pwd.getpwnam('vdsm') + vdsm_uid = pwdEntry.pw_uid + vdsm_gid = pwdEntry.pw_gid
# make directory (if it doesn't exist) and assign it to vdsm if not os.path.exists(dirName): os.makedirs(dirName) - os.chown(dirName, vdsm_uid, 0) + os.chown(dirName, vdsm_uid, vdsm_gid)
open(backup, 'w').write(content) os.chown(backup, vdsm_uid, 0)
-- To view, visit http://gerrit.ovirt.org/7156 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I4a3ea0303656cba87bf17b14929fe59bf0d46337 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
oVirt Jenkins CI Server has posted comments on this change.
Change subject: fix crash when sync with libvirt network ......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/441/ : ABORTED
-- To view, visit http://gerrit.ovirt.org/7156 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I4a3ea0303656cba87bf17b14929fe59bf0d46337 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Royce Lv has posted comments on this change.
Change subject: fix crash when sync with libvirt network ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(2 inline comments)
.................................................... File vdsm/configNetwork.py Line 218: Line 219: # make directory (if it doesn't exist) and assign it to vdsm Line 220: if not os.path.exists(dirName): Line 221: os.makedirs(dirName) Line 222: os.chown(dirName, vdsm_uid, vdsm_gid) Actually, we are little confused why we need to chown here,it is vdsmd who create this directory Line 223: Line 224: open(backup, 'w').write(content) Line 225: os.chown(backup, vdsm_uid, 0) Line 226: logging.debug("Persistently backed up %s "
Line 221: os.makedirs(dirName) Line 222: os.chown(dirName, vdsm_uid, vdsm_gid) Line 223: Line 224: open(backup, 'w').write(content) Line 225: os.chown(backup, vdsm_uid, 0) and this one ,too Line 226: logging.debug("Persistently backed up %s " Line 227: "(until next 'set safe config')", backup) Line 228: Line 229: def _networkBackup(self, network):
-- To view, visit http://gerrit.ovirt.org/7156 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I4a3ea0303656cba87bf17b14929fe59bf0d46337 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has posted comments on this change.
Change subject: fix crash when sync with libvirt network ......................................................................
Patch Set 1: (2 inline comments)
Thanks Royce.
.................................................... File vdsm/configNetwork.py Line 218: Line 219: # make directory (if it doesn't exist) and assign it to vdsm Line 220: if not os.path.exists(dirName): Line 221: os.makedirs(dirName) Line 222: os.chown(dirName, vdsm_uid, vdsm_gid) Me, too. We can just remove this line. There is a related patch submitted by Igor Lvovsky. That patch just avoid to backup the config files. http://gerrit.ovirt.org/#/c/7063/ Line 223: Line 224: open(backup, 'w').write(content) Line 225: os.chown(backup, vdsm_uid, 0) Line 226: logging.debug("Persistently backed up %s "
Line 221: os.makedirs(dirName) Line 222: os.chown(dirName, vdsm_uid, vdsm_gid) Line 223: Line 224: open(backup, 'w').write(content) Line 225: os.chown(backup, vdsm_uid, 0) Thanks. Line 226: logging.debug("Persistently backed up %s " Line 227: "(until next 'set safe config')", backup) Line 228: Line 229: def _networkBackup(self, network):
-- To view, visit http://gerrit.ovirt.org/7156 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I4a3ea0303656cba87bf17b14929fe59bf0d46337 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Mark Wu has posted comments on this change.
Change subject: fix crash when sync with libvirt network ......................................................................
Patch Set 1:
Zhengsheng, Could you please paste the backtrace of the exception here? Thanks
-- To view, visit http://gerrit.ovirt.org/7156 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I4a3ea0303656cba87bf17b14929fe59bf0d46337 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: fix crash when sync with libvirt network ......................................................................
Patch Set 1: (1 inline comment)
.................................................... File vdsm/configNetwork.py Line 218: Line 219: # make directory (if it doesn't exist) and assign it to vdsm Line 220: if not os.path.exists(dirName): Line 221: os.makedirs(dirName) Line 222: os.chown(dirName, vdsm_uid, vdsm_gid) this function is called by superVdsm, which runs as root.
as far as I recall, the intention behind these chown's was to let vdsm to clear the backup directory, or even run addNetwork. yet I do not think this is strictly required these days, and would be glad to get rid of these chowns after proper mapping of the issue.
I do not understand why uid was not enough, though, and gid is needed, too. Line 223: Line 224: open(backup, 'w').write(content) Line 225: os.chown(backup, vdsm_uid, 0) Line 226: logging.debug("Persistently backed up %s "
-- To view, visit http://gerrit.ovirt.org/7156 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I4a3ea0303656cba87bf17b14929fe59bf0d46337 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Royce Lv has posted comments on this change.
Change subject: fix crash when sync with libvirt network ......................................................................
Patch Set 1: No score
MainThread::ERROR::2012-08-13 13:58:34,249::vdsm::99::vds::(startVdsm) Exception raised Traceback (most recent call last): File "/usr/share/vdsm/vdsm", line 97, in startVdsm serve_clients(log) File "/usr/share/vdsm/vdsm", line 47, in serve_clients cif = clientIF.getInstance(log) File "/usr/share/vdsm/clientIF.py", line 126, in getInstance cls._instance = clientIF(log) File "/usr/share/vdsm/clientIF.py", line 75, in __init__ self._syncLibvirtNetworks() File "/usr/share/vdsm/clientIF.py", line 203, in _syncLibvirtNetworks configWriter.removeLibvirtNetwork(network) File "/usr/share/vdsm/configNetwork.py", line 192, in removeLibvirtNetwork self._networkBackup(network) File "/usr/share/vdsm/configNetwork.py", line 229, in _networkBackup self._persistentNetworkBackup(network) File "/usr/share/vdsm/configNetwork.py", line 247, in _persistentNetworkBackup network, content) File "/usr/share/vdsm/configNetwork.py", line 220, in writeBackupFile os.chown(dirName, vdsm_uid, 0) OSError: [Errno 1] Operation not permitted: '/var/lib/vdsm/netconfback/logicalnetworks/'
-- To view, visit http://gerrit.ovirt.org/7156 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I4a3ea0303656cba87bf17b14929fe59bf0d46337 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has posted comments on this change.
Change subject: fix crash when sync with libvirt network ......................................................................
Patch Set 2:
Mark Wu, here is the stack trace.
MainThread::ERROR::2012-08-14 11:43:53,500::vdsm::74::vds::(run) Exception raised Traceback (most recent call last): File "/usr/share/vdsm/vdsm", line 72, in run serve_clients(log) File "/usr/share/vdsm/vdsm", line 40, in serve_clients cif = clientIF.getInstance(log) File "/usr/share/vdsm/clientIF.py", line 126, in getInstance cls._instance = clientIF(log) File "/usr/share/vdsm/clientIF.py", line 75, in __init__ self._syncLibvirtNetworks() File "/usr/share/vdsm/clientIF.py", line 203, in _syncLibvirtNetworks configWriter.removeLibvirtNetwork(network) File "/usr/share/vdsm/configNetwork.py", line 192, in removeLibvirtNetwork self._networkBackup(network) File "/usr/share/vdsm/configNetwork.py", line 232, in _networkBackup self._persistentNetworkBackup(network) File "/usr/share/vdsm/configNetwork.py", line 250, in _persistentNetworkBackup network, content) File "/usr/share/vdsm/configNetwork.py", line 226, in writeBackupFile os.chown(backup, vdsm_uid, 0) OSError: [Errno 1] Operation not permitted: '/var/lib/vdsm/netconfback/logicalnetworks/virbr0'
-- To view, visit http://gerrit.ovirt.org/7156 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I4a3ea0303656cba87bf17b14929fe59bf0d46337 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Royce Lv has posted comments on this change.
Change subject: fix crash when sync with libvirt network ......................................................................
Patch Set 2:
You can simply reproduce this bug by: (1)service vdsmd stop (2)manually add a bridge to system (3)service vdsmd start (4)then you can see vdsm log for this error
-- To view, visit http://gerrit.ovirt.org/7156 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I4a3ea0303656cba87bf17b14929fe59bf0d46337 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has posted comments on this change.
Change subject: fix crash when sync with libvirt network ......................................................................
Patch Set 2:
Before Igor Lvovsky's commit b60650fc, vdsmd will backup when sync with libvirt network, so writeBackupFile function is called by vdsmd indirectly, then it will trigger a permission error. So I let it chown to vdsm:kvm to avoid that error.
After discussion with Royce Lv, we discovered that neither this patch nor commit b60650fc is elegant.
This patch chown to vdsm:kvm to avoid the permission problem, and commit b60650fc skip the backup by adding a not-so-nice bool parameter to the createLibvirtNetwork function.
Since Dan said the writeBackupFile function is supposed to called only by superVdsm, when vdsmd sync with libvirt network, the clientIF._syncLibvirtNetworks function should be delegated to superVdsm, then there will be no permission problems. In general, sync network config may need root privilege in various situations, do delegate it to superVdsm is more appropriate.
-- To view, visit http://gerrit.ovirt.org/7156 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I4a3ea0303656cba87bf17b14929fe59bf0d46337 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has abandoned this change.
Change subject: fix crash when sync with libvirt network ......................................................................
Patch Set 2: Abandoned
In general, sync network config may need root privilege in various situations, to delegate it to superVdsm is more appropriate.
-- To view, visit http://gerrit.ovirt.org/7156 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: abandon Gerrit-Change-Id: I4a3ea0303656cba87bf17b14929fe59bf0d46337 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
vdsm-patches@lists.fedorahosted.org