| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-06-18 | |||
| 13:02:35 | gibi | I see | |
| 13:02:37 | sean-k-mooney | but if we are supporting it for mdev i dont see why we would not support it for pci | |
| 13:02:53 | sean-k-mooney | i kind fo feel like we shoud go one way or the other | |
| 13:03:13 | gibi | I agree that having 'resource:' in the flavor is a shortcut, but it is an established form for VGPU already | |
| 13:03:14 | sean-k-mooney | the resouce class basiclaly will give use the same level of indrection the pci alais has today | |
| 13:03:28 | sean-k-mooney | but without the need to configure pci aliase in the first place | |
| 13:04:33 | sean-k-mooney | gibi: basically im wonderign should look to eventually remove the pci alaise and just use resouce: for pci passthough or mdev passtough in the future | |
| 13:04:44 | gibi | I can accept that ^^ | |
| 13:05:01 | gibi | they feel pretty equal to me regarding expressivity | |
| 13:05:23 | gibi | but I don't think we have to do that now. as you said, in the future :) | |
| 13:05:58 | sean-k-mooney | ill at least mention it in the spec in the alternitive section as possibel future work | |
| 13:06:06 | gibi | that is totally cool with me | |
| 13:07:25 | sean-k-mooney | the spec will already be split into 2 spec 1 basic passtogh 2 neutron integration i can add 3 which is replaceing alsias with resouce: but that wont happen this cycle | |
| 13:07:40 | gibi | sure | |
| 13:07:44 | gibi | sounds like a plan :) | |
| 13:08:07 | sean-k-mooney | bauzas: so just looping back to your discussion | |
| 13:08:27 | sean-k-mooney | i think im ok with useing resouce: and just checking the config on the compute node for the RC classes | |
| 13:09:07 | sean-k-mooney | i would still prefer to use resouce_class as the config option name but thats minor | |
| 13:09:26 | sean-k-mooney | i can also live with mdev_class if we just have good help text | |
| 13:09:42 | sean-k-mooney | gibi: did you have any other open question on bauzas proposal | |
| 13:10:04 | sean-k-mooney | gibi: lyarwood asked first so im going to review his spec shortly but i can look at bauzas next | |
| 13:11:57 | bauzas | I diverted from IRC | |
| 13:12:05 | gibi | sean-k-mooney, bauzas: that settles my last quesiton in the mdev spec, so I going to reply in the spec and approve it | |
| 13:12:08 | bauzas | any thoughts ? | |
| 13:12:15 | bauzas | ack, ok | |
| 13:13:39 | sean-k-mooney | ok ill try and get to https://review.opendev.org/c/openstack/nova-specs/+/794799 and https://review.opendev.org/c/openstack/nova-specs/+/792796/3/specs/xena/approved/generic-mdevs.rst in the next hour or so | |
| 13:27:56 | gibi | bauzas, sean-k-mooney: could you hit this small spec fix: https://review.opendev.org/c/openstack/nova-specs/+/795493 | |
| 13:29:31 | sean-k-mooney | gibi: i just fast approved that | |
| 13:29:46 | sean-k-mooney | oh | |
| 13:29:49 | sean-k-mooney | actully no | |
| 13:29:55 | sean-k-mooney | can you rename the file to match | |
| 13:30:22 | sean-k-mooney | it shoudl be cyborg-admin-user-client.rst | |
| 13:30:36 | gibi | OK, let me fix that quickly | |
| 13:30:41 | sean-k-mooney | the move implemented spec tool use that to find the blueprint | |
| 13:33:47 | opendevreview | Balazs Gibizer proposed openstack/nova-specs master: Fix the bp link in the cyborg admin token spec https://review.opendev.org/c/openstack/nova-specs/+/795493 | |
| 13:34:00 | gibi | sean-k-mooney: ^^ | |
| 13:35:41 | sean-k-mooney | gibi++ | |
| 13:35:53 | sean-k-mooney | we dont have a karma bot here but still | |
| 13:36:19 | gibi | sean-k-mooney: thanks | |
| 13:36:32 | gibi | the happy days of doing the virtual paperwork as a PTL | |
| 13:36:57 | sean-k-mooney | at least you dont have to do it in triplicate | |
| 13:37:48 | sean-k-mooney | not today but i can proably look at adding a small script that will check for this in the ci | |
| 13:38:12 | gibi | good idea | |
| 13:38:20 | gibi | on the CI script | |
| 13:38:40 | artom | lyarwood, done | |
| 13:38:41 | sean-k-mooney | get the added filename, strip the rst, grep and see if there is a url that end with that and curl it to make sure it does not have a 404 | |
| 13:43:49 | lyarwood | thanks | |
| 13:45:48 | stephenfin | an interesting Python problem | |
| 13:45:51 | stephenfin | import json | |
| 13:45:51 | stephenfin | class Foo: | |
| 13:45:51 | stephenfin | @property | |
| 13:45:51 | stephenfin | def bar(self): | |
| 13:45:51 | stephenfin | return json.loads(self._bar) | |
| 13:45:53 | stephenfin | @bar.setter | |
| 13:45:54 | stephenfin | def bar(self, value): | |
| 13:45:56 | stephenfin | self._bar = json.dumps(value) | |
| 13:45:58 | stephenfin | foo = Foo() | |
| 13:46:02 | stephenfin | foo.bar = {} | |
| 13:46:04 | stephenfin | foo.bar['test'] = 'hello' | |
| 13:46:06 | stephenfin | assert foo.bar == {'test': 'hello'} | |
| 13:46:08 | stephenfin | ^ that fails | |
| 13:46:31 | sean-k-mooney | yes | |
| 13:46:32 | sean-k-mooney | that is expected | |
| 13:46:35 | sean-k-mooney | but you can fix that | |
| 13:46:37 | stephenfin | I get why (the setter is called for setting the attribute itself, not attributes of the attribute) but I don't know how to fix it | |
| 13:47:02 | sean-k-mooney | foo.bar is returing a copy of the data in self._bar | |
| 13:47:07 | sean-k-mooney | which is a dict | |
| 13:47:25 | sean-k-mooney | acn dyou cant do {} = {"key":"val"} | |
| 13:48:28 | stephenfin | well I need to fix it, because I've got a bug here https://github.com/openstack/nova/blob/master/nova/objects/migrate_data.py#L71-L89 | |
| 13:48:51 | stephenfin | that's the pattern I used there and it doesn't work - the 'OS_VIF_DELEGATION' attribute of the embedded profile is never set :-( | |
| 13:49:24 | sean-k-mooney | i fixed this in one of my pathces | |
| 13:49:46 | sean-k-mooney | well for the fiels i added | |
| 13:51:29 | stephenfin | if you can find that I'd like to have a look at it, because right now I'm stumped | |
| 13:51:57 | sean-k-mooney | i tough tit was that to be honnest | |
| 13:51:57 | jkulik | foo.bar = foo.bar.update({'test': 'hello'}) ? | |
| 13:52:26 | jkulik | too far off from the initial intention? | |
| 13:52:36 | sean-k-mooney | jkulik: well the fix is to inste do the update on the underling dict | |
| 13:53:09 | sean-k-mooney | so yes that works | |
| 13:54:00 | stephenfin | dict.update doesn't return anything, so the idea is sound but it needs a slight tweak | |
| 13:54:34 | sean-k-mooney | right not update but you read modify write | |
| 13:54:42 | gibi | yepp the problem is that you have a getter and a setter but foo.bar['test'] only use the getter but not the setter :) | |
| 13:54:50 | stephenfin | foo.bar = dict(foo.bar, **{'test': 'hello'}) | |
| 13:55:04 | stephenfin | will do the trick. Still not as pretty but at least it works | |
| 13:55:16 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/objects/migrate_data.py#L81-L89 should work though | |
| 13:55:28 | stephenfin | gibi: Yeah. Slightly amazed that I haven't (that I can recall) hit this in all my years writing Python :D | |
| 13:55:46 | sean-k-mooney | i hit a proably with it and change it | |
| 13:55:52 | stephenfin | sean-k-mooney: the getter works, the setter does not | |
| 13:55:56 | gibi | I think we need get rid of the foo.bar['aaa'] by returning a frozen dict | |
| 13:55:56 | stephenfin | at least not in tests | |
| 13:56:28 | sean-k-mooney | stephenfin: its suoudl this was one of the things i had to workaround when writhing tha tpatch | |
| 13:56:31 | gibi | as the getter returns a copy of the underlining data not a reference for it | |
| 13:56:48 | gibi | so we have to tell the caller that it is copy | |
| 13:56:52 | gibi | not a live ref | |
| 13:56:55 | jkulik | frozen dict sounds good. then nobody can accidentally use the non-working pattern | |
| 13:57:49 | sean-k-mooney | frozen dict wont work | |
| 13:58:03 | sean-k-mooney | the issue is we are storing a json string | |
| 13:58:19 | gibi | sean-k-mooney: it does not solve the problem, it forces the caller to avoid modifying what is returned | |
| 13:58:23 | stephenfin | well it would indicate that you can't update it | |
| 13:58:25 | sean-k-mooney | and then we are returning it as a dict and we want update to that property to modify the json | |
| 13:58:37 | sean-k-mooney | gibi: right but we want them to be able to do that | |