Change in vdsm[master]: Adding the status field to getDeviceLit info.

abaron at redhat.com abaron at redhat.com
Mon Jun 25 15:59:25 UTC 2012


Ayal Baron has posted comments on this change.

Change subject: Adding the status field to getDeviceLit info.
......................................................................


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

(14 inline comments)

....................................................
Commit Message
Line 9: This lun status is the probable result of pvcreate on the lun.
This status tries to predict the result of pvcreate on the lun.

Line 12: free: fpr a lun that is probably straight forward to use in a SD.
s/free:.*/free: pvcreate --test on this lun succeeded
s/used:.*/used: pvcreate --test on this lun failed

Line 15: Caveat emptor: pvcreate on the device can fail in spite that the
s/in spite.*/even if 'force' flag is passed in case for example there is a mounted file system on this lun/

....................................................
File vdsm/API.py
Line 753:     def create(self, name, devlist, force):
I think that defaults should be set here

....................................................
File vdsm/storage/hsm.py
Line 1583:                                     if devicemapper.isDmDevice(dev["GUID"])]
I don't think this should be here at all!

Line 1584:         usables, unusables = lvm.testPVCreate(devNamesToPVTest,
s/usables/unusedDevs/
s/unusables/usedDevs/

Line 1590:             if dev['GUID'] in devNamesToPVTest:
this if should also be removed

Line 1597:                                    "undeterminated for %s", dev)
s/is undeterminated/cannot be determined/

....................................................
File vdsm/storage/lvm.py
Line 616: def _initpvs(devices, metadataSize, mode=""):
this 'mode' is wrong.
This should be 2 separate functions.
The amount of shared code is close to zero.

Line 620:     mode: "test" or "force".
or ""

Line 638:         unsables = set()
change names here as well

Line 647:         _lvminfo._invalidatepvs(devices)
previously this was run whether the command succeeded or failed (because it could have partially failed)..
This is a regression.

Line 673:     try:
if mode == "test"

....................................................
File vdsm/storage/multipath.py
Line 342:         if not "mpath-" in uuid:
remove this

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6af6628a29bb597cf46b57a2b439e1b950bef9ab
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>


More information about the vdsm-patches mailing list