Change in vdsm[master]: vm: Automatically add a serial port for a console device

[Vinzenz Feenstra]

Vinzenz Feenstra has uploaded a new change for review.

Change subject: vm: Automatically add a serial port for a console device
......................................................................

vm: Automatically add a serial port for a console device

Without a serial device the console support does not work for
linux systems. We switch from virtio to serial and append a serial
port if a console device was defined.

Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Signed-off-by: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
---
M vdsm/vm.py
1 file changed, 14 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/79/25979/1

diff --git a/vdsm/vm.py b/vdsm/vm.py
index 9cb0e82..7ce7c59 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -1171,6 +1171,18 @@
         commandLine.appendChildWithArgs('qemu:arg', value='keyboard')
         self.dom.appendChild(commandLine)
 
+    def appendSerial(self):
+        """
+        Add a serial port for the console device if it exists
+        <serial type='pty'>
+            <target port='0'>
+        </serial>
+        """
+        if len(self._devices.getElementsByTagName('console')) == 1:
+            s = XMLElement('serial', type='pty')
+            s.appendChildWithArgs('target', port='0')
+            self._devices.appendChild(s)
+
     def appendGraphics(self):
         """
         Add graphics section to domain xml.
@@ -1888,7 +1900,7 @@
         </console>
         """
         m = self.createXmlElem('console', 'pty')
-        m.appendChildWithArgs('target', type='virtio', port='0')
+        m.appendChildWithArgs('target', type='serial', port='0')
         return m
 
 
@@ -3025,6 +3037,7 @@
             domxml.appendEmulator()
 
         self._appendDevices(domxml)
+        domxml.appendSerial()
 
         for drive in self._devices[DISK_DEVICES][:]:
             for leaseElement in drive.getLeasesXML():


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 3:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6777/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7567/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7677/ : FAILURE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 4:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6778/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7568/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7678/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Michal Skrivanek has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.ovirt.org/#/c/25979/4/vdsm/vm.py
File vdsm/vm.py:

Line 1170:         commandLine.appendChildWithArgs('qemu:arg',
value='-usbdevice')
Line 1171:         commandLine.appendChildWithArgs('qemu:arg',
value='keyboard')
Line 1172:         self.dom.appendChild(commandLine)
Line 1173: 
Line 1174:     def appendSerial(self):
why not part of ConsoleDevice's getXML?
Line 1175:         """
Line 1176:         Add a serial port for the console device if it exists.
Line 1177: 
Line 1178:         <serial type='pty'>


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.ovirt.org/#/c/25979/4/vdsm/vm.py
File vdsm/vm.py:

Line 1170:         commandLine.appendChildWithArgs('qemu:arg',
value='-usbdevice')
Line 1171:         commandLine.appendChildWithArgs('qemu:arg',
value='keyboard')
Line 1172:         self.dom.appendChild(commandLine)
Line 1173: 
Line 1174:     def appendSerial(self):
>  why not part of ConsoleDevice's getXML? 
I tried to, but it
will fail as it expects only one element :(
Line 1175:         """
Line 1176:         Add a serial port for the console device if it exists.
Line 1177: 
Line 1178:         <serial type='pty'>


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.ovirt.org/#/c/25979/4/vdsm/vm.py
File vdsm/vm.py:

Line 1170:         commandLine.appendChildWithArgs('qemu:arg',
value='-usbdevice')
Line 1171:         commandLine.appendChildWithArgs('qemu:arg',
value='keyboard')
Line 1172:         self.dom.appendChild(commandLine)
Line 1173: 
Line 1174:     def appendSerial(self):
>  I tried to, but it will fail as it expects only one element :(
Let's try to use a more specific name: appendConsoleSerial (so we know that this
is only for the console).
Line 1175:         """
Line 1176:         Add a serial port for the console device if it exists.
Line 1177: 
Line 1178:         <serial type='pty'>


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Michal Skrivanek has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 5: Code-Review+1

deserves a bug, I'd like to see this in 3.4.z too

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.ovirt.org/#/c/25979/4/vdsm/vm.py
File vdsm/vm.py:

Line 1170:         commandLine.appendChildWithArgs('qemu:arg',
value='-usbdevice')
Line 1171:         commandLine.appendChildWithArgs('qemu:arg',
value='keyboard')
Line 1172:         self.dom.appendChild(commandLine)
Line 1173: 
Line 1174:     def appendSerial(self):
>  Let's try to use a more specific name: appendConsoleSerial (so we
> know that 
Done
Line 1175:         """
Line 1176:         Add a serial port for the console device if it exists.
Line 1177: 
Line 1178:         <serial type='pty'>


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 5:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8736/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8872/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7946/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 6:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8744/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8880/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7954/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Michal Skrivanek has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 6:

