Martin Polednik has posted comments on this change.

Change subject: vmxml: export container metadata
......................................................................


Patch Set 23: Code-Review-1

(6 comments)

https://gerrit.ovirt.org/#/c/60481/23/vdsm/virt/vmxml.py
File vdsm/virt/vmxml.py:

PS23, Line 29: from vdsm.containersconnection import XML as xmlcont
This is added in the following patch, why is it here?


Line 28: from vdsm import constants
Line 29: from vdsm.containersconnection import XML as xmlcont
Line 30: from vdsm import cpuarch
Line 31: from vdsm import utils
Line 32: 
unrelated and unneeded
Line 33: 
Line 34: METADATA_VM_TUNE_URI = 'http://ovirt.org/vm/tune/1.0'
Line 35: METADATA_VM_TUNE_ELEMENT = 'qos'
Line 36: METADATA_VM_TUNE_PREFIX = 'ovirt'


Line 33: 
Line 34: METADATA_VM_TUNE_URI = 'http://ovirt.org/vm/tune/1.0'
Line 35: METADATA_VM_TUNE_ELEMENT = 'qos'
Line 36: METADATA_VM_TUNE_PREFIX = 'ovirt'
Line 37: 
unrelated and unneeded
Line 38: 
Line 39: _BOOT_MENU_TIMEOUT = 10000  # milliseconds
Line 40: 
Line 41: 


PS23, Line 296: six.iteritems(drive_map)
same as below


PS23, Line 612: six.iteritems(custom)
Do you expect custom to be ever accessed concurrently? itemview seems 
reasonable in python3 meaning dict.items() could be used.


PS23, Line 616: except ValueError:
If key starts with 'volume:', you'll always have 2 values e.g.

"volume:" ~> ['volume', '']

Are you expecting some different ValueError than one related to unpacking 
values? Please explain.


-- 
To view, visit https://gerrit.ovirt.org/60481
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ade3c0c7d300c5ce33cb23723c3d0e59e4af664
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Reply via email to