| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-11 | |||
| 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? | |
| 13:38:49 | efried | So if the instance is hard stopped, and then we restart the compute service, we have to rebuild the domain XML from somewhere. | |
| 13:39:13 | efried | or am I misunderstanding that lifecycle flow completely? | |
| 13:39:15 | alex_xu | efried: if you just stop the instance, the xml still in libvirt. do you mean remove the domain from the libvirt? | |
| 13:39:31 | dansmith | but the xml doesn't go away until you rebuild it, so you can read it, then do your regeneration | |
| 13:39:50 | efried | okay, cool. Then I guess we're okay. | |
| 13:39:52 | efried | but | |
| 13:40:00 | dansmith | we depend on libvirt keeping the domain xml for us in lots of cases | |
| 13:40:09 | efried | in this flow, the claim needs to happen before spawn is invoked | |
| 13:40:16 | efried | i.e. before we even start to build the domain xml | |
| 13:40:46 | efried | so the claim info will need to be stored temporarily somewhere else | |
| 13:40:53 | alex_xu | yes | |
| 13:40:56 | alex_xu | in memory | |
| 13:40:58 | efried | and then we can flush it once the instance is alive | |
| 13:41:01 | efried | okay, that wfm. | |
| 13:41:07 | stephenfin | bauzas: (Because I don't have a deployment available to test this on) Can standard, non-admin users specify the availability zone an instance lands in? | |
| 13:41:28 | bauzas | stephenfin: that's the whole purpose of AZs :) | |
| 13:41:40 | efried | alex_xu: Are you planning to work on this? | |
| 13:41:54 | alex_xu | yes, I can take a look that | |
| 13:42:01 | stephenfin | bauzas: okay, phew. So they can specify an AZ but not a destination host/node within that AZ, yeah? | |
| 13:42:05 | stephenfin | By default, that is | |
| 13:42:05 | bauzas | stephenfin: to tell a way how to land somewhere you don't know but different from some other way you don't want | |
| 13:42:25 | bauzas | stephenfin: yeah it's borked, because the AZ hack is a terrible way | |
| 13:42:50 | bauzas | so, yeah, we're accepting users to set an AZ name but they're trampled if they use ':' | |
| 13:43:00 | stephenfin | bauzas: yeah, that's why https://review.opendev.org/#/c/666767/ and its predecessor exists, presumably | |
| 13:43:36 | bauzas | stephenfin: I need to take a look on those things | |
| 13:43:45 | bauzas | efried: alex_xu: my meeting is done, wazzup ? | |
| 13:43:50 | stephenfin | bauzas: I'm rewriting the AZ/host aggregate docs atm. They're not done but when they are, you'll be the first person I ping for reviews ;) https://review.opendev.org/#/c/667133/ | |
| 13:44:05 | bauzas | (well I have other meetings in 20 mins, so my time is counted :( ) | |
| 13:44:12 | efried | bauzas: We were just discussing the race condition whereby two instances could try to assign the same vgpu | |
| 13:44:31 | efried | talking about moving the vgpu "assignment" (though actually not the assignment, see below) into instance_claim | |
| 13:44:31 | bauzas | efried: k, basically alex_xu made a good point | |
| 13:44:46 | bauzas | efried: that's overthinking I feel | |
| 13:44:59 | bauzas | stephenfin: ack, bump me with review requests tomorrow :) | |
| 13:45:27 | efried | tldr we're talking about adding claim_for_instance/unclaim_for_instance to the ComputeDriver interface, since only the specific virt driver can know how to identify the specific device, and what needs to be done to "reserve" it. | |
| 13:45:48 | efried | In the case of the libvirt driver for vgpus, it would be sufficient to mark it in an in-memory dict as belonging to the instance | |
| 13:46:05 | efried | then purge that dict entry during spawn, since that information can be gleaned from the domain XML thereafter. | |
| 13:46:45 | efried | this takes hypervisor-specific code out of the RT, which is ++ | |
| 13:46:49 | efried | and fixes the race | |
| 13:47:01 | efried | and is reusable for other things like vpmem | |
| 13:47:21 | bauzas | efried: FWIW, this is somehow related to the spec you love :) | |
| 13:47:36 | efried | which, vgpu affinity? | |
| 13:48:21 | bauzas | efried: correct, as we check things in instance_claim() already | |
| 13:48:35 | bauzas | efried: and that's what I really want to amend for vGPUs | |
| 13:48:48 | efried | yeah, basically "figure out which specific thingy on the system is going to be assigned to the instance for a particular unit of resource class in an allocation" should be the purview of the virt driver. | |
| 13:49:00 | bauzas | as I said, the vGPU weigher and the object update aren't necessary | |
| 13:49:01 | efried | because only the virt driver has a view to "specific thingy on the system" | |
| 13:49:13 | bauzas | sec, giving you a link | |
| 13:50:54 | sean-k-mooney | bauzas: its even more related to the draft i wrote for that | |
| 13:52:39 | sean-k-mooney | efried: i think bauzas means https://review.opendev.org/#/c/650963/9/specs/train/approved/libvirt-vgpu-numa-affinity.rst | |
| 13:53:20 | bauzas | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2223 eventualls calls https://github.com/openstack/nova/blob/master/nova/compute/claims.py#L220 | |
| 13:53:27 | bauzas | efried: ^ | |
| 13:55:37 | bauzas | efried: so what you'd like is that the device assignment (ie. the mdev choice of https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L6416) would be called by something close to https://github.com/openstack/nova/blob/master/nova/compute/claims.py#L220 ? | |
| 13:55:47 | bauzas | if so, I'm not opposed to | |
| 13:56:18 | bauzas | but xen needs substantial changes too | |
| 13:57:12 | coreycb | kashyap: cpaelzer added it to his 20.04 virt stack work. thanks for the heads up. | |
| 13:57:24 | efried | bauzas: I would like it if the hypervisor-specific aspects of all of that were in the virt driver -- but yes, to fix the race, the "mdev choice" needs to happen somewhere inside instance_claim. | |