mooli tayer has posted comments on this change.

Change subject: Fix failure while related services conf files do not exist
......................................................................


Patch Set 1: Code-Review-1

(2 comments)

https://www.youtube.com/watch?v=vgZm8okgH6c

http://gerrit.ovirt.org/#/c/32166/1/lib/vdsm/tool/configurators/configfile.py
File lib/vdsm/tool/configurators/configfile.py:

Line 86:                  version,
Line 87:                  sectionStart='## beginning of configuration section 
by vdsm',
Line 88:                  sectionEnd='## end of configuration section by vdsm',
Line 89:                  prefix='# VDSM backup',
Line 90:                  lineComment='by vdsm'):
I'm missing something here:

If You throw exception at init if file missing,
how can you later touch it?
Line 91:         self._filename = filename
Line 92:         self._context = False
Line 93:         self._sectionStart = sectionStart
Line 94:         self._sectionEnd = sectionEnd


Line 241:                         return True
Line 242:                 return False
Line 243:         except IOError as e:
Line 244:             if e.errno == errno.ENOENT:
Line 245:                 utils.touchFile(self._filename)
I'm not sure touching the file is the right thing to do.

e.g 

I think if we demand libvirt rpm we can demand libvirtd.conf.
If it is missing we can give a nicer warning.

There is the case where vdsm-cli is installed with vdsm tool
(vdsm-python) and with out vdsm. In that case I believe we should change the 
active configurators.(I have an Idea how, lets talk if you go this way)


The solution of touching the file will cause:
new libvirtd.conf that has no meaning on the machine
(because there is no libvirt) also if vdsm will be installed later it will 
afaik be moved to  libvirtd.conf.rpmsave.
We will configure the new libvirtd.conf again.
Line 246:                 return False
Line 247: 
Line 248: 
Line 249: class ParserWrapper(object):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idb6565ce9cef6c7aa3f14c85f1dae0715e1858e2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: mooli tayer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to