Milan Zamazal has posted comments on this change.

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


Patch Set 6:

(5 comments)

https://gerrit.ovirt.org/#/c/48404/6//COMMIT_MSG
Commit Message:

Line 8: 
Line 9: ovirt-vmconsole allows access to VMs via serial console.  However, this
Line 10: currently works only for processes running on the VM operating system,
Line 11: such as getty.  It would be useful if vmconsole interacted immediately
Line 12: since the start of a VM.
> This is not clear - how do you wan to interact with the vm if not with some
Clarified.
Line 13: 
Line 14: This change is the first step towards the goal.  It enables BIOS serial
Line 15: console interaction in a VM's XML domain description when a serial
Line 16: console is available.


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

Line 1592:         for consoleDev in self._devices[hwclass.CONSOLE]:
Line 1593:             if consoleDev.isSerial:
Line 1594:                 break
Line 1595:         else:
Line 1596:             consoleDev = None
> Please avoid for-else syntax, it is always confusing and unneeded.
Done
Line 1597:         bios_use_serial = consoleDev is not None
Line 1598: 
Line 1599:         domxml = vmxml.Domain(self.conf, self.log, self.arch)
Line 1600:         domxml.appendOs(bios_use_serial=bios_use_serial)


Line 1593:             if consoleDev.isSerial:
Line 1594:                 break
Line 1595:         else:
Line 1596:             consoleDev = None
Line 1597:         bios_use_serial = consoleDev is not None
> This variable it not needed.
Removed.
Line 1598: 
Line 1599:         domxml = vmxml.Domain(self.conf, self.log, self.arch)
Line 1600:         domxml.appendOs(bios_use_serial=bios_use_serial)
Line 1601: 


Line 1596:             consoleDev = None
Line 1597:         bios_use_serial = consoleDev is not None
Line 1598: 
Line 1599:         domxml = vmxml.Domain(self.conf, self.log, self.arch)
Line 1600:         domxml.appendOs(bios_use_serial=bios_use_serial)
> This should be:
Done
Line 1601: 
Line 1602:         if self.arch == caps.Architecture.X86_64:
Line 1603:             osd = caps.osversion()
Line 1604: 


https://gerrit.ovirt.org/#/c/48404/6/vdsm/virt/vmxml.py
File vdsm/virt/vmxml.py:

Line 230:         self.dom.setAttr('xmlns:' + METADATA_VM_TUNE_PREFIX,
Line 231:                          METADATA_VM_TUNE_URI)
Line 232:         self.dom.appendChild(self._metadata)
Line 233: 
Line 234:     def appendOs(self, bios_use_serial=False):
> Do we have other useserial attributes in this element?
Done
Line 235:         """
Line 236:         Add <os> element to domain:
Line 237: 
Line 238:         <os>


-- 
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: 6
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