| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-cyborg - 2019-08-22 | |||
| 03:12:19 | Sundar | #topic Merging patches | |
| 03:12:48 | Sundar | As s_shogo pointed out, a lot of things are blocked on merging the P5-P9 patches. Can you all please prioritize that? | |
| 03:13:05 | Coco_gao_ | Agree | |
| 03:13:20 | wangzhh | Sure. | |
| 03:14:09 | Coco_gao_ | About P5 patch, a lot of patch is denpending on it. | |
| 03:14:23 | Sundar | Great. Xinran's patches on Nova notification and Placement are important too. Once they also merge, we can enable tempest CI, and that is what we need to kick off Nova integ. | |
| 03:14:58 | Yumeng | hi Sundar: I noticed you gave a -2 on P5: Basic changes for API layer. https://review.opendev.org/#/c/670466/ are there any problems? | |
| 03:15:00 | Coco_gao_ | yumeng also need to modify the database | |
| 03:15:00 | Sundar | Coco_gao: Agreed. I have tested the P5-P9 with fake and FPGA drivers. | |
| 03:15:44 | xinranwang | Sundar: do you want to discuss vendor_id, vendor_name stuff, which we discussed yesterday? | |
| 03:15:59 | Sundar | Yumeng: It is a procedural -2 to prevent the patch series from merging piece by piece. It will be cleaner if we all review all the patches as a whole and then the whole thing merges around the same time. | |
| 03:16:22 | Sundar | Same reasoning as with the -2 for https://review.opendev.org/631242 in Nova | |
| 03:17:05 | xinranwang | Please review placement report patch, it has less dependency on P5-P6 patch. :) | |
| 03:17:11 | Yumeng | Sundar: got it . thanks | |
| 03:17:14 | Coco_gao_ | So can we make the priority list about the patches? | |
| 03:18:15 | Sundar | Coco_gao: I would suggest: P5-P9, Nova notification, Placement report. What do you all think? | |
| 03:18:32 | Sundar | Not saying other patches are less important :) | |
| 03:18:51 | Sundar | Just tryin to unblock as much as we can and get the ball back to Nova developers' court. | |
| 03:19:17 | Coco_gao_ | I think some of the patches had dependency issues so I am asking about the priority. | |
| 03:19:22 | xinranwang | I think we can review placement from now, cause it has less dependency on your patches. | |
| 03:19:47 | xinranwang | notification after p5-p9 | |
| 03:20:05 | Coco_gao_ | Both dependency and our Train release goal are matter. | |
| 03:20:12 | Sundar | xinranwang: Not disagreeing, but Placement needs to be tested with P5-P9 patches, right? | |
| 03:20:24 | xinranwang | No | |
| 03:20:46 | Coco_gao_ | P5-P9, Nova notification, Placement report. What about python3? | |
| 03:20:49 | xinranwang | we can run placement patch without nova-cyborg integration | |
| 03:21:29 | Sundar | Ok, we could do partial testing with CYborg driver -> agent -> conductor (db) -> Placement. Without using Nova. Is that what you are saying? | |
| 03:21:44 | xinranwang | Yes, exactl. | |
| 03:21:51 | xinranwang | *exactly | |
| 03:21:58 | Coco_gao_ | OK, got that. | |
| 03:22:00 | Sundar | Agreed. | |
| 03:22:35 | xinranwang | Also, placement provides cmd line like openstack resource provider list, we can check the result directly by using this. | |
| 03:23:11 | Sundar | Ok, then order with dependencies is: [ P5-P9, Placement report], then nova notification. Sounds good? | |
| 03:23:31 | Coco_gao_ | Great. | |
| 03:23:34 | chenke | yes. | |
| 03:24:16 | xinranwang | yes, great | |
| 03:24:17 | Coco_gao_ | I see there are several patches which had conflict problems. | |
| 03:24:36 | Coco_gao_ | Will that be solved if we merge P5-P9 | |
| 03:25:17 | Sundar | Coco_gao_: In https://review.opendev.org/#/q/status:open+project:openstack/cyborg+branch:master, I see 4 merge conflicts and they are all old patches only for reference. Rest should be good. | |
| 03:25:18 | Coco_gao_ | Seems good to me because I know where to start my reviews. | |
| 03:26:13 | xinranwang | Sundar wangzhh: I noticed that GPU driver report vendor id and fill the vendor field with ID, is there any possibility that we fill vendor field by vendor name? | |
| 03:27:11 | Sundar | IMHO, we need the vendor name for the device somehwere, so that we can create the traits with the vendor name, as listed in https://review.opendev.org/gitweb?p=openstack/cyborg-specs.git;f=specs/train/approved/cyborg-nova-placement.rst;hb=refs/changes/45/603545/7#l179 | |
| 03:27:14 | Coco_gao_ | We can get the vendor name from vendor ID right? | |
| 03:27:44 | Coco_gao_ | Since the vendor ID is unique | |
| 03:28:01 | Sundar | Yes, but the driver report does not include the vendor name as a field today | |
| 03:28:14 | wangzhh | Of course, but I suggest record vendorId in db. And translate it to name if needed. | |
| 03:28:39 | Sundar | Perhaps it will be easiest to add vendor name as a separate field in the driver_objects.device, to minimize changes to GPU driver and other drivers | |
| 03:29:14 | Coco_gao_ | OK. | |
| 03:29:26 | Coco_gao_ | Agree that we should keep vendor ID | |
| 03:29:38 | xinranwang | each driver should know about the mapping relation between id and name, but cyborg-agent/conductor don't | |
| 03:30:11 | Sundar | The Cyborg driver for a vendor could even hardcode the vendor name, if it will handle only one vendor. | |
| 03:30:35 | xinranwang | So IMO, driver should report vendor name anyway. | |
| 03:30:54 | Sundar | xinranwang: Agreed. Proposal: Add vendor name to driver_objects.device . Any objections? | |
| 03:30:56 | wangzhh | Sundar, It is hardcoded in driver now. | |
| 03:31:08 | wangzhh | Agreed | |
| 03:31:26 | xinranwang | no objection. | |
| 03:31:45 | xinranwang | Btw, i find driver name in deployable tables, but no assignment. | |
| 03:32:04 | shaohe_feng | Yes， I translate the vendor ID to name for FPGA in my DEMO | |
| 03:32:05 | Coco_gao_ | Agree. The object.device will not add vendor name? Only driver_objects.device will? | |
| 03:32:54 | shaohe_feng | it can add a vendor name | |
| 03:33:24 | shaohe_feng | but maybe more driver infos, such as driver version. | |
| 03:33:24 | Sundar | Coco_gao_: Good point. It will be easier to add it to device object too, for use in ext-arq binding for creating traits | |
| 03:34:06 | Sundar | Do we need to add that to the db then? Oherwise, how how do we get it in the device object by querying? | |
| 03:34:07 | wangzhh | So just add name in object, not in db, right? | |
| 03:34:09 | Coco_gao_ | My question is do you think we should keep vendor name in object or database? | |
| 03:34:35 | shaohe_feng | for a long term. maybe we can translate the ID to name at present. | |
| 03:34:37 | shaohe_feng | DB | |
| 03:34:58 | Coco_gao_ | vendor ID is already in DB? | |
| 03:35:03 | xinranwang | Sundar: what is driver name you added in deployable table stand for? | |
| 03:35:22 | wangzhh | coco yep | |
| 03:35:27 | Sundar | If you don;t add the vendor name to db, the translation from id to name may have to be outside the driver too, which is not good | |
| 03:35:32 | xinranwang | Coco_gao_: yes, in device tables | |
| 03:36:09 | Yumeng | Coco_gao_:yes. already have vendor_id in db device table | |
| 03:36:42 | Coco_gao_ | About the ext-arq binding, do we need to qurey DB for vendor name? | |
| 03:36:44 | Sundar | xinranwang: It is the driver that is used for programming and device updates. At discovery time, we know which deployable has which driver. So, we can use that info to locate the driver needed for programming. | |
| 03:37:00 | shaohe_feng | https://github.com/shaohef/cyborg/commit/7e353bf09259a9b4cfba3763f3a99eb1adfaaa2b#diff-7ca60ed56d570ebe3490 | |
| 03:37:11 | xinranwang | Sundar: | |
| 03:37:20 | xinranwang | there is no assignment of this field | |
| 03:37:24 | Sundar | Coco_gao_: For ext_arq binding, we are queryig db for the deployable now. We can extend tthat to query the device and its vendor name too | |
| 03:37:38 | xinranwang | maybe we can extract vendor name from this field? | |
| 03:37:40 | shaohe_feng | here is what I do, just for a temporary solution | |
| 03:38:19 | shaohe_feng | driver_name = db_deployable.driver_name | |
| 03:39:44 | Sundar | xinranwang: The intention is that, when the agent discovers a device from a driver, it sends the driver name to the conductor and has it persisted. | |
| 03:40:39 | Sundar | shaohe_feng: That is what we are doing today. We could use the driver name and vendor name as same-- at least for Train. Is that what I am hearing from many of you? | |
| 03:41:24 | xinranwang | Sundar: so what will it look like? intel_fpga_driver? or 0x8086 like shaohe's demo does. | |
| 03:41:40 | shaohe_feng | https://github.com/shaohef/cyborg/blob/profile-ctrl/cyborg/accelerator/drivers/fpga/base.py#L23 | |
| 03:42:16 | shaohe_feng | for fpga driver, I maintain a map, both vendor ID and name can work. | |
| 03:42:40 | shaohe_feng | Sundar: yes, we need a driver name | |
| 03:43:17 | Sundar | xinranwang: just 'intel', for example. That is what we use to load a driver module. The intel_fpga_driver is the name in setup.cfg. | |
| 03:44:08 | xinranwang | so we should fill driver_name with 'intel' for example? | |
| 03:44:28 | xinranwang | if so, we can extract vendor name for driver_name.. | |
| 03:44:54 | Sundar | Yes, xinranwang. To summarize, I think we are agreeing to update the driver_name field in the deployable and use that as 'vendor name' to create the traits as per https://review.opendev.org/gitweb?p=openstack/cyborg-specs.git;f=specs/train/approved/cyborg-nova-placement.rst;hb=refs/changes/45/603545/7#l179 | |
| 03:45:02 | Sundar | Sounds good? | |
| 03:45:21 | Sundar | Then no need to add vendor_name to any object | |
| 03:45:44 | xinranwang | yes | |
| 03:46:56 | Sundar | Great. If others have objecttions, please LMK. Let's move on in the interest of time | |
| 03:47:04 | Sundar | #topic python cyborg client | |
| 03:47:44 | s_shogo | I'm working that with P5-P9 patch in advance. | |
| 03:48:12 | Sundar | Thanks, s_shogo. I am wondering if we have the time to update the cyborg client to v2 API? | |
| 03:48:28 | Coco_gao_ | So zhenghao's patch should use Vendor ID and then update deployable vendor name? | |