Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-04-02
12:25:26 stephenfin bauzas: You think you could review https://review.opendev.org/#/c/710037/8/doc/ext/extra_specs.py ? It's not the least bit complicated, if that helps :)
12:25:38 bauzas oh yeah
12:25:42 stephenfin Happy to trade reviews
12:34:20 sean-k-mooney bauzas: at the moment i assume you are not allowing multiple vGPU types on a single pGPU
12:34:41 sean-k-mooney bauzas: there is noting in princiapl that prevents that form working even if nvida does not support that
12:35:21 sean-k-mooney the vfio mediated device framework fully support using multiple mdev_types on the same devcie concurrnetly
12:35:26 alex_xu anyone know why we can swap the same volume before https://review.opendev.org/#/c/693828/25/nova/api/openstack/compute/volumes.py@495...
12:36:35 sean-k-mooney alex_xu: lyarwood is on pto i think but i think we can may do that for a volume retype on the cinder side
12:36:38 alex_xu oh, I guess we can't, that will fail at later voluem status check
12:36:53 alex_xu sean-k-mooney: ah, thanks
12:37:25 sean-k-mooney alex_xu: im not sure if my guess above was correct by the way. we could very well just fail later as you said
12:37:41 alex_xu yea
12:38:13 sean-k-mooney but i think cinder allows use to retype a voule while its attached to an instance and this is the only way i can think if for it to tell nova that it has updated
12:39:20 alex_xu sean-k-mooney: ok, thanks
12:41:26 sean-k-mooney alex_xu: i didnt review the rest of that patch but just -1'd it since its aliasing the id builtin fucntion and using id as a varible which is wrong
12:41:44 sean-k-mooney in this context it wont break anything but we should never do that
12:51:28 alex_xu sean-k-mooney: I'm not sure we can check that, I need to check the api code. we use 'id' in many API. but I agree that seems wrong
12:53:22 sean-k-mooney im not sure we call id directly in nova since we normally dont care about the objects address in memory
12:53:42 sean-k-mooney but its just not good practic to alias builtins in general
12:54:20 sean-k-mooney it would only really be an issue if you did "id=dir" or something at module scope
12:54:47 sean-k-mooney inside a function it will jsut make id unavialbel in that function body
12:57:37 sean-k-mooney but if you do python3 -c "id=dir; help(id)" it will show the help for dir so if you do that at module scope anything that uses id after will get dir instead
12:58:16 alex_xu yea, agree with that
12:58:39 alex_xu sean-k-mooney: this is where id coming from https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/routes.py#L820
13:00:12 sean-k-mooney ah i see ya we proably shoucl change that to <resouce>_id for all of the usages and add a hacking check.
13:00:33 sean-k-mooney that is a seperate change however so ill remove my -1
13:00:46 sean-k-mooney this is an existing bug
13:02:43 openstackgerrit Huaqiang Wang proposed openstack/nova master: Introduce the 'CPUAllocationPolicy.MIXED' enum https://review.opendev.org/716267
13:02:43 openstackgerrit Huaqiang Wang proposed openstack/nova master: hardware: Add support for 'hw:cpu_dedicated_mask' extra spec https://review.opendev.org/714706
13:02:44 openstackgerrit Huaqiang Wang proposed openstack/nova master: Introduce 'pcpuset' field for InstanceNUMACell https://review.opendev.org/716984
13:19:44 artom stephenfin, could you take a look at the bottom patch in the NUMA LM series? Dan Smith is +2, if you agree, we can at least get the first 2 patches in that series in the gate
13:19:54 stephenfin sure
13:20:06 artom The rest can wait until after FF I guess, since there are probably more important series to review
13:20:26 artom stephenfin, err, link: https://review.opendev.org/#/c/714228/2
13:21:43 bauzas sean-k-mooney: that's a correct assumption, I prevent such thing (supporting multiple types concurrently for *each* pGPU)
13:22:38 sean-k-mooney bauzas: ok. well that is not limitation of mdevs just of the nova/nvida usage
13:22:52 sean-k-mooney bauzas: that is fine for now in anycase
13:22:53 bauzas sean-k-mooney: like I also doesn't want to support yet asking for multiple allocations using groups
13:23:16 bauzas i limit to only one allocation for the VGPU resource class
13:23:25 bauzas I mean, people can ask VGPU>1
13:23:29 bauzas (of course)
13:23:40 bauzas but not with numbered request groups
13:23:45 sean-k-mooney ya that is fine
13:24:04 sean-k-mooney i dont think we shoudl use numbered request groups in teh flavor anyway so im fine with that
13:24:50 bauzas I don't return an exception for this
13:24:54 bauzas I just take the first one
13:25:25 sean-k-mooney that could be an improvement to add as a followup
13:25:39 sean-k-mooney bauzas: its documented which is the important thing
13:25:46 bauzas yup
13:28:21 bauzas sean-k-mooney: yeah I know the mdev framework does that
13:28:41 bauzas (I mean having multiple vGPU types per pGPU=
13:29:13 bauzas but then if you create one mdev with the nvidia driver, then automatically and magically the available instances of vGPU for the other types turn to 0 :D
13:29:28 bauzas (at least GRID10 AFAIK)
13:30:23 bauzas anyway, rushing to finish my change so that I can decently review stephenfin's extra spec validation thingies https://review.opendev.org/#/c/704643/20
13:53:36 openstackgerrit jayaditya gupta proposed openstack/nova master: Support for nova-manage placement heal_allocations --cell https://review.opendev.org/714459
13:54:57 nightmare_unreal mriedem: if you get some time can you see this : https://review.opendev.org/#/c/714459/ . I can't seem to figure out the issue with the unit test case I wrote. Thank you :) . I have fixed rest of the points you mentioned
14:15:16 mriedem nightmare_unreal: this? "Exception: This test uses methods that set internal oslo_db state, but it does not claim to use the database. This will conflict with the setup of tests that do use the database and cause failures later."
14:15:40 mriedem that means the unit test class is using NoDBTestCase or whatever it's called, which means something in the test is trying to make a database connection and isn't allowed,
14:16:10 mriedem so either move that test to the functional test class where there is a DB connection, or you have to mock out the DB call to raise the NotFound error or whatever for the test
14:16:46 mriedem your test can mock objects.CellMapping.get_by_uuid to raise CellMappingNotFound
14:17:59 mriedem @mock.patch('nova.objects.CellMapping.get_by_uuid', side_effect=exception.CellMappingNotFound('uuid')
14:18:05 mriedem something like that as a decorator to the test case method
14:20:19 openstackgerrit Merged openstack/nova master: Combine the limits policies in single place https://review.opendev.org/715678
14:28:28 nightmare_unreal Thanks !!
14:35:11 openstackgerrit jayaditya gupta proposed openstack/nova master: Support for nova-manage placement heal_allocations --cell https://review.opendev.org/714459
14:35:37 dansmith brinzhang_: are you working on feedback on the PUT patch?
14:41:43 francoisp stephenfin, just to draw your attention, I answered your last question on https://review.opendev.org/#/c/669674/10 - thanks!
14:42:03 stephenfin ack
14:48:26 openstackgerrit Alexandre arents proposed openstack/nova master: Calculate over_committed_disk_size for raw instances https://review.opendev.org/717037
15:38:22 gmann melwitt: that is correct. I need to propose some ideas on warning disable on oslo side what discussed with lbragstad. But yes all these are planed before FF. I am hoping to finish the policy things first by this Sat or Sunday.
15:50:16 gibi nova meeting starts in 10 minutes on #openstack-meeting-3
15:50:41 bauzas melwitt: sorry I didn't had time yet to review your nova-manage patches, is it still worth it given the -W ?
15:50:51 dansmith gibi: fyi, I'm running final tests on a change to that PUT patch to address just the critical bits of alex_xu's review
15:50:53 bauzas but of course I'll do it if you want
15:51:23 dansmith didn't get a response from brinzhang_ so I assume he's not working on it right now
15:52:58 gibi dansmith: ack. thanks
15:58:18 gmann dansmith: gibi also we need to add schema tests coverage for None to verify those things.
15:59:27 dansmith gmann: I'm changing how those checks work, which might mean not
15:59:47 dansmith device_name appears to be nullable at least, in the schema and in the db
16:00:02 dansmith the others are all separate definitions I think
16:01:06 gmann yeah, other does not allow None from schema only device do.
16:01:53 openstackgerrit Huaqiang Wang proposed openstack/nova master: Introduce 'pcpuset' field for InstanceNUMACell https://review.opendev.org/716984
16:02:45 openstackgerrit Balazs Gibizer proposed openstack/nova master: Fix assertEqual param order in Accelerator tests https://review.opendev.org/717069
16:02:46 openstackgerrit Balazs Gibizer proposed openstack/nova master: Stabilize functional tests https://review.opendev.org/717070
16:04:18 openstackgerrit Dan Smith proposed openstack/nova master: Allow PUT volume attachments API to modify delete_on_termination https://review.opendev.org/693828
16:04:52 dansmith gmann: ^
16:05:08 gmann thanks checking
16:08:00 tosky for stable maintainers: this last review should remove any reference to legacy-tempest-dsvm-full-devstack-plugin-nfs (then we are doing the same in Cinder): https://review.opendev.org/#/c/714958/
16:08:14 gmann dansmith: do not we nee do check dot with old value vefore change - https://review.opendev.org/#/c/693828/25..26/nova/api/openstack/compute/volumes.py@460
16:08:17 gmann before
16:08:54 dansmith gmann: why? we sad that if you PUT with no changes it's not an error right?
16:09:37 dansmith technically BDM should not be doing a db save if there are no changes, but not all objects do that properly.. but either way, no reason to just add more to the conditional really
16:10:12 gmann dansmith: yeah but when we add policy in next patch that become difficult to handle on "update request" vs "no change means not update request"
16:10:29 dansmith gmann: I don't follow
16:11:08 gmann in case of "swap + update" - we consider request is for update only when value is changed right. if value is same then we need to consider no update request and it is onyl swap
16:11:45 gmann in that case, how we can check the update policy so that that policy is being checked only when request is update (means value change)
16:12:02 dansmith in 2.85 we should only consider it a swap if volumeId is changed
16:12:17 gmann but we support swap + update together also
16:12:23 dansmith i.e. only check the stricter policy if volumeId is changed, otherwise we check the regular PUT policy
16:12:24 gmann >2.85

Earlier   Later