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