Adam Litke has posted comments on this change.

Change subject: Live Merge: Resolve unknown merge status in vdsm after host 
crash
......................................................................


Patch Set 3: Verified+1

(3 comments)

http://gerrit.ovirt.org/#/c/31267/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 5394:                            "monitoring.  Unable to perform live 
merge.")
Line 5395:             return errCode['mergeErr']
Line 5396: 
Line 5397:         # If engine passes empty values for baseVolUUID and 
topVolUUID then we
Line 5398:         # will just try to recover from a previous merge.  Just 
create a fake
> I wonder if wouldn't be better to have an explicit verb for this. But I gue
This is quite a corner case and we're hoping to do away with it altogether in 
the future by asking libvirt for two-phase blockCommit even for internal 
snapshots.  Then, we will always be in control of whether the snapshot has been 
removed from the chain or not.

Hoping to keep the API surface of live merge as small as possible.
Line 5399:         # block job and allow the cleanup logic to run again.
Line 5400:         if not baseVolUUID and not topVolUUID:
Line 5401:             with self._jobsLock:
Line 5402:                 try:


Line 5570:             # properly recover the VM in case we crash.  At this 
point the
Line 5571:             # active layer contains the same data as its parent so 
the ILLEGAL
Line 5572:             # flag indicates that the VM should be restarted using 
the parent.
Line 5573:             curVols = set([x['volumeID'] for x in drive.volumeChain])
Line 5574:             newVols = curVols - set((drive.volumeID,))
> Why not just
Thanks for pointing it out.  I'll update it if I need a resubmit.
Line 5575:             self.cif.irs.imageSyncVolumeChain(drive.domainID, 
drive.imageID,
Line 5576:                                               drive['volumeID'], 
newVols)
Line 5577: 
Line 5578:             self.log.info("Requesting pivot to complete active layer 
commit "


Line 5656:         """
Line 5657:         Returns True if the last cleanup phase succeeded and the job 
can be
Line 5658:         removed.
Line 5659:         """
Line 5660:         return bool(self.success and self.mode == self.MODE_CLEANUP)
> not a fan of explicit bool() but again, no big deal.
ok, will update if there is a resubmit.
Line 5661: 
Line 5662: 
Line 5663: def _devicesWithAlias(domXML):
Line 5664:     return vmxml.filter_devices_with_alias(vmxml.all_devices(domXML))


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ca5e4ad67216d30c3149a53c2ea65cc97601bfc
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: [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