would be cool to have a working serial access for headless VMs….:-)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Antoni Segura Puimedon has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 6: Code-Review+1

For the current master and for 3.4 I'm okay with this. However, we should revisit
after the 3.5 branching the composed device solution so that serial device is it's own
subdevice of the console device.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Francesco Romani has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 6:

(1 comment)

I agree with Toni here. Spice GraphicsDevices may be another case of composite devices
(for spice vmc channels), or anyway a case worth considering.

http://gerrit.ovirt.org/#/c/25979/6/tests/vmTests.py
File tests/vmTests.py:

Line 300:         self.assertXML(console.getXML(), consoleXML)
Line 301:         domxml = vm._DomXML(self.conf, self.log,
Line 302:                             caps.Architecture.X86_64)
Line 303:         domxml.appendConsoleSerial()
Line 304:         assert(len(domxml._devices.getElementsByTagName('serial')) ==
0)
I'd use self.assertFalse for better reporting
Line 305:         domxml._devices.appendChild(console.getXML())
Line 306:         domxml.appendConsoleSerial()
Line 307:         self.assertXML(domxml._devices, serialXML, 'serial')
Line 308: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Itamar Heim has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 6:

ping

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Francesco Romani has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 6: Code-Review+1

Needs a big rebase, but I'm fine with the concept.
Minor comment inside

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 7:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11128/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12070/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11917/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1568/ : There was an
infra issue, please contact infra(a)ovirt.org

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 7: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 7:

Tested on RHEL6 - With this patch I can now attach to the serial console of a guest via
virsh - Previously this did not work.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Antoni Segura Puimedon has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 7: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Francesco Romani has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 7: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 7: Code-Review-1

(3 comments)

http://gerrit.ovirt.org/#/c/25979/7//COMMIT_MSG
Commit Message:

Line 6: 
Line 7: vm: Automatically add a serial port for a console device
Line 8: 
Line 9: Without a serial device the console support does not work for
Line 10: linux systems. We switch from virtio to serial and append a serial
linux systems -> Linux guests

but is that really true? In what way "does it not work"? I thought that virtio
console was problematic on Windows, and actually fine for Linux.
Line 11: port if a console device was defined.
Line 12: 
Line 13: Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f


Line 7: vm: Automatically add a serial port for a console device
Line 8: 
Line 9: Without a serial device the console support does not work for
Line 10: linux systems. We switch from virtio to serial and append a serial
Line 11: port if a console device was defined.
defined -> requirested.
Line 12: 
Line 13: Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f


Line 8: 
Line 9: Without a serial device the console support does not work for
Line 10: linux systems. We switch from virtio to serial and append a serial
Line 11: port if a console device was defined.
Line 12: 
Have you considered possible issues for guest OS that sees a new implementation for its
console if started on a 3.6 vdsm? Please mention that the effects are tolerable. (I
suppose not oVirt users cares about console throughput enough to merit virtio)
Line 13: Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: vm: Automatically add a serial port for a console device
......................................................................


Patch Set 7:

(3 comments)

http://gerrit.ovirt.org/#/c/25979/7//COMMIT_MSG
Commit Message:

Line 6: 
Line 7: vm: Automatically add a serial port for a console device
Line 8: 
Line 9: Without a serial device the console support does not work for
Line 10: linux systems. We switch from virtio to serial and append a serial
>  linux systems -> Linux guests 
The problem on windows is that
it prevented the shutdown for some reason. Otherwise there was no problem.

When using the serial device for the console, I can actually use the console via virsh. I
wasn't able to get it work otherwise. This might be due to the fact that if not using
serial console a special kernel commandline might be required (which I did not verify)
however using the approach of the patch enables me to do this with the configuration of
the guest out of the Box.

Even for FBSD9 I can get the console support working by just enabling the terminal.


