11:16 a.m.
Hans De Goede has uploaded a new change for review.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
libvirtvm.py: Extend controller support to allow configuring USB controllers
This allows creating a vm with a USB-2.0 (EHCI) controller with USB-1.1
companion controllers (UHCI), ie:
vdsClient 0 create /dev/null \
vmId=00000001-0002-0003-0004-000000000006 vmName=F14-x86_64 memSize=512 \
macAddr=52:54:00:7a:b4:7e nicModel=virtio bridge=test display=qxl \
spiceDisableTicketing=1 \
devices='{type:disk,device:disk,path:/home/virt-images/f14-i386.qcow2.img,propagateErrors:off,iface:ide,index:0,readonly:False,format:cow}'
\
devices='{type:video,device:qxl,specParams:{vram:65536},address:{type:pci,domain:0x0000,bus:0x00,slot:0x02,function:0x0}}'
\
devices='{type:controller,device:virtio-serial,address:{type:pci,domain:0x0000,bus:0x00,slot:0x03,function:0x0}}'
\
devices='{type:controller,device:usb,model:ich9-ehci1,address:{type:pci,domain:0x0000,bus:0x00,slot:0x04,function:0x7}}'
\
devices='{type:controller,device:usb,model:ich9-uhci1,master:{startport:0},address:{type:pci,domain:0x0000,bus:0x00,slot:0x04,function:0x0,multifunction:on}}'
\
devices='{type:controller,device:usb,model:ich9-uhci2,master:{startport:2},address:{type:pci,domain:0x0000,bus:0x00,slot:0x04,function:0x1}}'
\
devices='{type:controller,device:usb,model:ich9-uhci3,master:{startport:4},address:{type:pci,domain:0x0000,bus:0x00,slot:0x04,function:0x2}}'
Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Signed-off-by: Hans de Goede <hdegoede(a)redhat.com>
---
M vdsm/libvirtvm.py
1 file changed, 7 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/95/4095/1
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
9:05 a.m.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
libvirtvm.py: Extend controller support to allow configuring USB controllers
This allows creating a vm with a USB-2.0 (EHCI) controller with USB-1.1
companion controllers (UHCI), ie:
vdsClient 0 create /dev/null \
vmId=00000001-0002-0003-0004-000000000006 vmName=F14-x86_64 memSize=512 \
macAddr=52:54:00:7a:b4:7e nicModel=virtio bridge=test display=qxl \
spiceDisableTicketing=1 \
devices='{type:disk,device:disk,path:/home/virt-images/f14-i386.qcow2.img,propagateErrors:off,iface:ide,index:0,readonly:False,format:cow}'
\
devices='{type:video,device:qxl,specParams:{vram:65536},address:{type:pci,domain:0x0000,bus:0x00,slot:0x02,function:0x0}}'
\
devices='{type:controller,device:virtio-serial,address:{type:pci,domain:0x0000,bus:0x00,slot:0x03,function:0x0}}'
\
devices='{type:controller,device:usb,model:ich9-ehci1,address:{type:pci,domain:0x0000,bus:0x00,slot:0x04,function:0x7}}'
\
devices='{type:controller,device:usb,model:ich9-uhci1,master:{startport:0},address:{type:pci,domain:0x0000,bus:0x00,slot:0x04,function:0x0,multifunction:on}}'
\
devices='{type:controller,device:usb,model:ich9-uhci2,master:{startport:2},address:{type:pci,domain:0x0000,bus:0x00,slot:0x04,function:0x1}}'
\
devices='{type:controller,device:usb,model:ich9-uhci3,master:{startport:4},address:{type:pci,domain:0x0000,bus:0x00,slot:0x04,function:0x2}}'
Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Signed-off-by: Hans de Goede <hdegoede(a)redhat.com>
---
M vdsm/libvirtvm.py
1 file changed, 2 insertions(+), 1 deletion(-)
Approvals:
Dan Kenigsberg: Verified; Looks good to me, approved
Igor Lvovsky: Looks good to me, but someone else must approve
Oved Ourfali: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eli Mesika <emesika(a)redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
9:05 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
Patch Set 4: Verified; Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eli Mesika <emesika(a)redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
5:50 a.m.
Oved Ourfali has posted comments on this change.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
Patch Set 4: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eli Mesika <emesika(a)redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
5:46 a.m.
Igor Lvovsky has posted comments on this change.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
Patch Set 4: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eli Mesika <emesika(a)redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
2:51 a.m.
Hans De Goede has posted comments on this change.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
Patch Set 3: (2 inline comments)
....................................................
File vdsm/libvirtvm.py
Line 864: doc = xml.dom.minidom.Document()
Done
Line 866: ['index', 'model',
'master', 'address'])
master may be necessary to be able to set a different startport then the default if we
ever want to do that, address will get assigned automatically by libvirt the first time
the vm is created, but AFAIK after the first time ovirt will store the given address and
specify it the next time so that hardware does not move around when a vm gets re-created
later. Note that specifying of either is optional, if you don't specify any of these
they simply won't be used.
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eli Mesika <emesika(a)redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
8:08 a.m.
Oved Ourfali has posted comments on this change.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
Patch Set 3: (2 inline comments)
....................................................
File vdsm/libvirtvm.py
Line 864: doc = xml.dom.minidom.Document()
Remove this line, as doc isn't used in this function.
Line 866: ['index', 'model',
'master', 'address'])
Will we need the master and address, now that it is part of libvirt's responsibility?
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eli Mesika <emesika(a)redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
6:15 a.m.
Oved Ourfali has posted comments on this change.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
Patch Set 3:
If working with usb 2.0 only, what devices do we need?
Is:
devices='{type:controller,device:usb,model:ich9-ehci1,address:{type:pci,domain:0x0000,bus:0x00,slot:0x04,function:0x7}}'
and the redirect devices:
devices='{type:redir,device:spicevmc,bus:usb,address:{type:usb,bus:0,port:1}}'
devices='{type:redir,device:spicevmc,bus:usb,address:{type:usb,bus:0,port:2}}'
devices='{type:redir,device:spicevmc,bus:usb,address:{type:usb,bus:0,port:3}}'
....
....
....
enough?
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eli Mesika <emesika(a)redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofrenkel(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
5:45 a.m.
Hans De Goede has posted comments on this change.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
Patch Set 3:
Correct, the engine would need to provide a master dict, with a startport entry in there.
As for the engine not being able to handle this, this is following the same structure as
the already existing address support is.
More importantly it closely mirrors the libvirt-xml structure. Now that RHEV is using
libvirt for all newer versions, I don't think it makes sense to put a lot of
translation between the xmlrpc contents for vm configuration and the libvirt xml.
The closer the 2 are together, the easier it will be to extend vdsm to support new devices
supported by libvirt. As shown by how little code this patch changes.
IOW I believe the engine should adapt to generate xmlrpc calls with parameters closely
matching the libvirt-xml syntax. The engine needs to be modified anyways whenever new
devices need to be supported. Why then not make
these changes in such a way that no translation needs to be done by vdsm, eliminating a
possible source of errors?
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eli Mesika <emesika(a)redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofrenkel(a)redhat.com>
4 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
Patch Set 3: I would prefer that you didn't submit this
so, startpoint would be provided by Engine? Are they aware of this API? I am not sure
they can handle this, they wanted an ugly specParam dictionary per device.
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eli Mesika <emesika(a)redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
3:42 a.m.
Igor Lvovsky has posted comments on this change.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
Patch Set 3: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
10:20 a.m.
Hans De Goede has posted comments on this change.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
Patch Set 3: Verified
Tested and works for me.
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
6:31 a.m.
Hans De Goede has posted comments on this change.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
Patch Set 2: (1 inline comment)
....................................................
File vdsm/libvirtvm.py
Line 866: ['index', 'model',
'master', 'address'])
startport is not an attribute of the controller element itself, but an attribute of the
master child element. Since master, like address is a child element its representation in
the xmlrpc will be a dict. And like with address all name:value pairs in the dict will get
set as attributes on the master child element.
Notice how the value of master is a dict here:
devices='{type:controller,device:usb,model:ich9-uhci3,master:{startport:4},address:{...}}'
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
3:32 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(1 inline comment)
....................................................
File vdsm/libvirtvm.py
Line 866: ['index', 'model',
'master', 'address'])
the commit message is long, the change is small, but I do not undestand where the
"startport" text is supposed to come from...
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
10:37 a.m.
Hans De Goede has posted comments on this change.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
Patch Set 2:
Yes we also need redirdev support, see the new version of the set which I've just
uploaded this, which also adds redirdev support
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
9:57 a.m.
Igor Lvovsky has posted comments on this change.
Change subject: libvirtvm.py: Extend controller support to allow configuring USB
controllers
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(2 inline comments)
Do we need to add USB Redirected devices as well?
In separate patch of course.
....................................................
File vdsm/libvirtvm.py
Line 861: ctrl.setAttribute('model', self.model)
As I can see from libvirt definition, you will need to add 'index' attribute as
well:
http://libvirt.org/formatdomain.html#elementsControllers
<devices>
<controller type='usb' index='0' model='ich9-ehci1'>
<address type='pci' domain='0' bus='0' slot='4'
function='7'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci1'>
<master startport='0'/>
<address type='pci' domain='0' bus='0' slot='4'
function='0' multifunction='on'/>
</controller>
...
</devices>
Line 871: ctrl.appendChild(master)
Is it necessary to handle 'master' as dictionary?
Can it be something else except single
<master startport='0'/> ?
--
To view, visit http://gerrit.ovirt.org/4095
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4fd6bed73cfa67fd6b29d3c352aebc7c25cff5f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hans De Goede <hdegoede(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
4424
days inactive
4438
days old
vdsm-patches@lists.fedorahosted.org
15 comments
4 participants
participants (4)
-
Dan Kenigsberg -
Hans de Goede -
Igor Lvovsky -
Oved Ourfali