Change in vdsm[master]: cpu mode support for vdsm

Laszlo Hornyak has uploaded a new change for review.

Change subject: cpu mode support for vdsm
......................................................................

cpu mode support for vdsm

This patch adds special hostPassthrough and hostModel as cpuType values.
If the cpuType is one of these values, the <model> tag will not be
created for libvirt, instead, a mode attribute will be crated in the
<cpu> tag with value 'host-passthrough' or 'host-model'.

Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Signed-off-by: Laszlo Hornyak 
---
M vdsm/libvirtvm.py
M vdsm_api/vdsmapi-schema.json
2 files changed, 14 insertions(+), 3 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/07/9507/1

diff --git a/vdsm/libvirtvm.py b/vdsm/libvirtvm.py
index f20968f..465a688 100644
--- a/vdsm/libvirtvm.py
+++ b/vdsm/libvirtvm.py
@@ -721,9 +721,17 @@
         model = features[0]
         cpu = self.doc.createElement('cpu')
         cpu.setAttribute('match', 'exact')
-        m = self.doc.createElement('model')
-        m.appendChild(self.doc.createTextNode(model))
-        cpu.appendChild(m)
+
+        #and now for something completely different
+        if model == 'hostPassthrough':
+            cpu.setAttribute('mode', 'host-passthrough')
+        elif model == 'hostModel':
+            cpu.setAttribute('mode', 'host-model')
+        else:
+            m = self.doc.createElement('model')
+            m.appendChild(self.doc.createTextNode(model))
+            cpu.appendChild(m)
+
         if ('smpCoresPerSocket' in self.conf or
             'smpThreadsPerCore' in self.conf):
             topo = self.doc.createElement('topology')
diff --git a/vdsm_api/vdsmapi-schema.json b/vdsm_api/vdsmapi-schema.json
index aa45059..dbfe153 100644
--- a/vdsm_api/vdsmapi-schema.json
+++ b/vdsm_api/vdsmapi-schema.json
@@ -2383,6 +2383,9 @@
 # @clientIp:              The IP address of the client connected to the display
 #
 # @cpuType:               The type of CPU being emulated