When considering http://fedoraproject.org/wiki/Features/VirtioSerial I think the problem
is that you rather have to explicitly spawn the console for the virtio/kvm console device.
Where as when you use 'serial' instead 'it just works (TM)' ;)
Line 11: port if a console device was defined.
Line 12: 
Line 13: Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f


Line 7: vm: Automatically add a serial port for a console device
Line 8: 
Line 9: Without a serial device the console support does not work for
Line 10: linux systems. We switch from virtio to serial and append a serial
Line 11: port if a console device was defined.
>  defined -> requirested. 
Actually the code implements it as
'when it was defined' not 'when it was requested'.
In the sense of 'defined in the generated xml'
Line 12: 
Line 13: Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f


Line 8: 
Line 9: Without a serial device the console support does not work for
Line 10: linux systems. We switch from virtio to serial and append a serial
Line 11: port if a console device was defined.
Line 12: 
>  Have you considered possible issues for guest OS that sees a new
> implementa 
I haven't noticed any issues with that. This patch only applies to
VMs that have the console device enabled already. 

The advantage of using a serial device is that it won't require virtio support for the
console on the guest side.
e.g. using this patch I can enable the serial console on a FreeBSD os and it works, where
as without this it doesn't.

I don't understand your comment in regards to oVirt users and console throughput. I
think that this is a feature which is quite useful to recover from broken network settings
on a guest os or anything without having to reboot the entire VM.
Line 13: Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 8:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/13722/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/12933/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/13885/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1978/ : There was an
infra issue, please contact infra(a)ovirt.org

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

Francesco Romani has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 8:

(2 comments)

I'm fine with the concept, but I'd like to pack as much as possible the creation
of the bits of XML into the Devices.

http://gerrit.ovirt.org/#/c/25979/8/vdsm/virt/vmdevices.py
File vdsm/virt/vmdevices.py:

Line 97:         """
Line 98:         m = self.createXmlElem('console', 'pty')
Line 99:         consoleType = self.specParams.get('consoleType',
'virtio')
Line 100:         m.appendChildWithArgs('target', type=consoleType,
port='0')
Line 101:         return m
Please add a method (getSerialXML?) and move from appendSerialConsole the creation of the
related XML.
Line 102: 
Line 103: 
Line 104: class Controller(Base):
Line 105:     __slots__ = ('address', 'model', 'index',
'master')


http://gerrit.ovirt.org/#/c/25979/8/vdsm/virt/vmxml.py
File vdsm/virt/vmxml.py:

Line 466:         emulator = Element('emulator', text=emulatorPath)
Line 467: 
Line 468:         self._devices.appendChild(emulator)
Line 469: 
Line 470:     def appendConsoleSerial(self):
I'd like more to see this into the Console device, with a method to add the custom XML
when needed, much like GraphicsDevice does with getSpiceVmcChannelsXML
and Drive does with getLeasesXML.
Line 471:         """
Line 472:             Add a serial port for the console device if it exists and is a
Line 473:             'serial' type device.
Line 474: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 9:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/13460/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/2045/ : There was an
infra issue, please contact infra(a)ovirt.org

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/14417/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/14249/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <toni(a)midokura.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

Francesco Romani has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 9: Code-Review+1

(2 comments)

looks ok, thanks for the change.
In the inline comment I suggested a possible readability improvement, but patch looks good
enough already.

http://gerrit.ovirt.org/#/c/25979/9/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 2561:                 domxml._devices.appendChild(graphDev.getSpiceVmcChannelsXML())
Line 2562:                 break
Line 2563: 
Line 2564:         for consoleDev in self._devices[CONSOLE_DEVICES]:
Line 2565:             if consoleDev.specParams.get('consoleType',
'virtio') == 'serial':
with the new property, this will read as

  if consoleDev.isSerial:
     ...
Line 2566:                 domxml._devices.appendChild(consoleDev.getSerialDeviceXML())
Line 2567:                 break
Line 2568: 
Line 2569:         for drive in self._devices[DISK_DEVICES][:]:


http://gerrit.ovirt.org/#/c/25979/9/vdsm/virt/vmdevices.py
File vdsm/virt/vmdevices.py:

Line 78:     def __init__(self, *args, **kwargs):
Line 79:         super(Console, self).__init__(*args, **kwargs)
Line 80: 
Line 81:         if not hasattr(self, 'specParams'):
Line 82:             self.specParams = {}
let's add one property, since the check looks otherwise a little awkward:

  @property
  def isSerial(self):  # or whatever name you find better
    return self.specParams.get('consoleType', 'virtio') ==
