Nir Soffer has posted comments on this change.

Change subject: virt: Make BIOS messages available on vmconsole
......................................................................


Patch Set 8: Code-Review+1

(2 comments)

Very nice, waiting for Francesco approval.

https://gerrit.ovirt.org/#/c/48404/8/tests/vmTests.py
File tests/vmTests.py:

Line 289:         xml = find_xml_element(domxml.dom.toxml(), './sysinfo')
Line 290:         self.assertXMLEqual(xml, sysinfoXML)
Line 291: 
Line 292:     @permutations([['serial', True], ['virtio', False]])
Line 293:     def testSerialBios(self, console_type, use_serial):
I would like to only lowercase names in new code, even if it is less consistent 
wit surrounding code, but it this is what Francesco prefer I'm ok with it.
Line 294:         devices = {'device': 'console', 'type': 'console',
Line 295:                    'specParams': {'consoleType': console_type}},
Line 296:         with fake.VM(devices=devices, create_device_objects=True) as 
fakevm:
Line 297:             dom_xml = fakevm._buildDomainXML()


https://gerrit.ovirt.org/#/c/48404/8/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 1569:         """
Line 1570:         for console in self._devices[hwclass.CONSOLE]:
Line 1571:             if console.isSerial:
Line 1572:                 return console
Line 1573:         return None
Maybe raise NotFound error instead returning None?

I'm seeing the future errors:

    AttributeError: 'NoneType' object has no attribute 'toxml'
Line 1574: 
Line 1575:     def _customDevices(self):
Line 1576:         """
Line 1577:             Get all devices that have custom properties


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc560d2562a913b7dba9a5c01952429459595973
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzama...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to