Liron Aravot has posted comments on this change.

Change subject: vm, guestagent: hash to include also the disk mapping
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.ovirt.org/#/c/31701/3/vdsm/virt/guestagent.py
File vdsm/virt/guestagent.py:

Line 304:                 disks.append(disk)
Line 305:             self.guestInfo['disksUsage'] = disks
Line 306:             self.guestDiskMapping = args.get('mapping', {})
Line 307:             self.diskMappingHash = 
hash(json.dumps(self.guestDiskMapping,
Line 308:                                                     sort_keys=True))
> do we need to use json here or a simpler str() would be sufficient?
with the json the inner dicts are sorted as well.
Line 309:         elif message == 'number-of-cpus':
Line 310:             self.guestInfo['guestCPUCount'] = int(args['count'])
Line 311:         else:
Line 312:             self.log.error('Unknown message type %s', message)


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

Line 4514:     def _getUnderlyingVmInfo(self):
Line 4515:         self._lastXMLDesc = self._dom.XMLDesc(0)
Line 4516:         devxml = _domParseStr(self._lastXMLDesc).childNodes[0]. \
Line 4517:             getElementsByTagName('devices')[0]
Line 4518:         self._devXmlHash = hash(devxml.toxml())
> +1 with Nir again. If you believe the change at this line it is an improvem
this is not about how hashes are handled, the hash was a string to avoid 
converting it each time. as i introduce this patch there's no need to have it 
as string or to have also the new hash in guestagent.py as string, that's 
redundant as we'll convert it to string on the return time (notice that 
previously there's a reason to do that, now there's not).
Line 4519: 
Line 4520:         return self._lastXMLDesc
Line 4521: 
Line 4522:     def _ejectFloppy(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I148196ccf353613f9cffed7753e7100bd1dd30de
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[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