| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-04-21 | |||
| 14:03:08 | gibi | kashyap: sure, more viewpoint helps | |
| 14:06:02 | sean-k-mooney | the feature listed in virsh capabliteis are the cpu feature on the host | |
| 14:06:24 | sean-k-mooney | and the model listed is the closes standard model to the host feature set | |
| 14:06:45 | kashyap | Yeah, as I wrote in my small primer on differences: | |
| 14:06:50 | sean-k-mooney | wehn we use dom caps that actully provides what will be used by the vm when that model is selected | |
| 14:07:04 | kashyap | sean-k-mooney: No, not "what will be used by the VM" - rather: | |
| 14:07:43 | sean-k-mooney | its providing the content of what we will be generted in the libvirt domain xml | |
| 14:07:48 | sean-k-mooney | if we requested host model | |
| 14:08:15 | sean-k-mooney | which is what i ment by "what will be used" in my previous sentence | |
| 14:08:16 | kashyap | sean-k-mooney: It answers two questions: (a) whether a feature is supported by the host; and (b) whether it is supported by a given QEMU binary running on the host | |
| 14:09:16 | sean-k-mooney | artom: gibi by the way i can confirm that https://review.opendev.org/c/openstack/os-brick/+/838871 fixes the unit test failures | |
| 14:09:52 | sean-k-mooney | so this backport https://review.opendev.org/q/topic:bug%252F1947370 | |
| 14:09:58 | sean-k-mooney | is what causes the lock path issues | |
| 14:10:14 | sean-k-mooney | the reviert is not nessisarly the correct approch | |
| 14:10:25 | sean-k-mooney | but the backport made a previously optional config option required | |
| 14:10:39 | sean-k-mooney | we neither set the config option in our test or the enviornmental variable | |
| 14:10:56 | sean-k-mooney | so strictly speaking i do no think os-brick shoudl have backported that change as written | |
| 14:11:40 | sean-k-mooney | it did not comply with satable policy since it requirs config to be updated. | |
| 14:11:53 | sean-k-mooney | elodilles:^ correct me if that is a wrong intepretation | |
| 14:12:21 | sean-k-mooney | if they provided a sane default then i think it woudl have been fine | |
| 14:12:49 | sean-k-mooney | like /run/os-brick/lock or /tmp/os-brick/lock | |
| 14:13:10 | sean-k-mooney | but in its current form its a breaking change for any deployemnt that did not set that | |
| 14:40:28 | sean-k-mooney | artom: gibi https://bugs.launchpad.net/os-brick/+bug/1969794 | |
| 14:53:35 | elodilles | sean-k-mooney: yepp, that's against stable policy and was discussed here with cinder team: https://review.opendev.org/c/openstack/releases/+/829590 | |
| 14:54:32 | gibi | sean-k-mooney: good catch | |
| 14:55:32 | sean-k-mooney | so the option is form oslo_concurrency | |
| 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 | |