Dan Kenigsberg has submitted this change and it was merged. Change subject: vm: safe(r) Vm.conf update in creation thread ......................................................................
vm: safe(r) Vm.conf update in creation thread A VM is created asynchronously for performance's sake. It is registered in the internal vmContainer inside VDSM as early as possible, while real initialization is carried in background in the creation thread. While creation thread runs, the Vm status is queryable by Engine. In the VM creation thread, the code will change the device configuration data received from Engine, by adding new devices (example: graphic device is added) or by fixing the fields in existing device configuration (example: for disks configuration, truesize/apparentsize is added). The creation thread incorrectly does not take any lock while it is changing Vm.conf['devices'], so concurrent access can fail because it can find data mutating while iterating. This is just made evident by the recent switch to JSON-RPC, but it is a quite old flaw in VDSM. This is known to break in at least two cases: JSON encoding and deepcopy() itself, because both modules in the standard library use iteritems() internally when dealing with dicts(). Since VM creation thread always need to prepare conf data and to update it, the best fix is to prepare it without modifying Vm.conf unsafely and to update it atomically. This patch does that for buildConfDevices(). Bug-Url: https://bugzilla.redhat.com/1143968 Change-Id: Ie9dee2aa01b2c231b99e02a879dcfbd7ecc7f70a Signed-off-by: Francesco Romani <[email protected]> Reviewed-on: http://gerrit.ovirt.org/34813 Reviewed-by: Nir Soffer <[email protected]> Reviewed-by: Piotr Kliczewski <[email protected]> Reviewed-by: Michal Skrivanek <[email protected]> Reviewed-by: Dan Kenigsberg <[email protected]> --- M vdsm/virt/vm.py 1 file changed, 9 insertions(+), 2 deletions(-) Approvals: Piotr Kliczewski: Looks good to me, but someone else must approve Nir Soffer: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved Francesco Romani: Verified Michal Skrivanek: Looks good to me, but someone else must approve Objections: Martin Polednik: I would prefer that you didn't submit this -- To view, visit http://gerrit.ovirt.org/34813 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie9dee2aa01b2c231b99e02a879dcfbd7ecc7f70a Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
