Change in vdsm[master]: vm: Update domains list for disks attached to vm after LSM

Federico Simoncelli fsimonce at redhat.com
Wed Dec 18 17:58:28 UTC 2013


Federico Simoncelli has posted comments on this change.

Change subject: vm: Update domains list for disks attached to vm after LSM
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

+1 because looks urgent but I'd be very happy to see more work on improving sdIds (and append/remove). I'll spare my +2 for when there will be at least a WIP patch in that direction.

....................................................
File vdsm/vm.py
Line 4147:                 self.log.error("Unable to teardown the previous chain: %s",
Line 4148:                                diskToTeardown, exc_info=True)
Line 4149:             self.updateDriveParameters(dstDiskCopy)
Line 4150:             if "domainID" in srcDisk:
Line 4151:                 self.sdIds.append(dstDiskCopy['domainID'])
I'd be more comfortable in acking this patch if there was already another WIP patch that makes this append/remove process part of some pre-existing entry/exit point for using the disks... for example prepareVolumePath and teardownVolumePath. Maybe another thing that would make it more robust is to loop over the devices at every prepare/teardown to refresh a set, instead of keeping a list with possible multiple entries for the same domain.
Line 4152:                 self.sdIds.remove(srcDisk['domainID'])
Line 4153:         finally:
Line 4154:             self._delDiskReplica(srcDrive)
Line 4155:             self.startDisksStatsCollection()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6cda08eea5e94f4cb9039ce08eac2bfe0861e260
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list