| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-12-02 | |||
| 18:44:03 | dansmith | Sundar: sure, it just doesn't cover actually producing a device and getting libvirt to attach it | |
| 18:44:09 | Sundar | dansmith: Yes, we are planning a 3rd party CI for Intel FPGAs | |
| 18:44:22 | sean-k-mooney | we could maybe use some other virtual device for testing in a more robust way too | |
| 18:45:14 | dansmith | sean-k-mooney: yeah I was going to ask if we could pass some harmless pci device through, but wasn't sure if we can do that in the gate without nested | |
| 18:45:19 | Sundar | sean-k-mooney: We have plans to enhance the fake driver to simulate programming -- it will just respons to the programming API within Cyborg with a success/failure | |
| 18:45:35 | sean-k-mooney | dansmith: i was thinking of creating a file on disk and passing it to the guest | |
| 18:45:45 | dansmith | Sundar: yeah, that's good, I just want to see the actual "here's the pci device" stuff tested | |
| 18:45:48 | Sundar | That would cover more paths, including Glance interaction within Cyborg. But it would still not attach a PCI device to the VM | |
| 18:46:02 | dansmith | sean-k-mooney: how does a file on disk simulate a pci device/ | |
| 18:46:42 | dansmith | okay anyway, this log looks good for what we have now and can test now, so that's cool | |
| 18:46:57 | dansmith | I'll want to look at this one Sundar retools the current patches per the pending feedback | |
| 18:47:05 | sean-k-mooney | it does not but it will simulate genarting xml. i was really thinking of a lvm driver for local caching so we could have cyborg provide local storage to a vm. | |
| 18:47:20 | dansmith | Sundar: FWIW, I have this week and next week before I turn to a pumpkin for the rest of the year | |
| 18:47:38 | sean-k-mooney | e.g. have a driver that could be useful in producion but has no hardware depency | |
| 18:47:43 | Sundar | dansmith: I am going to squash https://review.opendev.org/631245 into https://review.opendev.org/631244, for the reasons I mentioned above | |
| 18:47:46 | dansmith | sean-k-mooney: is that a thing? I thought we were focused on pci devices for accelerators right now | |
| 18:48:27 | sean-k-mooney | we are but form a livirt driver point of there is very littel difference between generating the xml for a pci device vs a block device | |
| 18:48:32 | dansmith | Sundar: okay I skimmed over that.. IMHO, keeping the new event definition in its own patch makes sense just because of how much api stuff has to change | |
| 18:49:08 | dansmith | sean-k-mooney: seems fairly different to me.. I understand it's just a chunk of xml, but it's not the same kind of xml we really care about | |
| 18:49:11 | Sundar | dansmith: Agreed. The event definition patch https://review.opendev.org/692707 will stay as is. | |
| 18:49:36 | sean-k-mooney | if we just want a way to create a device (lvm volume) program it (dd image to volume) and validate it in the guset it was a way to do it. | |
| 18:49:41 | dansmith | sean-k-mooney: it's something, it's just not enough | |
| 18:49:52 | dansmith | Sundar: oh sorry I was looking at the wrong two tabs, lemme re-read | |
| 18:50:24 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Request and store the decrypted path of an attached encrypted volume https://review.opendev.org/696693 | |
| 18:50:40 | sean-k-mooney | ya i was also looking at ways to fake pci devices in linux and there are some ways but its not trivial | |
| 18:50:47 | Sundar | sean-k-mooney: The code paths within the virt driver are quite different for block devices and PCI devices. So, how does this help? | |
| 18:51:03 | dansmith | Sundar: 631234 all but goes away after you address my feedback | |
| 18:51:22 | dansmith | Sundar: so assuming you mean just bringing forward the cyborg client method, then sure | |
| 18:51:46 | dansmith | Sundar: it would be attaching a block device as an accelerator, so no typical nova block device handling for it | |
| 18:51:50 | sean-k-mooney | its a more complete end to end test that is all. also im not sure how https://review.opendev.org/#/c/689070/ is going to progress and its another way to enable local caching | |
| 18:52:16 | dansmith | sean-k-mooney: I don't think cyborg for block devices is a good way to address the local caching thing | |
| 18:52:24 | dansmith | sean-k-mooney: how is that not going to confuse people? | |
| 18:52:54 | sean-k-mooney | well given samsung are now supporting sriov on there new ssds it might be but sure ill let you get back to it | |
| 18:53:18 | dansmith | well, I guess we can have that discussion, but I definitely don't want to merge that with this effort at the moment | |
| 18:53:34 | dansmith | and since PCI is different enough, I still want to see PCI working for this | |
| 18:53:41 | sean-k-mooney | oh sure. its totally seperate | |
| 18:53:48 | sean-k-mooney | ya | |
| 18:53:57 | sean-k-mooney | i just have not figured out how to fake it yet | |
| 18:54:23 | sean-k-mooney | the kernel netdevsim module was the close i came up with in the past | |
| 18:54:25 | dansmith | yeah, well, as long as we'll have intel ci so we can see it working, that's something | |
| 18:54:30 | Sundar | dansmith: 631234 is a typo, right? Quoting myself from above: I think we need to squash https://review.opendev.org/631245 into the previous patch https://review.opendev.org/631244. Because once the get_resolved_arqs() moves to previous patch, the only things left behind are the virt driver changes. It would arguably be easier to understand those | |
| 18:54:31 | Sundar | changes together with the changes to get the arqs in compute/manager.py. | |
| 18:54:55 | dansmith | Sundar: I meant 631245 yes, sorry | |
| 18:55:01 | sean-k-mooney | ya. the fake driver should allow us to test the api flow. the intel ci should validate it works with real hardware | |
| 18:55:09 | Sundar | Cool, np | |
| 18:55:24 | dansmith | Sundar: the virt changes will be insulated from the cyborg client method is my point | |
| 18:56:08 | dansmith | Sundar: if I were you I would have one patch that adds the support into the manager, all the build_resources changes to capture the ARQs and pass them to the virt driver, | |
| 18:56:17 | dansmith | Sundar: then a separate patch to implement things for libvirt | |
| 18:56:17 | Sundar | dansmith: Sure, will try to get back ASAP with the changes | |
| 18:56:33 | Sundar | Oh, separate patch for libvirt? | |
| 18:56:40 | dansmith | yes definitely | |
| 18:56:47 | Sundar | That would be 63124 today | |
| 18:56:58 | Sundar | *631245 | |
| 18:57:21 | dansmith | Sundar: so when hyperv people come along, we point them at the libvirt patch and say "implement this part for your driver and you will be good"... separate from the plumbing in compute to wire everything up | |
| 18:58:04 | Sundar | I see, fine with that too | |
| 18:58:06 | dansmith | Sundar: in the compute manager patch, you just assume that any virt driver below you will ignore the ARQs you pass to it, which will be true of libvirt, until the next patch in the series where that becomes untrue for libvirt, but remains true for all the others | |
| 18:59:11 | openstackgerrit | Merged openstack/nova stable/pike: Add regression recreate test for bug 1830747 https://review.opendev.org/663124 | |
| 18:59:11 | openstack | bug 1830747 in OpenStack Compute (nova) pike "Error 500 trying to migrate an instance after wrong request_spec" [High,In progress] https://launchpad.net/bugs/1830747 - Assigned to Matt Riedemann (mriedem) | |
| 18:59:15 | openstackgerrit | Merged openstack/nova stable/pike: Workaround missing RequestSpec.instance_group.uuid https://review.opendev.org/663125 | |
| 18:59:27 | Sundar | dansmith: In the compute manager patch, the signature for libvirt driver's spawn() will have tio change to include the arqs as a parameter. | |
| 18:59:35 | dansmith | Sundar: does this method go away if you pass the stuff from compute manager to virt? https://review.opendev.org/#/c/631245/39/nova/accelerator/cyborg.py | |
| 18:59:49 | dansmith | Sundar: that would leave the virt patch you have as just the libvirt config bits | |
| 19:00:47 | dansmith | Sundar: correct, spawn() will need to take the ARQs, just like it takes network and block device info | |
| 19:01:51 | dansmith | Sundar: https://github.com/openstack/nova/blob/56d3cd7aa7f4d3be01fd2a5c10903fb548c49458/nova/compute/manager.py#L2491-L2499 | |
| 19:02:08 | dansmith | Sundar: just like block and network, you should pull out arqs from resources and pass to spawn | |
| 19:02:26 | dansmith | arguably we could just be passing in resources to spawn to avoid splitting it out, but let's not go down that hole in this set | |
| 19:03:06 | dansmith | Sundar: and since I will have a strong opinion on that later, let me go ahead and say, it should be called: | |
| 19:03:17 | dansmith | accel_info = resources['accel_info'] | |
| 19:03:20 | dansmith | :) | |
| 19:03:28 | Sundar | dansmith: That method would remain, albeit it would move to the compute manager patch. So that the waiting logic can call it once to decide whether to quit early. The virt driver patch 631245 will slim down to just the libvirt driver changes. The change in the signature for virt driver base class and the libvirt driver (to include arqs as a | |
| 19:03:29 | Sundar | parameter) will now be in the compute manager patch. | |
| 19:03:52 | dansmith | Sundar: ah right, forgot that would be the check, so yes, move it thusly | |
| 19:04:01 | Sundar | dansmith: What you said about resources['accel...'] is exactly what I said in the gerrit review :) | |
| 19:04:06 | dansmith | ack | |
| 19:04:35 | dansmith | Sundar: except not the same names | |
| 19:04:46 | dansmith | Sundar: I was giving you a freebie pre -1 on the *name* :) | |
| 19:05:02 | Sundar | What name? | |
| 19:05:13 | dansmith | accel_info instead of accelerator_requests | |
| 19:05:35 | Sundar | I see. To match network_info etc. | |
| 19:05:38 | dansmith | yes | |
| 19:05:49 | dansmith | because I have issues. my therapist and I are working on them. | |
| 19:05:49 | Sundar | Ok, I'm not too hung up on names | |
| 19:05:54 | dansmith | good :) | |
| 19:08:23 | Sundar | sean-k-mooney: still intrigued by your idea for a fake device, block or PCI. Do you have a way to simulate a PCI device? It can be *quite* difficult AFAICS with linux PCI subsystem. | |
| 19:08:56 | dansmith | so I was thinking about something else, like grab the last usb host and pass that through | |
| 19:09:28 | dansmith | Sundar: I would focus on getting the patches back into order, and let sean-k-mooney pull something out of his butt for faking the PCI stuff... :) | |
| 19:10:25 | Sundar | Yup, that's what I plan to do :) but a 'real fake driver' (heck, I'll grab the award for today's oxymoron) would be very useful for the future. | |
| 19:10:26 | dansmith | he pulled off some fancy stuff for the numa live migration stuff, so we should see if he can do it again here :) | |
| 19:10:31 | sean-k-mooney | Sundar: the closest i have come up with is the netdevsim kernel module wich does create sriov capable fake pci device for testing and developing the kernel ebpf code | |
| 19:10:50 | sean-k-mooney | Sundar: how ever i dont think you can actully pass those device to a qemu instace. i have not tried however | |
| 19:11:08 | sean-k-mooney | i did try to use it for sriov testing in the past but i could not get it to work | |
| 19:11:56 | sean-k-mooney | i was able to create the fake device but i could not assing the vfs the pf might have worked but we dont really want to have to compile kernel module in the gate if we can avoid it | |
| 19:12:20 | Sundar | sean-k-mooney: It would be useful to have a libvirt patch where the domain XML for hostdev can have an attribute that indicates 'do not pass this to qemu'. | |
| 19:12:35 | Sundar | That would be useful outside of OpenStack too, I would guess. | |
| 19:12:37 | dansmith | sean-k-mooney: that's why i was thinking pick something on the host we could pass through, like one of those USB hosts | |
| 19:12:56 | dansmith | Sundar: can't imagine the libvirt folks would go for that | |
| 19:12:56 | sean-k-mooney | dansmith: maybe ya | |
| 19:14:11 | sean-k-mooney | there was talk and patchs for adding the abilty to emulate sriov device in qemu | |
| 19:14:27 | sean-k-mooney | if that ever gets merged upstream we might be able to use that | |
| 19:14:42 | sean-k-mooney | but for now it think focusing on getting the patches in order is better | |