| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-10 | |||
| 12:24:03 | alex_xu | I'm thinking if the user done that, whether it means there already have other thing goes wrong | |
| 12:24:03 | sean-k-mooney | generally it can be recoverd but we should really treat that as write only | |
| 12:24:45 | sean-k-mooney | its possible yes | |
| 12:25:01 | alex_xu | so binding profile is only for neutron use, but nova should have it own copy | |
| 12:25:44 | alex_xu | the hard thing is if the nova copy is different with neutron one, what should nova do | |
| 12:25:54 | sean-k-mooney | binding profile was intended to be used to pass info from nova to neutorn in one direction | |
| 12:26:05 | sean-k-mooney | alex_xu: use the nova one | |
| 12:26:11 | artom | sean-k-mooney, yeah, that is a good idea | |
| 12:26:13 | alex_xu | ok | |
| 12:26:29 | alex_xu | then correct the neutron info | |
| 12:26:30 | artom | And to internalize the function that generates that map into the NUMATopology object itself | |
| 12:26:34 | sean-k-mooney | alex_xu: https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/definitions/portbindings.py#L31-L34 | |
| 12:26:43 | artom | Not sure we have enough time before FF though | |
| 12:26:50 | sean-k-mooney | alex_xu: correct field that differ ideally | |
| 12:27:04 | sean-k-mooney | but we cant do that based on the info cache we have today | |
| 12:27:08 | artom | sean-k-mooney, also, if those _filter functions are really meant to be purely functional, then @classmethod is not the way to do it - you put them outside the class entirely :) | |
| 12:27:26 | artom | I'll see what I can hack up today | |
| 12:27:45 | sean-k-mooney | artom: why class method are prefectly valid ways to do that | |
| 12:28:02 | alex_xu | for yongli's patch, should we block it for that issue? | |
| 12:28:04 | sean-k-mooney | you coudl put them outside the class entirely or even make them static | |
| 12:28:23 | alex_xu | I feel it is hard for yongli to fix that, since it is boarder issue | |
| 12:28:26 | sean-k-mooney | alex_xu: am no we could proceed and clean this up next cycle | |
| 12:28:38 | alex_xu | yea | |
| 12:28:48 | sean-k-mooney | alex_xu: im just uncomfortable realying on that | |
| 12:29:05 | sean-k-mooney | i was hoping we could ask cybrog for the arq using the neutorn port uuid | |
| 12:29:18 | sean-k-mooney | and have cyborg be the souce of truth | |
| 12:29:40 | alex_xu | but nothing we can do it now, I don't think system metadata is good place, since we said before, we shouldn't anything to it, since it without version control | |
| 12:30:18 | sean-k-mooney | alex_xu: actully i thought the preferece was to use it for things linke this over adding more tables | |
| 12:30:29 | sean-k-mooney | alex_xu: we can have version contol by using ovos | |
| 12:30:48 | sean-k-mooney | it allow use to store addtional info without db migrations | |
| 12:31:03 | sean-k-mooney | but ok lets put this issue assign for now and maybe add a todo | |
| 12:31:42 | sean-k-mooney | alex_xu: can we maybe add a test to vaoidate unshelve and ensure that the port profile is updated with the new arq? | |
| 12:32:32 | alex_xu | sean-k-mooney: I don't yongli implement that, I remember he said unshelve is out of scope for the spec | |
| 12:32:53 | sean-k-mooney | didnt we add support for shelve with cyborg recently | |
| 12:33:06 | sean-k-mooney | if its not supproted with neutron port then we would need to block it yes | |
| 12:33:48 | alex_xu | I think when yongli's spec merged, the shelve with cyborg doesn't merge yet | |
| 12:34:21 | alex_xu | I think the missing part is update the new arq uuid | |
| 12:34:45 | sean-k-mooney | yep | |
| 12:34:52 | sean-k-mooney | so we allow shelve now https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4169 | |
| 12:35:15 | sean-k-mooney | if the compute service version is new enough | |
| 12:36:34 | sean-k-mooney | alex_xu: ok what we can do is proceed as it is | |
| 12:37:08 | sean-k-mooney | and when i submit the patch to block shelve for sriov instance with a workaoutnd config option i can include cyborg too | |
| 12:37:51 | sean-k-mooney | infact it will do it for both automaticlly | |
| 12:37:59 | alex_xu | ok, so we want a explictly block for shelve instance? | |
| 12:38:13 | sean-k-mooney | i was just going to check if any of the port vnic types were in VNIC_TYPES_DIRECT_PASSTHROUGH which the cyborg ones are | |
| 12:38:53 | sean-k-mooney | yes becuse a vm form one tenatn can break another tenat by unshelving | |
| 12:39:37 | sean-k-mooney | so like numa migration i want to block shelve for all instance using neutron sriov port with a [workaround]/allow_unsafe_shelve to renable it and backprot that | |
| 12:39:44 | sean-k-mooney | then i plan to fix it as a bug | |
| 12:40:06 | sean-k-mooney | so we can fix shleve for both sriov and cybrog by ensuring the port profile si updated for the new arq | |
| 12:40:33 | sean-k-mooney | so i planned to return a 403 permission deined | |
| 12:41:11 | sean-k-mooney | unless you had [workaround]/allow_unsafe_shelve=true or you had a fixed verion of nova | |
| 12:41:34 | sean-k-mooney | alex_xu: does that sound resaonable? | |
| 12:41:50 | sean-k-mooney | gmann: ^ that should be ok form the api point of view yes? | |
| 12:42:19 | sean-k-mooney | gmann: the exisitng issue for sriov ports is a long term bug so i dont want to have to make an api micorverion bump for that | |
| 12:42:50 | sean-k-mooney | 403 can be retruned form this already and i think that should be valid? | |
| 12:43:50 | alex_xu | sean-k-mooney: or we have a check whether instance have a port with device profile? then we needn't an workaround option | |
| 12:43:57 | sean-k-mooney | gmann: the current block we have uses https://github.com/openstack/nova/blob/ab07507e5cfce6232fef373d07ff92ea704541da/nova/exception.py#L158-L159 | |
| 12:44:27 | sean-k-mooney | alex_xu: i want the workaound for people that are using this with sriov today | |
| 12:44:47 | sean-k-mooney | alex_xu: i know that it currenlty does the wrong thing and you can end up using the wrong device | |
| 12:44:53 | sean-k-mooney | which is why i want to blcok it | |
| 12:45:16 | sean-k-mooney | but we have had custoemr manually update the port profile and hard reboot the instace to match the claimed device | |
| 12:45:31 | sean-k-mooney | its a horable hack but they really wanted to keep shelve | |
| 12:45:47 | alex_xu | sean-k-mooney: ok, got it | |
| 12:45:55 | sean-k-mooney | so if they want to shoot them selves in the foot then that is what the workaroudn is for | |
| 12:46:25 | alex_xu | ok | |
| 12:47:03 | sean-k-mooney | alex_xu: if your ok with handeling the shelve issue this way i guess im ok with using the requeted_networks for now | |
| 12:47:11 | sean-k-mooney | and the port profile | |
| 12:47:16 | alex_xu | sean-k-mooney: yes, I'm ok with that | |
| 12:47:49 | alex_xu | then we need to wait yonglihe back, he is going to take his boy back from school :) | |
| 12:49:09 | sean-k-mooney | ill go link this converation to the gerrit review | |
| 12:49:32 | alex_xu | cool | |
| 12:50:58 | sean-k-mooney | im more or less ok with the patch other then that but still want to see if gibi is ok with defining the constanct before they are need | |
| 12:51:52 | alex_xu | yea, definitely need gibi's feedback | |
| 12:51:55 | sean-k-mooney | we may also need some extra test coverage for the vnic_types i havent fully review the last patch | |
| 13:07:21 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP zuul: Replace grenade and nova-grenade-multinode with grenade-multinode https://review.opendev.org/c/openstack/nova/+/778885 | |
| 13:18:55 | sean-k-mooney | artom: so since your pci socket policy patch conflit with both my port numa patch and vdpa series we proably should set them up in a chain | |
| 13:19:28 | artom | sean-k-mooney, sure. Which one of yours is more likely to land at this point? | |
| 13:19:36 | artom | I guess put that one on top of my socket stuff? | |
| 13:19:37 | sean-k-mooney | vdpa obviously goes at the end since that is the lesast likely to be ready | |
| 13:19:45 | artom | Yeah, what I thought :) | |
| 13:19:51 | sean-k-mooney | ya the port numa one | |
| 13:20:15 | artom | I'm in the process of adding some patches to my series, I'll let you know when it's pushed? | |
| 13:20:54 | sean-k-mooney | would you be oke putting your patch on top of the port numa one? if i rebase it on top of master to pic up the patches of your that already merged | |
| 13:21:45 | sean-k-mooney | hum i guess yours already had 2 +2s buyt you need to change it for the multi socket thing | |
| 13:22:16 | sean-k-mooney | i dont think there are outstanding quesion left for mine. | |
| 13:22:24 | artom | sean-k-mooney, mind linking your series? | |
| 13:22:45 | sean-k-mooney | its one patch or the prot numa polices https://review.opendev.org/c/openstack/nova/+/773792 | |
| 13:22:47 | artom | Ah, there's only the one? https://review.opendev.org/c/openstack/nova/+/773792 | |
| 13:23:12 | sean-k-mooney | ya i should have a second for numa vswitch but i wont get to that until next cycle so just one for sriov | |
| 13:23:43 | artom | There aren't serious conflicts there, though | |
| 13:23:44 | sean-k-mooney | i was going to say maybe defer to stephenfin and gibi for order | |
| 13:23:57 | artom | Maybe some trivial "context code" in the tests | |
| 13:23:58 | sean-k-mooney | correct its minor | |
| 13:24:09 | sean-k-mooney | just dont wnat them to fight in the gate | |
| 13:24:21 | sean-k-mooney | sicne one of the two will merge first | |
| 13:24:30 | sean-k-mooney | and the other need to be respun | |
| 13:24:46 | artom | sean-k-mooney, lemme drop the kids off | |
| 13:24:51 | artom | Back in ~1 hour | |
| 13:26:09 | sean-k-mooney | sure ill wait for stephenfin or gibi to comment and ill do whatever they say | |
| 13:26:36 | sean-k-mooney | if they say put user first ill rebase on top of https://review.opendev.org/c/openstack/nova/+/772779 | |