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