Change in vdsm[master]: Handle and store serial to guest device names mapping

nsoffer at redhat.com nsoffer at redhat.com
Tue Aug 19 12:11:22 UTC 2014


Nir Soffer has posted comments on this change.

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


Patch Set 2:

(1 comment)

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

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']})
> The mapping {serial: name} maybe not vice versa. The name is a guest inform
Regarding the mapping, this is now question for engine guys, how do they want to get the info. We do use mapping of id to value a lot, so probably {serial: name} is a good format, or if we want to extend it, {serial: {"name": name}}. But again if most other data is returned in the same list of dicts, format, I'm ok with both. 

Rgarding building the list, there is important difference - there is no state where the list of disks is partial because you are in the middle of the loop adding the devices.

You can get the same by:

    tmp = []
    for item in oldlist:
        tmp.append({...})
    self.list = tmp

Using list comprehension this nice property is built in.

    self.list = [{...} 
                 for item in oldlist]

In your code, you clear the list of disks, and the your add disks only if there is mapping info. So you drop the old state, and add new state only if it exists. Is this intentional or an error?
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 <vfeenstr at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr 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