Earlier  
Posted Nick Remark
#openstack-nova - 2022-04-21
14:55:40 sean-k-mooney not form os-brick or nova
14:55:47 sean-k-mooney so we can adress this in a few ways
14:56:03 sean-k-mooney in nova we could jsut set the env varable to somethign sane in the tox.ini
14:56:21 sean-k-mooney that woudl be a minimal fix we could also mock some of the calls in a fixture
14:56:44 sean-k-mooney apprently both nova and cinder docuemtn that this shoudl be set in the config in our install guides
14:56:55 sean-k-mooney so in practice all cloud should have the config option set
14:57:11 gibi so I assume nova also uses that config option for its of file lock
14:57:12 sean-k-mooney elodilles: gibi any prefernce on how to proceed.
14:57:20 sean-k-mooney yes
14:57:23 sean-k-mooney we do
14:57:42 gibi so somewhere in the nova test env we already provide a default for that option
14:57:44 sean-k-mooney and os-brick is runnign in novas process space so its reading our config value
14:58:04 sean-k-mooney gibi: that or we mock the locking code
14:58:09 gibi yeah
14:58:09 sean-k-mooney so it does not write to disk
14:58:27 gibi we already do something so that the nova usage of that opt is covered
14:58:28 sean-k-mooney in this case since its failing its actully trying to create a file systm lock in the unit test
14:58:31 sean-k-mooney which it shoudl not be
14:58:48 sean-k-mooney i woudl guess we are just missing a fixture in those tets
14:59:00 gibi yeah that is what I think too
14:59:09 gibi we need to find what is missing and extend the fixture
15:13:02 kashyap gibi: To tie up the lose end on caps vs domCaps -- the guidance from the libvirt folks is (a) no, we can't treat the caps == domCaps w/ 'host-model' mode; and (b) we should use domCaps wherever possible.
15:13:23 kashyap gibi: So that means, we should introduce a new config object
15:13:32 gibi kashyap: ack, make sense
15:14:02 kashyap gibi: I might need some help on this XML parsing ... I'll take a stab
15:14:21 kashyap ... at it.
15:14:42 gibi kashyap: just ping me if you need another set of eyes
15:14:57 kashyap Will do; thx
15:59:14 mnaser i would appreciate some reviews on https://review.opendev.org/c/openstack/nova/+/830646 (wrt allowing addition of viommu to vms)
16:13:35 bauzas mnaser: you're not the first one to ask for reviews on that stephenfin's old patch, will mark it as review priority for the team
16:14:10 mnaser bauzas: cool thanks, the other one might have been ricolin but thats coming from the same side, so not sure if thats super fair ;) haha
16:14:34 bauzas that being said, the bp isn't validated yet
16:15:14 bauzas and not sure stephenfin will unghost himself :)
16:15:39 stephenfin bauzas: I think mnaser and ricolin are taking care of it now?
16:16:00 bauzas good question
16:16:14 mnaser yeah it's ready (imho) from a code perspective, but if there's something else we have to do, we can take care of
16:17:20 bauzas mnaser: this is just a paperwork question
16:17:33 bauzas since a new extraspec is added + the api validation
16:17:42 bauzas this has to be tracked correctly
16:17:55 bauzas and we need to balance in a meeting whether we need a spec or not
16:18:40 bauzas mnaser: that being said, I'm more than glad to welcome you as a new feature contributor ! :p
16:19:03 bauzas we don't have badges but I can create one for Berlin :p
16:19:23 stephenfin I can't say if it needs a spec, but it definitely needs a blueprint and some discussion in the meeting
16:19:36 stephenfin given there's now an extra spec (I think?
16:19:38 stephenfin )
16:20:34 bauzas yeah and yeah
16:20:45 bauzas (needs a blueprint and probably don't need a spec)
16:21:26 bauzas stephenfin: have you just added some fancy gerrit topic for some non-existing related blueprint ?
16:21:43 bauzas if so, you're bragging.
16:22:00 bauzas :p
16:37:19 opendevreview Kashyap Chamarthy proposed openstack/nova master: libvirt: Add workaround to remove compareCPU() check on the destination https://review.opendev.org/c/openstack/nova/+/838926
16:40:37 opendevreview Kashyap Chamarthy proposed openstack/nova master: libvirt: Add workaround to skip compareCPU() check on the destination https://review.opendev.org/c/openstack/nova/+/838926
17:08:48 sean-k-mooney mnaser: viommu is not useful without a lot of extra work
17:08:57 sean-k-mooney stephenfin: that definetly needs a spec
17:09:24 sean-k-mooney viommu is trivial to enable i have implemeted that locally before
17:09:43 sean-k-mooney mnaser: the issue is all devices will be in the same iommu group
17:10:20 sean-k-mooney so either you disable the memory isolation in the guest vfio-pci module
17:10:35 sean-k-mooney or nova has to take full contol of the pcie toplogy
17:10:52 sean-k-mooney mnaser: stephenfin did some poc work related to this
17:11:03 sean-k-mooney but i dont think its complete
17:11:15 sean-k-mooney and it had upgrade impacts potentially
17:11:21 stephenfin far from complete
17:11:35 sean-k-mooney so it needs a spec to figure that out and more expirece playing with the poc
17:12:00 sean-k-mooney mnaser: by the way apprently the prefomcne of the viommu is really bad with passthough devices
17:12:13 sean-k-mooney im not sure why but that is what the virt team told me in the past
17:12:58 sean-k-mooney hopefully that has changed mnaser have you done any tests locally with livbirt?
17:13:27 mnaser sean-k-mooney: so, my knowledge level to this is that we have a company which ships pci cards which are hardware accelerators
17:13:58 mnaser sean-k-mooney: as we tried to get them to use native openstack, we figured out the extra qemu parameters they had and it included the iommu stuff
17:14:10 mnaser Supposedly, the cards won’t work without them in PCI pass through in the guests.
17:14:22 sean-k-mooney ack
17:14:31 sean-k-mooney what driver to you use in the guest
17:14:41 sean-k-mooney is it vfio-pci or a normal kernel dirver
17:14:58 sean-k-mooney if they dont need to be in there own iommu group the stephenfin patch would enable your usecase
17:15:04 mnaser I think you do bring up a good point that we’re adding it by default to q35 machine types, so I think making another flag to make it opt in might be good
17:15:17 sean-k-mooney so we could perhaps start with the simpel turn this one feature
17:15:44 sean-k-mooney we could possble add it by default
17:16:01 sean-k-mooney but what we cant do by defualt is change the pci layout to put each device in its own iommu group
17:16:11 sean-k-mooney at least not without a lot of testing
17:16:18 mnaser Yeah I don’t think this is what the device needs in this case
17:16:32 sean-k-mooney havign a hw:viommu=on|off extra spec if simple
17:17:14 sean-k-mooney ok stephenfin i would be ok with a specless bluepint for just adding the extraspec/image propety for this
17:17:38 sean-k-mooney but if we were to do the iommu group split out that would need the spec
17:18:38 sean-k-mooney mnaser: tldr if we want each device to be in its own iommu group we need to create a pcie expantion bridge per device. and to do that we need to assign virtual pci devivie adress to every device in the xml
17:19:10 sean-k-mooney mnaser: so that is the bit that has upgade concerns as basically every device in the vm would change its address potentially
17:21:04 sean-k-mooney mnaser: have you tried stephenfin's patch with that card?
17:22:02 sean-k-mooney mnaser: im not sure im comfortable with exposeing the adress with by the way
17:22:08 sean-k-mooney that feel a bit too low level
17:30:50 mnaser sean-k-mooney: i haven't yet, but it's kindof a reimplementation of how they've done things with flat libvirt
17:31:05 mnaser but i am thinknig hw:viommu=on|off is a good idea
17:31:11 sean-k-mooney im about to -1 the patch
17:31:21 mnaser in terms of controlling the address, i'm thinking since it's a flavor extra spec and not a image extra spec, it's an operator-level decision
17:31:25 mnaser okay great
17:31:32 sean-k-mooney no
17:31:39 mnaser we'll discuss further there and ill try to poc something as well
17:31:47 sean-k-mooney so image extra specs are ment to be used for contoleing emulated hardware
17:31:49 sean-k-mooney not extra specs
17:32:01 sean-k-mooney so in generaly this shoudl only be exposed as an image property
17:32:17 sean-k-mooney but if we are to expose it in the image i would be open to having it in both
17:33:02 sean-k-mooney so i think we should have somehtin like hw_viommu_modle=none|intel|smmuv3|virtio in the image to contol turing this on
17:33:22 sean-k-mooney and i woudl be open to also haveing hw:viommu_modle=none|intel|smmuv3|virtio for partiy in the flavor

Earlier   Later