Francesco Romani has posted comments on this change.

Change subject: vm: ensure finish recover with libvirt connection
......................................................................


Patch Set 4:

(3 comments)

http://gerrit.ovirt.org/#/c/24423/4//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2014-02-13 09:31:07 +0100
Line 4: Commit:     Francesco Romani <[email protected]>
Line 5: CommitDate: 2014-02-14 13:40:25 +0100
Line 6: 
Line 7: vm: ensure finish recover with libvirt connection
> Did you mean:
Yes. fixed.
Line 8: 
Line 9: bz106336 highlights an scenario on which a Vm can
Line 10: go Up without a proper libvirt connection established (_dom
Line 11: attribute initialized), and this lead to an out-of-sync


http://gerrit.ovirt.org/#/c/24423/4/vdsm/vm.py
File vdsm/vm.py:

Line 2027:                 self.log.error("Unable to get volume size for %s",
Line 2028:                                drv['volumeID'], exc_info=True)
Line 2029:                 raise StorageUnavailableError("Volume %s is 
corrupted"
Line 2030:                                               " or missing" %
Line 2031:                                               drv['volumeID'])
> Raising StorageUnavailableError is a good change, but the correct check for
Done
Line 2032:         else:
Line 2033:             drv['truesize'] = 0
Line 2034:             drv['apparentsize'] = 0
Line 2035: 


Line 2103:                         # bz1063336;
Line 2104:                         # storage unavailable is not fatal on 
recovery
Line 2105:                         pass
Line 2106:                     else:
Line 2107:                         raise
> I would replace it with:
Done
Line 2108: 
Line 2109:         self.normalizeDrivesIndices(devices[DISK_DEVICES])
Line 2110: 
Line 2111:         # Preserve old behavior. Since libvirt add a memory balloon 
device


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c33a62c29fc70170b6f802a20b6016b301e96de
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: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to