Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-11
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 sean-k-mooney not really
20:35:57 artom To better understand what we need to encode in the objects/business logic
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 artom sean-k-mooney, heh, OK, define hypervisor then :)
20:39:07 sean-k-mooney its an aclleration driver/kernel module
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 edmondsw yep... and all could go away at this point, right?
20:42:56 artom And I'm proposing to remove all of them: https://review.openstack.org/#/c/636210/
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: Make dest send NUMAMigrateData to the source https://review.openstack.org/634828
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: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
21:53:27 openstackgerrit Merged openstack/nova stable/queens: Add functional regression test for bug 1794996 https://review.openstack.org/623354
21:53:28 openstack bug 1794996 in OpenStack Compute (nova) queens "_destroy_evacuated_instances fails and kills n-cpu startup if lazy-loading flavor on a deleted instance" [High,In progress] https://launchpad.net/bugs/1794996 - Assigned to Matt Riedemann (mriedem)
22:13:32 mriedem oh blarg, our NeutronFixture is not a KSA client fixture...
23:03:06 mriedem efried: hmm, i'm hoping this isn't a regression http://paste.openstack.org/show/744898/
23:03:28 mriedem this is on rollback of a failed resize where conductor reverts the allocation move by trying to swap the allocations back from the migration consumer to the instance consumer
23:03:40 mriedem the GET /allocations/$consumer_id in the code is using placement 1.28
23:03:45 mriedem which should have project_id in the response
23:04:26 mriedem i wonder if maybe allocations is {} ?
23:05:10 efried That seems possible.
23:05:55 efried reproducible? Stuff in some debug logs?
23:06:56 mriedem OH I KNOW
23:07:15 mriedem so this is the conductor MigrationTask trying to swap the allocs, but the compute service in this case already reverted them
23:07:24 mriedem so conductor is trying to move something that doesn't exist b/c it's already been moved
23:07:36 mriedem not a regression, just not robust code
23:08:58 efried seems like we've seen this before
23:09:09 efried thought we had a fix up that would ignore empty source allocations
23:10:56 mriedem this is a pretty specific part of nova so i'm guessing it's just not been an issue before,

Earlier   Later