+#                         special values 'hostPassthrough' and 'hostModel'
+#                         are reserved for host-passthrough and host-mode cpu
+#                         mode
 #
 # @custom:                A dictionary of custom, free-form properties
 #

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/161/ (1/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/195/ (2/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/161/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/195/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: oVirt Jenkins CI Server

Doron Fediuck has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 1: (1 inline comment)

....................................................
File vdsm/libvirtvm.py
Line 733:             cpu.appendChild(m)
Line 734: 
Line 735:         if ('smpCoresPerSocket' in self.conf or
Line 736:             'smpThreadsPerCore' in self.conf):
Line 737:             topo = self.doc.createElement('topology')
Laszlo, 
IIUC, pass-through according to the libvirt docs looks like this:

<cpu mode='host-passthrough'/>

which means all other <cpu> child-nodes are ignored. So the above topology may be ignored. Unsure how cpu-pinning behaves. I suggest testing it first with virsh or similar.
Line 738:             vcpus = int(self.conf.get('smp', '1'))
Line 739:             cores = int(self.conf.get('smpCoresPerSocket', '1'))
Line 740:             threads = int(self.conf.get('smpThreadsPerCore', '1'))
Line 741:             topo.setAttribute('sockets', str(vcpus / cores / threads))

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: oVirt Jenkins CI Server

Laszlo Hornyak has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 1: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/libvirtvm.py
Line 733:             cpu.appendChild(m)
Line 734: 
Line 735:         if ('smpCoresPerSocket' in self.conf or
Line 736:             'smpThreadsPerCore' in self.conf):
Line 737:             topo = self.doc.createElement('topology')
Yes, this is in work in progress status, just to share with Dan and you if you like it more as the other solution. (I still prefer that)
Line 738:             vcpus = int(self.conf.get('smp', '1'))
Line 739:             cores = int(self.conf.get('smpCoresPerSocket', '1'))
Line 740:             threads = int(self.conf.get('smpThreadsPerCore', '1'))
Line 741:             topo.setAttribute('sockets', str(vcpus / cores / threads))

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: oVirt Jenkins CI Server

Doron Fediuck has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 1: (1 inline comment)

....................................................
File vdsm/libvirtvm.py
Line 733:             cpu.appendChild(m)
Line 734: 
Line 735:         if ('smpCoresPerSocket' in self.conf or
Line 736:             'smpThreadsPerCore' in self.conf):
Line 737:             topo = self.doc.createElement('topology')
Sure. I agree with the concept.
Line 738:             vcpus = int(self.conf.get('smp', '1'))
Line 739:             cores = int(self.conf.get('smpCoresPerSocket', '1'))
Line 740:             threads = int(self.conf.get('smpThreadsPerCore', '1'))
Line 741:             topo.setAttribute('sockets', str(vcpus / cores / threads))

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: oVirt Jenkins CI Server

Laszlo Hornyak has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 1:

Dan, si this approach ok for you?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: oVirt Jenkins CI Server

Laszlo Hornyak has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 1: (1 inline comment)

....................................................
File vdsm/libvirtvm.py
Line 733:             cpu.appendChild(m)
Line 734: 
Line 735:         if ('smpCoresPerSocket' in self.conf or
Line 736:             'smpThreadsPerCore' in self.conf):
Line 737:             topo = self.doc.createElement('topology')
cpu-pinnng, with
        <cputune>
                <vcpupin cpuset="1" vcpu="1"/>
                <vcpupin cpuset="0" vcpu="0"/>
        </cputune>
        <cpu match="exact" mode="host-passthrough">
                <topology cores="1" sockets="2" threads="1"/>
        </cpu>

working fine
Line 738:             vcpus = int(self.conf.get('smp', '1'))
Line 739:             cores = int(self.conf.get('smpCoresPerSocket', '1'))
Line 740:             threads = int(self.conf.get('smpThreadsPerCore', '1'))
Line 741:             topo.setAttribute('sockets', str(vcpus / cores / threads))

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: oVirt Jenkins CI Server

Doron Fediuck has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 1: Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: oVirt Jenkins CI Server

Royce Lv has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 1: I would prefer that you didn't submit this

(3 inline comments)

I like the concept, just minor comments

....................................................
Commit Message
Line 7: cpu mode support for vdsm
Line 8: 
Line 9: This patch adds special hostPassthrough and hostModel as cpuType values.
Line 10: If the cpuType is one of these values, the <model> tag will not be
Line 11: created for libvirt, instead, a mode attribute will be crated in the
"crated"-->"created":)
Line 12: <cpu> tag with value 'host-passthrough' or 'host-model'.
Line 13: 
Line 14: Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021

....................................................
File vdsm/libvirtvm.py
Line 709:         """
Line 710:         Add guest CPU definition.
Line 711: 
Line 712:         <cpu match="exact">
Line 713:             <model>qemu64</model>
Maybe we can update the example in comments to make it more clear?
Line 714:             <topology sockets="S" cores="C" threads="T"/>
Line 715:             <feature policy="require" name="sse2"/>
Line 716:             <feature policy="disable" name="svm"/>
Line 717:         </cpu>

Line 719: 
Line 720:         features = self.conf.get('cpuType', 'qemu64').split(',')
Line 721:         model = features[0]
Line 722:         cpu = self.doc.createElement('cpu')
Line 723:         cpu.setAttribute('match', 'exact')
For 'host-model' mode:Neither match attribute nor any feature elements can be used in this mode.Since 0.8.5 the match attribute can be omitted and will default to exact. May be we can just omit this?
Line 724: 
Line 725:         #and now for something completely different
Line 726:         if model == 'hostPassthrough':
Line 727:             cpu.setAttribute('mode', 'host-passthrough')

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

Laszlo Hornyak has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 1: (3 inline comments)

....................................................
Commit Message
Line 7: cpu mode support for vdsm
Line 8: 
Line 9: This patch adds special hostPassthrough and hostModel as cpuType values.
Line 10: If the cpuType is one of these values, the <model> tag will not be
Line 11: created for libvirt, instead, a mode attribute will be crated in the
indeed, thx!
Line 12: <cpu> tag with value 'host-passthrough' or 'host-model'.
Line 13: 
Line 14: Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021

....................................................
File vdsm/libvirtvm.py
Line 709:         """
Line 710:         Add guest CPU definition.
Line 711: 
Line 712:         <cpu match="exact">
Line 713:             <model>qemu64</model>
I would rather add another example as cpu-host is a tuning option and probably most users will not use it, so it would be better to keep this example simple and add a more complex one for host-cpu.
Line 714:             <topology sockets="S" cores="C" threads="T"/>
Line 715:             <feature policy="require" name="sse2"/>
Line 716:             <feature policy="disable" name="svm"/>
Line 717:         </cpu>

Line 719: 
Line 720:         features = self.conf.get('cpuType', 'qemu64').split(',')
Line 721:         model = features[0]
Line 722:         cpu = self.doc.createElement('cpu')
Line 723:         cpu.setAttribute('match', 'exact')
If I understand you correctly, it makes completely sense to omit that line, but it is out of scope for this patch.
Line 724: 
Line 725:         #and now for something completely different
Line 726:         if model == 'hostPassthrough':
Line 727:             cpu.setAttribute('mode', 'host-passthrough')

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

Laszlo Hornyak has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 3: I would prefer that you didn't submit this

work in progress, waiting for feedback from vdsm team

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

Laszlo Hornyak has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 3: No score

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

Doron Fediuck has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 3: Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

Laszlo Hornyak has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 3: Verified

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

Adam Litke has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 3: Looks good to me, but someone else must approve

(1 inline comment)

....................................................
File vdsm_api/vdsmapi-schema.json
Line 2384: #
Line 2385: # @cpuType:               The type of CPU being emulated
Line 2386: #                         special values 'hostPassthrough' and 'hostModel'
Line 2387: #                         are reserved for host-passthrough and host-mode cpu
Line 2388: #                         mode
Thanks for adding this to the documentation!
Line 2389: #
Line 2390: # @custom:                A dictionary of custom, free-form properties
Line 2391: #
Line 2392: # @devices:               An array of VM devices present

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

Royce Lv has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 3: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/libvirtvm.py
Line 719: 
Line 720:         features = self.conf.get('cpuType', 'qemu64').split(',')
Line 721:         model = features[0]
Line 722:         cpu = self.doc.createElement('cpu')
Line 723:         cpu.setAttribute('match', 'exact')
I mean, quote from http://libvirt.org/formatdomain.html#elementsCPU "host-model"--"Neither match attribute nor any feature elements can be used in this mode"

So 
<cpu match="exact" mode="host-model">
    <topology cores="1" sockets="2" threads="1"/>
</cpu>

is not right according to libvirt document, and this is what the xml will be like when 'model'=='host-model'
Line 724: 
Line 725:         #and now for something completely different
Line 726:         if model == 'hostPassthrough':
Line 727:             cpu.setAttribute('mode', 'host-passthrough')

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

Mark Wu has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 3: I would prefer that you didn't submit this

(3 inline comments)

....................................................
File vdsm/libvirtvm.py
Line 719: 
Line 720:         features = self.conf.get('cpuType', 'qemu64').split(',')
Line 721:         model = features[0]
Line 722:         cpu = self.doc.createElement('cpu')
Line 723:         cpu.setAttribute('match', 'exact')
Actually,  that's ok.  In the mode of 'host-model' and 'host-passthrough', the 'match' attribute will be ignored. But I don't like to rely on it. According to libvirt doc, since 0.8.5 the match attribute can be omitted and will default to exact.  So I think it should be safe to remove the 'match' attribute.

Plus,  I also have a concern about the live migration, especially for the host-passthrough mode.  I am not sure if engine requires the hosts in a cluster have totally the same cpu model.
Line 724: 
Line 725:         #and now for something completely different
Line 726:         if model == 'hostPassthrough':
Line 727:             cpu.setAttribute('mode', 'host-passthrough')

Line 721:         model = features[0]
Line 722:         cpu = self.doc.createElement('cpu')
Line 723:         cpu.setAttribute('match', 'exact')
Line 724: 
Line 725:         #and now for something completely different
Could you please explain what does this comment mean?  and why do you need it?
Line 726:         if model == 'hostPassthrough':
Line 727:             cpu.setAttribute('mode', 'host-passthrough')
Line 728:         elif model == 'hostModel':
Line 729:             cpu.setAttribute('mode', 'host-model')

Line 725:         #and now for something completely different
Line 726:         if model == 'hostPassthrough':
Line 727:             cpu.setAttribute('mode', 'host-passthrough')
Line 728:         elif model == 'hostModel':
Line 729:             cpu.setAttribute('mode', 'host-model')
Probably, you also need validate the variable 'features'.  If user specify cpu mode and also some features,  libvirt will fail to start vm with error of 'Non-empty feature list specified without CPU model'
Line 730:         else:
Line 731:             m = self.doc.createElement('model')
Line 732:             m.appendChild(self.doc.createTextNode(model))
Line 733:             cpu.appendChild(m)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

Laszlo Hornyak has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 3: (2 inline comments)

....................................................
File vdsm/libvirtvm.py
Line 721:         model = features[0]
Line 722:         cpu = self.doc.createElement('cpu')
Line 723:         cpu.setAttribute('match', 'exact')
Line 724: 
Line 725:         #and now for something completely different
It makes people want to know.
Line 726:         if model == 'hostPassthrough':
Line 727:             cpu.setAttribute('mode', 'host-passthrough')
Line 728:         elif model == 'hostModel':
Line 729:             cpu.setAttribute('mode', 'host-model')

Line 725:         #and now for something completely different
Line 726:         if model == 'hostPassthrough':
Line 727:             cpu.setAttribute('mode', 'host-passthrough')
Line 728:         elif model == 'hostModel':
Line 729:             cpu.setAttribute('mode', 'host-model')
Done
Line 730:         else:
Line 731:             m = self.doc.createElement('model')
Line 732:             m.appendChild(self.doc.createTextNode(model))
Line 733:             cpu.appendChild(m)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 4:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/232/ (2/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 4:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/266/ (1/2)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 4:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/232/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/266/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

Laszlo Hornyak has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 4: Verified

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

Royce Lv has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 4: Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: cpu mode support for vdsm
......................................................................

Patch Set 4: Looks good to me, approved

(2 inline comments)

....................................................
File tests/libvirtvmTests.py
Line 145:               <model>Opteron_G4</model>
Line 146:               <feature name="sse4.1" policy="require"/>
Line 147:               <feature name="sse4.2" policy="require"/>
Line 148:               <feature name="svm" policy="disable"/>
Line 149:               <topology cores="2" sockets="2" threads="2"/>
who cared about this ordering? I do not.
Line 150:           </cpu> """
Line 151:         cputuneXML = """
Line 152:           <cputune>
Line 153:               <vcpupin cpuset="2-3" vcpu="1"/>

....................................................
File vdsm/libvirtvm.py
Line 726:             cpu.setAttribute('mode', 'host-passthrough')
Line 727:         elif model == 'hostModel':
Line 728:             cpu.setAttribute('mode', 'host-model')
Line 729:         else:
Line 730:             m = self.doc.createElement('model')
heh, a gerrit bug does not show the fixed indentation.
Line 731:             m.appendChild(self.doc.createTextNode(model))
Line 732:             cpu.appendChild(m)
Line 733: 
Line 734:             # This hack is for backward compatibility as the libvirt

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: cpu mode support for vdsm
......................................................................

cpu mode support for vdsm

This patch adds special hostPassthrough and hostModel as cpuType values.
If the cpuType is one of these values, the <model> tag will not be
created for libvirt, instead, a mode attribute will be created in the
<cpu> tag with value 'host-passthrough' or 'host-model'.

Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Signed-off-by: Laszlo Hornyak 
---
M tests/libvirtvmTests.py
M vdsm/libvirtvm.py
M vdsm_api/vdsmapi-schema.json
3 files changed, 33 insertions(+), 22 deletions(-)

Approvals:
  Laszlo Hornyak: Verified
  Royce Lv: Looks good to me, but someone else must approve
  Dan Kenigsberg: Looks good to me, approved

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I54fb440ef52255f0a7933b000b9b599c2d056021
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Doron Fediuck 
Gerrit-Reviewer: Laszlo Hornyak 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Royce Lv 
Gerrit-Reviewer: oVirt Jenkins CI Server