Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-06-10
20:17:12 sean-k-mooney yep
20:17:19 sean-k-mooney and we default to auto confirm disabled
20:18:05 sean-k-mooney i more or less had this conversation with stephenfin and mriedem a few hours ago :)
20:18:20 artom (Which is weird UX, btw - because you 'nova migrate', but then you 'nova resize-revert')
20:18:32 sean-k-mooney yep we know :)
20:18:41 artom Yeah, didn't think I was teaching anyone anything
20:19:54 sean-k-mooney hehe its just funny to me because you are reiterating what stephenfin said more or less like 6 hours ago
20:20:32 artom Better late than never, I suppose
20:20:43 sean-k-mooney artom: http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-06-10.log.html#t2019-06-10T14:53:27 :)
20:21:43 artom *snerk* Yeah
20:24:25 sean-k-mooney so are you open to trying to remove the api change to the driver api and filtering the events as i was suggesting ? it shoudl reduce the size of the patch a resonable amount
20:24:43 artom sean-k-mooney, yeah
20:25:04 artom Still opposed to passing 'revert' to _create_domain_and_network though :)
20:25:42 artom But less oot driver pain is indeed better
20:26:29 sean-k-mooney well you might be able to tell form somthing else but why dont you like passing revert to _create_domain_and_network
20:26:41 sean-k-mooney i dont think you replied to the commets did you?
20:27:19 artom It's higher up logic that should live higher up
20:27:24 artom I kinda replied
20:27:45 artom So what I came up with is passing a list of events to wait for
20:28:25 artom I say higher up
20:28:39 sean-k-mooney oh ok. am i guess you could generate that in the driver
20:28:51 sean-k-mooney rather then pass it from teh compute manager
20:29:24 sean-k-mooney that an internal api so it more ok to modify the the driver api as you wont break anything at that point
20:29:47 artom Yeah, so really "higher up" means "more specific"
20:30:06 artom Ie, finish_revert_migration knows we're doing a revert, _cdan has no idea, nor should it
20:30:29 sean-k-mooney artom: dansmith raised a vaild point we need to make sure not to change livemigration behavior
20:30:54 sean-k-mooney artom: oh and i just saw your top level comment regarding revert
20:31:35 artom sean-k-mooney, yeah, which is why I straight up added https://review.opendev.org/#/c/644881/24/nova/compute/manager.py@6499
20:31:53 artom I can probably just call it revert
20:32:00 artom Because it also applied for reverting cold migrations
20:32:21 sean-k-mooney ya you could rename the other one and add live
20:32:26 sean-k-mooney actully no
20:32:29 sean-k-mooney that is more work
20:32:32 sean-k-mooney and its unrelated
20:35:23 artom sean-k-mooney, where are you in Ireland again, btw?
20:35:31 sean-k-mooney Shannon
20:35:53 sean-k-mooney on the west coast more or less
20:37:15 artom https://i.imgur.com/lqfeJZk.jpg West Coast!
20:39:56 sean-k-mooney i was debating if it could go into the compute untils module
20:40:09 sean-k-mooney so in here somewhere https://github.com/openstack/nova/blob/master/nova/compute/utils.py
20:40:27 sean-k-mooney then you can use it for the driver or the compute manger
20:40:44 sean-k-mooney and not put it in the vif if you dont like that
20:40:57 artom It just feels so alien there
20:41:07 artom It's business logic, after all, right?
20:41:17 artom Really specific one at that
20:41:17 sean-k-mooney i was not suggsting puting the event generation in teh VIF by the way
20:41:36 sean-k-mooney jsut the function that says if it should wait in the manager or driver
20:41:49 artom Well.... "is_manager_vif" or w/e you called it is basically the same logic :)
20:42:16 artom It's not the generation part that I have issues with, it's that the VIF model suddenly understand the difference between compute manager and virt driver
20:42:26 sean-k-mooney yes but if we get teh info form neutron in the future i can simply change the impliation of that method
20:42:52 artom Anyways, we're ratholing :)
20:43:01 sean-k-mooney and only for the revert resize case too
20:43:32 sean-k-mooney so nova.compute.utils
20:43:52 sean-k-mooney i think it qualifies as a """Compute-related Utilities and helpers."""
20:44:17 sean-k-mooney that has things like is_volume_backed_instance
20:44:46 artom Fair enough
20:45:14 sean-k-mooney you could put it in the virtapi i guess too
20:46:01 sean-k-mooney if you did that then the manager would call the driver to get teh events
20:46:17 artom Eh?
20:46:19 artom How so?
20:46:51 sean-k-mooney well were you thinking of making it a module level function or adding it to the driver interface
20:47:47 sean-k-mooney virtapi is actuly just event stuff...
20:47:49 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/virt/virtapi.py
20:48:33 sean-k-mooney i guess that would be fine too.
20:49:06 artom This *is* event stuff :)
20:49:20 sean-k-mooney ya i would be fine with adding it there
20:50:09 sean-k-mooney i assummed wrongly that the base compute driver api would be defiended there but that is in driver.py
20:51:25 efried aspiers: Reviewed.
20:51:30 aspiers thanks!
20:51:37 sean-k-mooney damn no wonder the libvirt virt driver is 5000 lines lon the base class is almost 2000
21:12:42 openstackgerrit Artom Lifshitz proposed openstack/nova master: WIP: Revert resize: wait for events according to hybrid plug https://review.opendev.org/644881
21:12:43 openstackgerrit Artom Lifshitz proposed openstack/nova master: WIP: Include direct-physical in compute manager events check https://review.opendev.org/664431
21:13:03 artom sean-k-mooney, mriedem, ^^
21:13:10 artom And I think I need to head home at this point.
21:13:22 artom Or potentially to daycare, if wife doesn't make it there in time :/
21:14:01 sean-k-mooney ack
21:14:18 sean-k-mooney at first glance it certenly shorter
21:14:30 sean-k-mooney if it fixes the issue even better ill take a look
21:14:47 artom Well tests are missing
21:15:14 sean-k-mooney details :)
21:17:37 mriedem probably want to revive sean-k-mooney's dnm test patch to recreate the issue on top of this again if you changed the fix
21:17:59 sean-k-mooney sure i can restroe it and rebase
21:18:29 sean-k-mooney mriedem: did your change to tempest merge or shoudl i add a depense on to remvoe teh skip
21:20:08 mriedem it's approved
21:20:16 mriedem sorry, +2
21:20:23 mriedem but i'm not sure that matters for what you were testing,
21:20:44 mriedem you could merge artom's patch to run the revert resize tests in the nova-live-migration job and add your ovs_hybrid_plug config
21:20:49 sean-k-mooney well artom abandoned his chagne that ran the revert tests
21:21:01 mriedem and you abandoned yours - just copy his stuff into your DNM change right?
21:21:03 sean-k-mooney ya i could do that
21:29:57 openstackgerrit sean mooney proposed openstack/nova master: [DNM] testing bug/1813789 revert resize events https://review.opendev.org/664442
21:32:06 sean-k-mooney interesting... gerrit auto added artom to the comitt i jsut pushed and also aprently he is the author of ^
21:33:27 sean-k-mooney i didnt think i was ment to be allowed to that but whatever
21:43:58 openstackgerrit Matt Riedemann proposed openstack/nova master: Add Migration.cross_cell_move and get_by_uuid https://review.opendev.org/614012
21:43:59 openstackgerrit Matt Riedemann proposed openstack/nova master: Add InstanceAction/Event create() method https://review.opendev.org/614036
21:43:59 openstackgerrit Matt Riedemann proposed openstack/nova master: Add Instance.hidden field https://review.opendev.org/631123
21:44:00 openstackgerrit Matt Riedemann proposed openstack/nova master: Add TargetDBSetupTask https://review.opendev.org/627892
21:44:00 openstackgerrit Matt Riedemann proposed openstack/nova master: Add CrossCellMigrationTask https://review.opendev.org/631581
21:44:01 openstackgerrit Matt Riedemann proposed openstack/nova master: Execute TargetDBSetupTask https://review.opendev.org/633853
21:44:01 openstackgerrit Matt Riedemann proposed openstack/nova master: Add prep_snapshot_based_resize_at_dest compute method https://review.opendev.org/633293

Earlier   Later