Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-11
20:01:57 openstackgerrit Artom Lifshitz proposed openstack/nova master: Add migration param to check_can_live_migrate_destination https://review.openstack.org/634605
20:01:58 openstackgerrit Artom Lifshitz proposed openstack/nova master: New objects to transmit NUMA config from dest to source https://review.openstack.org/634827
20:01:58 openstackgerrit Artom Lifshitz proposed openstack/nova master: [WIP] Use live_migration_claim() to check dest resources https://review.openstack.org/634606
20:01:59 openstackgerrit Artom Lifshitz proposed openstack/nova master: [WIP] LM: update NUMA-related XML on the source https://review.openstack.org/635229
20:01:59 openstackgerrit Artom Lifshitz proposed openstack/nova master: [WIP] LM: generate new NUMA XML on the dest and send it to the source https://review.openstack.org/634828
20:02:00 openstackgerrit Artom Lifshitz proposed openstack/nova master: Remove _legacy_dict methods https://review.openstack.org/636210
20:03:01 openstackgerrit Matt Riedemann proposed openstack/nova master: Avoid BadRequest error log on volume attachment https://review.openstack.org/581453
20:03:19 artom mriedem, ^^ The bottom-most patch there is remove the _to_dict stuff
20:03:45 artom (I assume the bot just spammed the channel with my patches, I wasn't actually connected, and eavesdrop isn't showing anything (yet?))
20:04:51 openstackgerrit Matt Riedemann proposed openstack/nova master: Provide a useful error message when trying to update non-compute services https://review.openstack.org/620667
20:12:09 artom Wait, so what's the point of default=None in Nova objects if the field doesn't actually get set to None when you init the object without it?
20:12:36 artom Is default just for "actual" values?
20:15:21 dansmith artom: it's the default for if you ask the object to set its defaults
20:15:38 dansmith remember, an object can represent something that is a fraction of the total,
20:15:40 artom dansmith, ah, so not a fallback kind of thing
20:15:48 dansmith like instance if you don't ask for all the stuff to be filled out
20:15:54 artom Fair enough.
20:16:10 dansmith if we took the defaults everywhere, we'd overwrite things in the DB with the defaults because someone didn't query for them and then called .save() to update one little field
20:16:19 artom I still have the omnipresent 'if field in obj and obj.field is not None' construct ;)
20:16:43 artom It makes sense when you explain it that way :)
20:17:02 artom s/still have/still hate/
20:17:54 artom Or obj_attr_is_set
20:18:29 artom But yeah, I see why we need both "unset" and "None", and having automatic None defaults would clobber the former with the latter
20:18:33 artom And they don't mean the same thing
20:27:46 sean-k-mooney artom: i think you can use in or hasattr with ovo too because the baseclass overlods __contains__
20:27:55 sean-k-mooney https://github.com/openstack/oslo.versionedobjects/blob/master/oslo_versionedobjects/base.py#L321
20:28:22 sean-k-mooney so "if x in my_obj:" will work to test if the field is set
20:29:12 artom sean-k-mooney, yep. I'd hoped to avoid all that by setting a None default, but that won't be possible
20:29:29 artom sean-k-mooney, BTW, going to back to yesterday's discussion on https://review.openstack.org/#/c/634828/9/nova/virt/libvirt/driver.py@6808
20:29:45 artom When you say "cell in enumerate(instance_numa_toplogy.cells)"
20:30:05 artom That means the guest cells are *ordered* inside i_n_t, and their order represents the guest cell ID?
20:30:15 artom I mean, I know it's a list, and lists are ordered
20:30:23 artom I just didn't think we encoded anything in that order
20:30:39 sean-k-mooney artom: yes i belive so
20:30:51 artom Because the cell.id field is the *host* cell ID, right?
20:30:57 sean-k-mooney ya
20:31:15 artom OK
20:31:33 artom Hrmpf, yeah, maybe stick with building the LibvirtConfigObjects on the dest?
20:31:39 artom It seems safer :P
20:31:50 artom As a first whack at it, anyays
20:31:51 sean-k-mooney i think we can for now.
20:31:58 sean-k-mooney we really should not have too
20:32:05 sean-k-mooney but its is simpler
20:32:16 artom Yeah, I thought the same when I first tried it in Rocky
20:32:23 artom And then went to code the XML update stuff on the source
20:32:32 artom And realized i_n_t isn't enough
20:33:00 sean-k-mooney the problem is the xml generation is mix with the buisness logic
20:33:13 sean-k-mooney they should have been kept seperate but they werent
20:33:25 artom I don't disagree :S
20:33:31 artom A lot of things should have been ;)
20:33:52 sean-k-mooney so for now i think keeping the changes minimal are fro the best adn maybe we should just decuple it fully latter
20:34:33 sean-k-mooney and by later i mean as a sperate activiy in train
20:35:36 artom We'd need a quorum of virt drivers that can support NUMA LM
20:35:48 sean-k-mooney i also hate that that file is like 9000 lines long
20:35:57 artom To better understand what we need to encode in the objects/business logic
20:35:57 sean-k-mooney not really
20:36:08 artom No? I mean, I have no idea what HyperV wants
20:36:26 artom I think it would help me if I had claudiub explain it to me :)
20:36:34 artom (If he's still around)
20:36:38 sean-k-mooney hyperv support numa but does not to cpu pinning the same way
20:36:59 openstackgerrit Eric Fried proposed openstack/nova master: Use tox 3.1.1 fixes https://review.openstack.org/582392
20:37:02 sean-k-mooney its done in the hyperviors rather then in the nova dirive i think
20:37:34 sean-k-mooney the issue with the libvirt dirver is that neither libvirt or qemu are hyperviors
20:38:39 artom Ah, kvm is the hypervisor
20:38:46 sean-k-mooney qemu is a emulator and libvirt is just an abstraction layer/api. but are totaly dumb and just do what you tell them so nova had to build most of the logic that would normally live in a hypvervisor
20:38:54 sean-k-mooney artom: kvm is not a hypervior either
20:39:07 sean-k-mooney its an aclleration driver/kernel module
20:39:07 artom sean-k-mooney, heh, OK, define hypervisor then :)
20:40:06 artom ... allowing the kernel to act as the hypervisor (according to wiki, at any rate_
20:40:32 artom No point in getting bogged down in semantics, I un-WIPed a couple of the patches, working on a third...
20:41:04 sean-k-mooney a hypervior is a set of services that mediates acess to hardware such that multiple kernel can share the hardware
20:41:15 sean-k-mooney :) true
20:42:00 edmondsw artom you're asking about getting rid of to_legacy_dict just for powervm or in general? I don't see it called for any hypervisor
20:42:29 artom edmondsw, yeah, I thought it as just powervm, but it looked to be pretty generalized
20:42:39 artom Ie, all drivers had _legacy_dict stuff in migrate_data.py
20:42:56 artom And I'm proposing to remove all of them: https://review.openstack.org/#/c/636210/
20:42:56 edmondsw yep... and all could go away at this point, right?
20:43:00 edmondsw sounds good
20:43:23 sean-k-mooney artom: well as the name suggest its likely a legacy fuction that was kept for handeling mixed compute node versions
20:44:44 artom Changing stuff in nova is like a plastic surgeon operating on Joan Rivers's face: you have no idea who else has touched stuff before, so really all you hope for is to do the minimal amount of damage while keeping Joan alive.
20:44:46 artom (RIP Joan)
20:45:15 artom sean-k-mooney, so to answer your point, I'm not touch anything, named "legacy" or not, unless I'm damn sure it can be removed.
20:47:10 sean-k-mooney artom: well at least for the base class it was added by dansmith in https://github.com/openstack/nova/commit/038dfd672f5b2be5ebe30d85bd00d09bae2993fc
20:47:38 artom Heh, yeah, saw that. And it occurs nowhere else that I could see.
20:47:44 artom So I figured we're OK to get rid of them all.
20:51:12 sean-k-mooney artom: so based on the conversation with dan earliar are you going to revert the change to https://review.openstack.org/#/c/634827/10/nova/objects/migrate_data.py and ath the if x in y... check
20:51:30 sean-k-mooney e.g. revert the defults
20:51:39 artom sean-k-mooney, yeah
20:51:46 artom And add a proper check
20:51:56 openstackgerrit Merged openstack/nova master: Fup for the bandwidth resource provider series https://review.openstack.org/633776
20:53:58 sean-k-mooney cool am im going to go to sleep earlish tonight i think. ill redeploy the series in my test vms tomorow.
20:54:42 sean-k-mooney i think it more or less should work at this point as the last version i tested seamed pretty close
20:55:02 openstackgerrit Artom Lifshitz proposed openstack/nova master: New objects to transmit NUMA config from dest to source https://review.openstack.org/634827
20:55:03 openstackgerrit Artom Lifshitz proposed openstack/nova master: [WIP] LM: update NUMA-related XML on the source https://review.openstack.org/635229
20:55:03 openstackgerrit Artom Lifshitz proposed openstack/nova master: [WIP] LM: Make dest send NUMAMigrateData to the source https://review.openstack.org/634828
20:55:14 artom sean-k-mooney, noice. There ^^
20:55:57 artom I did the change, will continue adding unit tests where I can without too much trouble, and then I'll start on the automated testing
20:57:32 sean-k-mooney oh cool i am going work on some os-vif stuff for another 30 mins or so. i shold have time to spin up a devstack install and manually live migrate a test vm in the background
21:22:42 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Filter duplicates from compute API get_migrations_sorted() https://review.openstack.org/636224
21:53:21 openstackgerrit Merged openstack/nova stable/queens: Handle IndexError in _populate_neutron_binding_profile https://review.openstack.org/635897

Earlier   Later