Change in vdsm[master]: Refactoring storage.misc: moving watchCmd and its deps to vd...

zhshzhou at linux.vnet.ibm.com zhshzhou at linux.vnet.ibm.com
Mon May 13 09:27:09 UTC 2013


Zhou Zheng Sheng has posted comments on this change.

Change subject: Refactoring storage.misc: moving watchCmd and its deps to vdsm.utils.
......................................................................


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

(2 inline comments)

Failed functional test: localfs storage.


The ability provided by watchCmd is that we can interrupt and kill a process started by task manager to run a long-time task. watchCmd is mainly used by the task subsystem and the task subsystem is mainly used by the storage subsystem. Modules other than storage and task manager do not need watchCmd, so why not do the reverse: move qemuImg.py from lib/vdsm/ to vdsm/storage/image.py ?

As I can grep, I found qemuImg.py is needed by vdsm/storage/imageRepository/formatConverter.py and vdsm/storage/image.py , so moving qemuImg.py from lib/vdsm/ to vdsm/storage/ is OK. In this way the modifications are less than move watchCmd and related things to utils.

-1 for  ./vdsm/storage/fileSD.py still use misc.stripNewLines after you moving stripNewLines to utils.

....................................................
File lib/vdsm/qemuImg.py
Line 22: 
Line 23: import utils
Line 24: 
Line 25: _qemuimg = utils.CommandPath("qemu-img",
Line 26:                              "/usr/bin/qemu-img",)  # Fedora, EL6
Actually I think the closing parenthesis stays alone in a line is a good idea, because if we append new items to command path, git diff will just show a new line added. If the closing parenthesis is the same line as the last item, when new items are appened, two lines affected in git diff.
Line 27: 
Line 28: 
Line 29: class FORMAT:
Line 30:     QCOW2 = "qcow2"


....................................................
File lib/vdsm/utils.py
Line 480:         err = err.splitlines(False)
Line 481: 
Line 482:     return (p.returncode, out, err)
Line 483: 
Line 484: 
grep -r 'stripNewLines' . shows

 ./vdsm/storage/fileSD.py:            return misc.stripNewLines(self._oop.directReadLines(

Maybe you shoul edit fileSD.py and change misc.stripNewLines to utils.stripNewLines
Line 485: def stripNewLines(lines):
Line 486:     return [l[:-1] if l.endswith('\n') else l for l in lines]
Line 487: 
Line 488: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I45eaf487b0260863af09087e5114fb836276a750
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Vallarelli <gvallare at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Giuseppe Vallarelli <gvallare at redhat.com>
Gerrit-Reviewer: Livnat Peer <lpeer 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