Change in vdsm[master]: LiveMerge: Sync volume chain information after live merge

fromani at redhat.com fromani at redhat.com
Wed Jun 11 15:20:24 UTC 2014


Francesco Romani has posted comments on this change.

Change subject: LiveMerge: Sync volume chain information after live merge
......................................................................


Patch Set 6:

(5 comments)

A couple of questions here.

http://gerrit.ovirt.org/#/c/26636/6/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2986:         # inconsistencies.  This can happen if a live merge completed while
Line 2987:         # vdsm was not running to catch the event.
Line 2988:         for drive in self._devices[DISK_DEVICES]:
Line 2989:             if drive['device'] == 'disk' and isVdsmImage(drive):
Line 2990:                 self._syncVolumeChain(drive)
How costly is this?
Is this a potentially blocking operation?
Line 2991: 
Line 2992:         # VmStatsThread may use block devices info from libvirt.
Line 2993:         # So, run it after you have this info
Line 2994:         self._initVmStats()


Line 5278:                 alias = aliasElement[0].getAttribute('name')
Line 5279: 
Line 5280:                 if alias == targetAlias:
Line 5281:                     return deviceXML
Line 5282:         return None
We can factorize (in a followup patch) quite some code from this method with
_updateDevicesDomxmlCache
Line 5283: 
Line 5284:     def _driveGetActualVolumeChain(self, drive):
Line 5285:         def pathToVolID(drive, path):
Line 5286:             for vol in drive.volumeChain:


Line 5303:             if not sourceXML:
Line 5304:                 break
Line 5305:             sourceAttr = ('file', 'dev')[drive.blockDev]
Line 5306:             path = sourceXML.getAttribute(sourceAttr)
Line 5307:             volChain.insert(0, pathToVolID(drive, path))
wild idea, feel free to ignore:

volChain.append(pathToVolId(drive, path))

...

return reversed(volChain)
Line 5308:             bsXML = findElement(diskXML, 'backingStore')
Line 5309:             if not bsXML:
Line 5310:                 return None
Line 5311:             diskXML = bsXML


Line 5339:             return
Line 5340: 
Line 5341:         volumeID = volumes[-1]
Line 5342:         # Sync this VM's data strctures.  Ugh, we're storing the same info in
Line 5343:         # two places so we need to change it twice.
Yep, I share this pain. :(
Line 5344:         device = self._lookupConfByPath(drive['path'])
Line 5345:         if drive.volumeID != volumeID:
Line 5346:             # If the active layer changed:
Line 5347:             #  Update the disk path, volumeID, and volumeInfo members


Line 5372:         if status == libvirt.VIR_DOMAIN_BLOCK_JOB_COMPLETED:
Line 5373:             self._syncVolumeChain(drive)
Line 5374:         else:
Line 5375:             self.log.warning("Block job '%s' did not complete successfully "
Line 5376:                              "(status:%i)", path, status)
It is correct to untrack a not completed block job?
Line 5377:         self.untrackBlockJob(jobID)
Line 5378: 
Line 5379:     def _initLegacyConf(self):
Line 5380:         self.conf['displayPort'] = GraphicsDevice.LIBVIRT_PORT_AUTOSELECT


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib86c19077695354b45818e0186e642ad0e8bc07c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list