| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-04-20 | |||
| 10:07:58 | bauzas | oh shit, wrong mix | |
| 10:08:05 | brinzhang | gibi: thanks | |
| 10:08:07 | bauzas | thanks gibi | |
| 10:08:11 | bauzas | migi, gibi, dammit | |
| 10:08:39 | bauzas | ah, sad, he's not connected here | |
| 10:08:49 | brinzhang | bauzas: done | |
| 10:09:42 | bauzas | brinzhang: ack, good, can respin since Zuul hasn't replied yet | |
| 10:11:14 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Allocate mdevs when resizing or reverting resize https://review.opendev.org/712741 | |
| 10:16:00 | brinzhang | bauzas: thanks quickly update ^ | |
| 10:16:48 | brinzhang | bauzas: stephenfin working on change "import mock" to "from unittest import mock", https://review.opendev.org/#/c/714676/3 | |
| 10:17:47 | brinzhang | bauzas: does this [1] need to change? or wait for this merged, than stephenfin update that patch? [1]https://review.opendev.org/#/c/712741/6/nova/tests/functional/libvirt/test_vgpu.py@17 | |
| 10:17:52 | bauzas | I honestly think this is a rathole :) | |
| 10:19:13 | brinzhang | bauzas: yeah, I think so | |
| 10:20:57 | brinzhang | bauzas: https://review.opendev.org/#/c/712741/6/nova/virt/libvirt/driver.py@10111 need someone to check? I saw you add ? in | |
| 10:22:03 | brinzhang | others looks good to me | |
| 10:32:52 | bauzas | brinzhang: not sure I understand your question ? | |
| 10:33:10 | bauzas | brinzhang: do you mean that the comment is confusing ? | |
| 10:33:20 | bauzas | b/c it's a question ? | |
| 10:33:27 | brinzhang | yes, | |
| 10:33:50 | brinzhang | that should a note, right? | |
| 10:34:03 | bauzas | ahah, no, it's just something like 'verify if we need to assign some mdevs" | |
| 10:34:33 | bauzas | if it was a question, it would be a FIXME or a TODO | |
| 10:35:06 | brinzhang | bauzas: ah, yes, I think you missed TODO or FIXME tag | |
| 10:36:10 | bauzas | brinzhang: again, no | |
| 10:36:14 | bauzas | it wasn't a question for others | |
| 10:37:08 | brinzhang | You only put one question here, and there is no extra detail, I think it needs to be added, isn't it? | |
| 10:38:01 | bauzas | brinzhang: it's not really a question for others, it's just explain what the method does | |
| 10:38:07 | bauzas | it just explains* sorry | |
| 10:39:35 | brinzhang | it's ok, it really confuses me, maybe I should take it seriously. | |
| 10:39:41 | brinzhang | thanks bauzas ^ | |
| 10:41:01 | bauzas | I can provide a FUP if you want | |
| 10:56:44 | gibi | brinzhang: responded in https://review.opendev.org/#/c/720670 | |
| 10:57:43 | gibi | bauzas: I have started looking at https://review.opendev.org/#/c/712741/ just now | |
| 11:00:57 | brinzhang | gibi: I think your think make sense, agree, thanks | |
| 11:03:14 | brinzhang | this is an invalid bug | |
| 11:22:13 | songwenping | gb:Hi gibi. I am working on nova-cyborg-interaction now, and commit this patch https://review.opendev.org/#/c/720670/ | |
| 11:22:38 | gibi | songwenping: hi | |
| 11:22:50 | songwenping | We haven't show the ARQ id in dashboard now. | |
| 11:23:09 | songwenping | But i think we will show it like cinder volume. | |
| 11:24:05 | songwenping | So should we handle the cyborg exception after showing it? | |
| 11:24:06 | gibi | songwenping: still, expecting the end user to _know_ where and how to clean up after a seemingly _successfull_ server delete operations feels bad | |
| 11:24:58 | gibi | basically after every server delete the end user would need to check the cyborg API to know if the ARQs are freed up or not | |
| 11:25:09 | gibi | I don't like that | |
| 11:26:20 | brinzhang | gibi, songwenping: agree with gibi, if there are so many resources leaked in Cyborg, it will be a heavy works to cleanup | |
| 11:26:49 | brinzhang | but compared with Cinder logical, it also has the same issue. | |
| 11:27:32 | brinzhang | maybe we shuold have a logical to deal with this, or dealed in Cinder and/or Cyborg | |
| 11:33:23 | songwenping | gb:Yeah, it's indeed a problem leaking many resources in system. I just want to keep pace with cinder logical. | |
| 11:36:57 | gibi | songwenping: what is the use case you want to solve? you mentioned deploy and undeploy cyborg. there I think before undeploy the admin needs to clean up the cyborg users. Also mentioned failure in cyborg. If that failure is intermittent (e.g service restart or network interrupt) then I think end user needs to retry the delete. if the cyborg failure is static then that is a cyborg but to be fixed | |
| 11:37:20 | gibi | s/but/bug/ | |
| 11:43:24 | songwenping_ | gibi:i want to solve the second use case. | |
| 11:45:33 | brinzhang | gibi: I give you use case from my customer | |
| 11:45:45 | brinzhang | s/use case/ a use case | |
| 11:45:54 | brinzhang | gibi: Due to the system upgrade, the cyborg service cannot be started. If the user wants to clear the instance that contains ‘accel: device_profile_name’ in the flavor, the instance will be in an error state and cannot release scarce resources such as GPU and FPGA. If that is the user's only resource, it may also be considered for manual cleaning. This is common for small customers. | |
| 11:47:25 | brinzhang | Of course, this is a scene of its edge. | |
| 11:47:32 | gibi | brinzhang: so during and upgrade some of the control plane services are still up (e.g. nova) but some of them are down (e.g cyborg) | |
| 11:48:26 | brinzhang | This may be a treatment, but it is not so perfect. | |
| 11:48:33 | gibi | but if cyborg is down then who the user could ever free up FPGA resources? | |
| 11:48:43 | gibi | s/who/how/ | |
| 11:49:31 | gibi | also even if it is freed up it cannot be used again as cyborg is down | |
| 11:49:51 | brinzhang | maybe that canbe done in db, this is perhaps the worst case | |
| 11:51:07 | gibi | sorry but I my mind if cyborg service is down, then the user cannot and should not do anything with resources managed by cyborg | |
| 11:51:13 | brinzhang | gibi: I just put forward such a scenario, and I agree with you, your consider is right | |
| 11:52:05 | gibi | then we agree that this use case is not valid :) | |
| 11:52:12 | songwenping_ | agree with gibi. | |
| 11:53:27 | brinzhang | yes, but I think I also need to think how to deal with this scenario, we did encounter this situation. | |
| 11:55:22 | brinzhang | another way, that can power off the server, and migrate its instance, than re-deployed the OpenStack | |
| 11:55:33 | brinzhang | in a new region | |
| 11:57:55 | gibi | honestly I don't see why does your deployment need to support manipulating FPGAs while cyborg service is doewn | |
| 11:57:58 | gibi | down | |
| 12:01:27 | brinzhang | yes, it isnot make sense. good bye gibi, hope you have a good day ^ | |
| 12:05:15 | gibi | brinzhang: have a nice afternoon | |
| 12:12:03 | bauzas | gibi: ack thanks | |
| 12:12:21 | bauzas | brinzhang: if you want, ping me tomorrow for discussing about something about reviews | |
| 12:12:36 | bauzas | brinzhang: (UTC+2 here) | |
| 12:40:16 | gibi | bauzas: will there be a FUP for the comments from lyarwood in https://review.opendev.org/#/c/712118/ ? | |
| 12:40:47 | bauzas | gibi: haven't seen them yet | |
| 12:41:15 | bauzas | gibi: FWIW, in https://review.opendev.org/#/c/712118/ like I said in a comment, this change is no longer needed for https://review.opendev.org/#/c/712741/ | |
| 12:42:14 | gibi | bauzas: will you then remove it from the series? | |
| 12:42:21 | bauzas | gibi: for the comment nits, sure I can do it in a FUP | |
| 12:42:41 | bauzas | gibi: we can merge it since I already provided a ML thread for out-of-tree drivers maintainers | |
| 12:42:41 | gibi | bauzas: yes, comment nits are totally FUPable but if the whole patch is not needed then it is even better | |
| 12:43:11 | bauzas | gibi: or wait, I'll rebase this one on top of https://review.opendev.org/#/c/712741/ and just provide a new revision for the nits | |
| 12:43:22 | bauzas | will be done in 1 min | |
| 12:43:29 | gibi | but then the allocation would be an unused param | |
| 12:43:31 | gibi | isn't it? | |
| 12:46:19 | bauzas | gibi: for finish_revert_migration() yes | |
| 12:46:31 | bauzas | gibi: to clarify, I'll just provide the new series | |
| 12:46:58 | bauzas | and people can discuss on the opportunity to merge https://review.opendev.org/#/c/712118/ if nothing uses the new param or not | |
| 12:47:14 | gibi | bauzas: OK, I will check the new series | |
| 12:47:57 | bauzas | gibi: should be done in 5 mins, just verifying unittests and functests because of a minor merge conflict | |
| 12:49:18 | gibi | cool | |
| 13:00:02 | francoisp | gibi hello, we would need an external reviewer (outside of RH) to check on https://review.opendev.org/#/c/669674/ , would you have time to have a look? | |
| 13:00:12 | bauzas | gibi: excellent concern FWIW https://review.opendev.org/#/c/712741/6/nova/tests/functional/libvirt/test_vgpu.py@45 | |
| 13:03:32 | gibi | francoisp: based on a recent agreement you only need to keep the two company rule for high impact changes, anything that | |
| 13:03:35 | gibi | involves a microversion, service version, rpc version, or database | |
| 13:03:36 | gibi | migration. | |
| 13:03:45 | gibi | francoisp: but sure I will look at that bugfix | |
| 13:04:50 | gibi | bauzas: honestly I failed to prove that it can actaully cause any problem but if you can add someting to the setUp to reset the test object level variable that could scratch my itch | |
| 13:04:52 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Allocate mdevs when resizing or reverting resize https://review.opendev.org/712741 | |
| 13:04:53 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Pass allocations to virt drivers when reverting resize https://review.opendev.org/712118 | |