Igor Lvovsky has posted comments on this change.

Change subject: Implement VMs live snapshots
......................................................................


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

(5 inline comments)

....................................................
File vdsm/libvirtvm.py
Line 1359:             """Find a drive name given its definition"""
Find drive itself not a name

Line 1391:             for vmDevName, drive in newDrives.items():
As Dan like to comment :), why copy things? use iteritems.

Line 1404:                     for k, v in drive.items():
As above, consider iteritems()

Line 1412:             for device in self.conf["devices"]:
self.conf["devices"][:]  ?
Ah Federico, sorry. it's my fault (I gave you this line in the comment), but I 
wrote the right thing in comment before  :)

Line 1421:             self.saveState()
One general question here. What do you think, should we continue/success 
snapshot if we didn't found the proper drive to update? I mean, you try to 
update self.conf and _devices and you just log  if you can't find the proper 
device to update. Is it OK?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id48a905938037eca2b1de966f4f83d801fdb9970
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to