'serial'
Line 83: 
Line 84:     def getSerialDeviceXML(self):
Line 85:         """
Line 86:         Add a serial port for the console device if it exists and is a


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

Francesco Romani has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 10: Code-Review+1

thanks, nice!

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 10:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/13467/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/2050/ : There was an
infra issue, please contact infra(a)ovirt.org

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/14424/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/14256/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

Vitor de Lima has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 10:

I don't know if it worth the effort, but ppc64 has another kind of paravirtualized
console, it can be instantiated in the XML like this:

<serial type='pty'>
    <target port='0'/>
    <address type='spapr-vio'/>
</serial>

Do you think it is worth the effort to implement support for this?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Vitor de Lima <vdelima(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

automation(a)ovirt.org has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 11:

* Update tracker::#1139159::OK
* Check Bug-Url::OK
* Check Public Bug::#1139159::OK, public bug
* Check Product::#1139159::OK, Correct product oVirt
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Vitor de Lima <vdelima(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 11:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/15155/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/14986/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/2198/ : There was an
infra issue, please contact infra(a)ovirt.org

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/14198/ : FAILURE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Vitor de Lima <vdelima(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

automation(a)ovirt.org has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 12:

* Update tracker::#1139159::OK
* Check Bug-Url::OK
* Check Public Bug::#1139159::OK, public bug
* Check Product::#1139159::OK, Correct product oVirt
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Vitor de Lima <vdelima(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 12:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/16705/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/16877/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Vitor de Lima <vdelima(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

Francesco Romani has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 12: Code-Review+1

(1 comment)

quite good enough, tiny suggestion inside for further improvement.

https://gerrit.ovirt.org/#/c/25979/12/vdsm/virt/vmdevices/core.py
File vdsm/virt/vmdevices/core.py:

Line 82:             self.specParams = {}
Line 83: 
Line 84:     @property
Line 85:     def isSerial(self):
Line 86:         return self.specParams.get('consoleType', 'virtio') ==
'serial'
a constant instead of string 'serial' or 'virtio' would be nicer
Line 87: 
Line 88:     def getSerialDeviceXML(self):
Line 89:         """
Line 90:         Add a serial port for the console device if it exists and is a


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Vitor de Lima <vdelima(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

automation(a)ovirt.org has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 13:

* Update tracker::#1139159::OK
* Check Bug-Url::OK
* Check Public Bug::#1139159::OK, public bug
* Check Product::#1139159::OK, Correct product oVirt
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Vitor de Lima <vdelima(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 13: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Vitor de Lima <vdelima(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 13: Code-Review+2

raising score

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Vitor de Lima <vdelima(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


vm: Allow to use 'serial' or 'virtio' type consoles

This introduces the optional capability of using 'serial' type
consoles for VMs.
Using a serial console is supported by systems like FreeBSD which
do not yet have virtio character device drivers. Also it is easier
for most administrators to configure, since they quite often know
the required configurations for linux systems to configure the serial
device parameters on the kernel commandline and in the systems settings.

This patch introduces a way too keep backwards compatibility and only
uses 'serial' type consoles when explicitly told so.

Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Bug-Url: https://bugzilla.redhat.com/1139159
Signed-off-by: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Reviewed-on: https://gerrit.ovirt.org/25979
Reviewed-by: Francesco Romani <fromani(a)redhat.com&gt;
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M tests/vmTests.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/virt/vm.py
M vdsm/virt/vmdevices/core.py
4 files changed, 104 insertions(+), 2 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Vinzenz Feenstra: Verified
  Dan Kenigsberg: Looks good to me, approved
  Francesco Romani: Looks good to me, but someone else must approve



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Vitor de Lima <vdelima(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org

Change in vdsm[master]: vm: Allow to use 'serial' or 'virtio' type consoles

automation(a)ovirt.org has posted comments on this change.

Change subject: vm: Allow to use 'serial' or 'virtio' type consoles
......................................................................


Patch Set 14:

* Update tracker::#1139159::OK
* Check TR::#1139159::ERROR, 3.6.0 should not match .*

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa7b02a7bcaad63017c35c811a194fa42e2b694f
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com&gt;
Gerrit-Reviewer: Vitor de Lima <vdelima(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No