Change in vdsm[master]: vdsm-upgrade: adds wrapper to ovirt-node-upgrade

Alon Bar-Lev alonbl at redhat.com
Wed Jun 11 08:26:50 UTC 2014


Alon Bar-Lev has posted comments on this change.

Change subject: vdsm-upgrade: adds wrapper to ovirt-node-upgrade
......................................................................


Patch Set 5:

(5 comments)

http://gerrit.ovirt.org/#/c/28244/5/vdsm_reg/vdsm-upgrade
File vdsm_reg/vdsm-upgrade:

Line 15: 
Line 16: from xml.sax import saxutils
Line 17: 
Line 18: _XML_QUOTES = {
Line 19:     "\n": ' ',
each line should be own message, no? so splitlines each string.
Line 20:     "'": '~'
Line 21: }
Line 22: 
Line 23: 


Line 49:     # To get the messages async we should read stderr when triggering
Line 50:     # the upgrade since the tool uses subprocess.Popen() that waits
Line 51:     # the command terminates.
Line 52:     while True:
Line 53:         line = upgrade_tool.stdout.readline()
using sync mode does not let us read both stdout and stderr, and we do want to see what in stderr...

if we want to do this simple, we should pipe both into same descriptor so we read only one synchronously.

if not, we need to use async method, see otopi[1], maybe vdsm has similar utility.

[1] http://gerrit.ovirt.org/gitweb?p=otopi.git;a=blob;f=src/otopi/plugin.py;hb=HEAD#l337
Line 54: 
Line 55:         if not line:
Line 56:             break
Line 57: 


Line 51:     # the command terminates.
Line 52:     while True:
Line 53:         line = upgrade_tool.stdout.readline()
Line 54: 
Line 55:         if not line:
this should be is None or you probably stop loop at first empty line?
Line 56:             break
Line 57: 
Line 58:         msg = "<BSTRAP component='ovirt-node-upgrade' " \
Line 59:             "status='OK' message='%s'/>" % saxutils.escape(line, _XML_QUOTES)


Line 58:         msg = "<BSTRAP component='ovirt-node-upgrade' " \
Line 59:             "status='OK' message='%s'/>" % saxutils.escape(line, _XML_QUOTES)
Line 60: 
Line 61:         print(msg)
Line 62:         logging.debug(msg)
if you debug each message, you also need this as function
Line 63: 
Line 64:     upgrade_tool.wait()
Line 65: 
Line 66:     if upgrade_tool.returncode == 0:


Line 72: 
Line 73:     print(msg)
Line 74:     logging.debug(msg)
Line 75: 
Line 76:     if upgrade_tool.returncode == 0:
why do you ask twice?

put the message format in function instead
Line 77:         msg = "<BSTRAP component='RHEV_INSTALL' status='OK'/>"
Line 78:     else:
Line 79:         msg = "<BSTRAP component='RHEV_INSTALL' status='FAIL'/>"
Line 80: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7b997d70a440545497246d1a19d9671b054a56a5
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Joey Boggs <jboggs at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list