Vinzenz Feenstra has posted comments on this change.

Change subject: Handle and store serial to guest device names mapping
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.ovirt.org/#/c/31497/2/vdsm/virt/guestagent.py
File vdsm/virt/guestagent.py:

Line 119:         self._socketName = socketName
Line 120:         self._sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
Line 121:         self._stopped = True
Line 122:         self.guestStatus = None
Line 123:         self.guestDiskMapping = []
> why it's not in self.guestInfo?
Because guestInfo gets exported to getVmStats
Line 124:         self.guestInfo = {
Line 125:             'username': user,
Line 126:             'memUsage': 0,
Line 127:             'guestCPUCount': -1,


Line 304:             self.guestInfo['disksUsage'] = disks
Line 305:             if 'mapping' in args:
Line 306:                 for disk in args['mapping']:
Line 307:                     self.guestDiskMapping.append({'serial': 
disk['serial'],
Line 308:                                                   'name': 
disk['name']})
> Why not a mapping of disk serial by name of device?
The mapping {serial: name} maybe not vice versa. The name is a guest 
information. The question is what is the best for the implementation to export 
it somehow in the API. It was suggested as part of the device information.

Not sure where the list comprehension here would be any simpler
The line count is the same, and the readability is even worse than without it.
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)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I69db18414b17b23eb7bcbfbd8b2584f622e53276
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Tal Nisan <[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