| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-11 | |||
| 13:06:31 | alex_xu | I talk that with bauzas yesterday | |
| 13:06:44 | efried | ah, okay. So it sounds like vgpu needs to be moved to instance_claim as well | |
| 13:06:58 | efried | do we have a bug for that yet? | |
| 13:07:54 | alex_xu | not yet, I can file a bug | |
| 13:10:25 | efried | alex_xu: hold on, isn't COMPUTE_RESOURCES_SEMAPHORE held throughout the `with instance_claim` block? | |
| 13:10:57 | efried | oh, no, never mind | |
| 13:11:07 | efried | instance_claim isn't itself a context manager; it returns one. | |
| 13:12:08 | alex_xu | efried: here is the bug, that help you understand the problem https://bugs.launchpad.net/nova/+bug/1836204 | |
| 13:12:08 | openstack | Launchpad bug 1836204 in OpenStack Compute (nova) "The allocation of VGPU has race problem" [Undecided,New] | |
| 13:13:19 | alex_xu | efried: also the vpmem isn't quite same with vgpu in the resize case, we need to allcoate vpmem for migration the data from the src host. so we need to assign a pmem in the beginning of resize https://review.opendev.org/#/c/662709/4/nova/compute/manager.py@4359 | |
| 13:14:07 | efried | in that case do we want to fold it into resize_claim? | |
| 13:14:38 | alex_xu | yes, and that doesn't sounds good we add new virt interface just for allocae a pmem. | |
| 13:14:58 | alex_xu | and there isn't a way to resolve the race problem just by listing libvirt domain | |
| 13:15:18 | efried | alex_xu: We shouldn't be putting this libvirt-specific code into the rt anyway | |
| 13:15:28 | alex_xu | since we assign vpmem without create libvirt domain on the dst host in the beginning of resize | |
| 13:15:38 | efried | a virt driver interface to say "claim the things in this allocation" would seem appropriate. | |
| 13:15:46 | efried | it doesn't need to be vpmem specific | |
| 13:16:02 | alex_xu | efried: yes, so I'm thinking we should use virt.get_available_resources reporting the vpmem to the rt, then rt will track the assignement | |
| 13:16:41 | alex_xu | efried: ok, I see you point | |
| 13:17:10 | alex_xu | another problem is we also need some code to handle the rollback like we spawn the instance failed | |
| 13:17:36 | efried | yup, the methods would need to be symmetrical. claim_for_instance/unclaim_for_instance | |
| 13:18:32 | efried | and the virt driver doesn't really need to do anything with the device other than mark it as "used" in whatever way it sees fit. | |
| 13:18:54 | alex_xu | efried: yea, that we can do | |
| 13:19:13 | alex_xu | efried: should we call that claim_for_instance inside rt.instance_claim? | |
| 13:19:19 | efried | totally | |
| 13:19:23 | efried | and resize_claim | |
| 13:19:30 | alex_xu | cool | |
| 13:19:35 | efried | but | |
| 13:19:47 | efried | I think we should probably create this framework outside of the vpmem series | |
| 13:19:53 | efried | and put the vgpu stuff into it | |
| 13:20:02 | efried | and then build the vpmem series on top of that | |
| 13:20:15 | sean-k-mooney | efried: well im not sure that is true in all cacses | |
| 13:20:15 | alex_xu | yea, that makes sense | |
| 13:20:17 | efried | Because we already know the vgpu stuff is "working" (other than the race condition" | |
| 13:20:20 | efried | ) | |
| 13:20:23 | sean-k-mooney | that the virt driver does not need to do anything | |
| 13:20:32 | efried | sean-k-mooney: That's the point, though | |
| 13:20:39 | efried | the virt driver gets to decide what all it needs to do | |
| 13:20:47 | efried | the RT cannot (and should not need to) know that. | |
| 13:21:08 | sean-k-mooney | but the RT would need to know about numa affintiy and other constrtits | |
| 13:21:14 | efried | E.g. this would be a great time to kick off cyborg fpga programming. | |
| 13:21:22 | kashyap | Anyone from Ubuntu / Debian packaging here? | |
| 13:22:03 | kashyap | We need some packaging work done in Debian & Ubuntu (I did it in Fedora) for the firmware packages. | |
| 13:22:25 | efried | kashyap: coreycb is who mriedem said was his go-to for that | |
| 13:22:46 | alex_xu | sean-k-mooney: in the future the placement will know about the numa affinity | |
| 13:23:02 | kashyap | efried: Noted | |
| 13:23:03 | sean-k-mooney | yes but it doesnte right now | |
| 13:23:08 | kashyap | coreycb: Hi, let me know when you have a block of 15 minutes to chat about some packaging work. | |
| 13:23:10 | efried | right, the fact that the RT has to know about NUMA affinity today is a bastardization | |
| 13:23:26 | sean-k-mooney | efried: not really | |
| 13:23:54 | efried | sean-k-mooney: remember, I come from a world where libvirt isn't the only virt driver in existence :P | |
| 13:24:21 | sean-k-mooney | sure but that does not mean that all legacy systems are bad | |
| 13:24:50 | coreycb | kashyap: can you point me to the source? | |
| 13:24:51 | efried | Didn't say that at all. | |
| 13:24:51 | efried | NUMA affinity is simply Not A Thing in pvm. So the fact that all that logic is in the RT is kind of incestuous from that perspective. | |
| 13:25:16 | sean-k-mooney | hyperv support numa affintiy | |
| 13:25:32 | sean-k-mooney | although i dont know if they use the RT for that | |
| 13:25:58 | kashyap | coreycb: Hi, see my Fedora PullRequest here: https://src.fedoraproject.org/rpms/edk2/pull-request/3 | |
| 13:25:59 | efried | missing the point. "Give my VM 5 VCPU" is universal. "NUMA nodes" is not. | |
| 13:26:17 | kashyap | coreycb: It is essentially to ship JSON "firmware descriptor" files. | |
| 13:26:17 | sean-k-mooney | and PVM systems still have non umiform memory access unless the hardware has only one memory contoler | |
| 13:26:27 | sean-k-mooney | they jsut dont expose it | |
| 13:26:43 | kashyap | coreycb: My commit message contains all the details. I'll actually write an e-mail to 'openstack-discuss' list, perhaps | |
| 13:26:44 | efried | I've mostly retired this soapbox since leaving PowerVM, but it's still an issue. | |
| 13:27:02 | kashyap | coreycb: As SUSE also needs to the packaging work. (They're aware of it; I informed them in Denver.) | |
| 13:27:05 | sean-k-mooney | ya i know it should be optional | |
| 13:27:19 | sean-k-mooney | and not enforced on driver that dont care | |
| 13:27:20 | efried | yes, that's exactly the point. PowerVM systems don't (need to) expose NUMA-isms. | |
| 13:27:24 | coreycb | kashyap: ok so they're part of qemu source? | |
| 13:27:41 | efried | right; and it's not much of a comfort that drivers that don't care can "just ignore" those code paths | |
| 13:27:44 | kashyap | coreycb: Yes. Currently in Git. Will be part of 4.1 -- due in Auguest. | |
| 13:27:45 | efried | that doesn't always work | |
| 13:27:49 | kashyap | (As noted in the commit message.) | |
| 13:28:10 | efried | like the convolutions that were necessary to make sure the sysfs code paths didn't get activated on PowerVM systems when doing PCI passthrough. | |
| 13:28:12 | sean-k-mooney | efried: anyway old argment we are changing this eventually | |
| 13:28:31 | coreycb | kashyap: i'll try to sync you up with cpaelzer. he does our qemu/libvirt packaging. | |
| 13:28:37 | sean-k-mooney | well we were not allowed to make those systems only work for libvirt | |
| 13:28:46 | sean-k-mooney | since virt driver are not allowed created db tables | |
| 13:29:02 | kashyap | coreycb: "Our"? I'm not sure you're of Ubuntu or Debian or... | |
| 13:29:30 | coreycb | kashyap: ubuntu | |
| 13:29:40 | sean-k-mooney | if the libvirt driver was allowed to have its own db table like neturon allows its ml2 driver to do then libvirt would have had its own RT | |
| 13:29:51 | coreycb | kashyap: well we contribute to both though | |
| 13:30:10 | sean-k-mooney | anyway brb | |
| 13:30:42 | kashyap | coreycb: (Nod) | |
| 13:30:44 | alex_xu | efried: sean-k-mooney so the virt_driver.claim_for_instance/unclaim_for_instance still makes sense? | |
| 13:31:14 | efried | alex_xu: sean-k-mooney: This is an interesting point. Whatever tracking the virt driver is doing (in memory) will need to be a) rebuilt when the compute service is restarted; and b) able to survive an instance reboot, because the same information will be necessary to rebuild the XML. | |
| 13:31:28 | efried | so what happens if we hard stop a VM and then restart the compute service and try to restart the VM? | |
| 13:31:59 | efried | without a real persistence mechanism, this may get flaky. | |
| 13:32:31 | dansmith | libvirt itself is a persistence mechanism | |
| 13:33:00 | alex_xu | yea, the compute service will sync some status I think | |
| 13:33:29 | efried | but kind of like what we've done pushing allocations onto the virt driver via what is effectively a callback (update_provider_tree), the results of which the RT uses to push to hard storage (placement db), I wonder if this claiming could do the same thing. | |
| 13:33:41 | efried | sorry, s/allocations/inventories etc./ | |
| 13:34:34 | efried | the artifacts RT pushes to hard storage (in this case the nova db) would have to be opaque to the RT. | |
| 13:34:39 | alex_xu | that claim is about specific device, the placement doesn't care about | |
| 13:34:49 | efried | yes, I'm drawing a parallel | |
| 13:35:49 | efried | I'm saying: {the way placement is used as the persistence mechanism for provider inventories etc} is equivalent in principle to {the way the nova db would need to be used for claim/assignment information we're discussing} | |
| 13:36:59 | alex_xu | actually we needn't persistent the claim and assigment in nova db, since libvirt persistent the info as dansmith said | |
| 13:37:49 | efried | You mean via the domain XML? | |
| 13:38:11 | alex_xu | yes, we read all assigned devices from the domain xml in the startup of compute service | |
| 13:38:12 | efried | But we rebuild the domain XML when we hard reboot the instance, right? | |