| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-26 | |||
| 15:33:59 | dansmith | he probably wants _get_live_migrate_numa_info() in the libvirt patch | |
| 15:34:09 | dansmith | https://review.opendev.org/#/c/634828/48/nova/virt/libvirt/driver.py | |
| 15:34:09 | artom | Since it only goes by the flavor extra spec | |
| 15:34:15 | artom | So we need extra logic to block it | |
| 15:34:20 | dansmith | yeah, I know | |
| 15:34:27 | dansmith | he's asking where the new guest numa topo gets created I think | |
| 15:34:27 | sean-k-mooney | dansmith: artom shoudl be calling the hardware moduel/RT to caluate a new numa toplogy, validate page size and claim pCPUs on the destation | |
| 15:34:45 | dansmith | sean-k-mooney: yeah, I'm not sure about that last bit | |
| 15:34:54 | dansmith | validate page size yes, in the claim | |
| 15:35:05 | dansmith | calculate a new topology, in the above link | |
| 15:35:14 | dansmith | but not sure I see where the dest claims that, tbh | |
| 15:35:19 | sean-k-mooney | dansmith: it shoudl all be don in can_fit_numa_to_host or something like that let me see if i can find it | |
| 15:35:31 | artom | sean-k-mooney, https://github.com/openstack/nova/blob/master/nova/compute/claims.py#L138 | |
| 15:35:53 | sean-k-mooney | yes | |
| 15:35:55 | sean-k-mooney | hardware.numa_fit_instance_to_host | |
| 15:36:11 | sean-k-mooney | is the entry point to all the nuam/hugepage/pinning code | |
| 15:36:20 | dansmith | oh, that's already there because of regular migrations right? | |
| 15:36:41 | sean-k-mooney | it looks at all those requets and returns a numa toplogy object that fultiles all of the request or raise an excepiton | |
| 15:36:42 | artom | dansmith, that's actually the non-move Claim, so even booting instances use it | |
| 15:36:48 | dansmith | okay | |
| 15:36:52 | sean-k-mooney | dansmith: its tere for cold migration | |
| 15:36:59 | dansmith | yeah | |
| 15:37:01 | dansmith | whatever | |
| 15:38:48 | sean-k-mooney | if hardware.numa_fit_instance_to_host does not raise one of like50 exceptions then the host is valid and it returns the numa toplogy | |
| 15:39:40 | artom | exception.TooHardIGiveUpDoItYourself | |
| 15:39:52 | artom | exception.TheCloudWasAMistak | |
| 15:58:59 | sean-k-mooney | artom: :) that might be clearer then some of the messages we raise | |
| 16:01:52 | artom | sean-k-mooney, hah! exception.TurnBackWhileYouCan | |
| 16:30:39 | stephenfin | bauzas: If you haven't already gone home, want to send this through? https://review.opendev.org/#/c/672336/ | |
| 16:31:05 | bauzas | stephenfin: sure, that will help not going crazy with my visa application form | |
| 16:32:48 | bauzas | stephenfin: ouch, a bit hard to review given we need to make sure we don't regress on the test coverage | |
| 16:32:55 | bauzas | did efried checked it too ? | |
| 16:33:50 | efried | yeah, I did. | |
| 16:33:51 | stephenfin | Yeah, I haven't removed anything | |
| 16:33:55 | stephenfin | It's just moving things around | |
| 16:34:06 | stephenfin | I add a good few things to that later so I wanted it broken up | |
| 16:43:19 | bauzas | ok, let's make a trust bond | |
| 16:47:13 | efried | fungi: stephenfin is quoting you here https://review.opendev.org/#/c/677969/ as agreeing we don't need eggs. If that's so, would you mind throwing a +1 on there for me? | |
| 16:47:56 | fungi | efried: sure, just a sec | |
| 16:50:43 | efried | fungi, stephenfin: okay, just as a sanity check, I codesearched: http://codesearch.openstack.org/?q=egg%3Dnova&i=nope&files=&repos= | |
| 16:51:58 | fungi | as far as i know, that's merely how you tell pip what to name the package it's installing. in modern usage it's actually going to build a wheel from the code there | |
| 16:52:20 | fungi | because it's not starting from an actual package, pip doesn't know what the package name for that repository should be | |
| 16:52:49 | fungi | i believe the name of the option is a bit of unfortunate legacy naming for the sake of backwards compatibility | |
| 16:52:52 | efried | o...kay | |
| 16:53:15 | fungi | i'll double-check the reference materials to confirm | |
| 17:00:06 | efried | dansmith: Are we allowed to rename a field like this https://review.opendev.org/#/c/678447/3/nova/db/sqlalchemy/models.py or do we have to burn it and just create a new one? | |
| 17:00:28 | dansmith | efried: no you can't do that | |
| 17:00:57 | dansmith | efried: well, wait, nothing has ever been written there right? | |
| 17:01:00 | efried | right | |
| 17:01:03 | dansmith | that one specifically | |
| 17:01:09 | efried | right | |
| 17:01:14 | dansmith | technically you could although it's... icky | |
| 17:01:14 | efried | never used for anything at all | |
| 17:01:28 | dansmith | this is why, btw, we should never merge db changes until the patches above are at least close | |
| 17:01:31 | efried | https://review.opendev.org/#/c/678447/3/nova/db/sqlalchemy/migrate_repo/versions/401_add_resources.py help? | |
| 17:01:39 | sean-k-mooney | efried: i was discussing this with alex_xu this morning | |
| 17:01:50 | efried | yeah, I still have the bruises from the original smackdown dansmith | |
| 17:01:56 | sean-k-mooney | i was suggesting just ahveing anothe migration to nuke it and add the new field | |
| 17:02:38 | sean-k-mooney | given that nothing has used it and we have not released with it | |
| 17:03:17 | sean-k-mooney | the previous version did this which was worse https://review.opendev.org/#/c/678447/2/nova/db/sqlalchemy/migrate_repo/versions/398_add_resources.py | |
| 17:03:58 | efried | dansmith: I'll -2 this for reasons stated if you want to review/approve it. | |
| 17:12:10 | fungi | efried: stephenfin: https://pip.pypa.io/en/stable/user_guide/#requirements-files #4 has an example of that syntax, and the terminology is still semi-relevant since sdists include a packagename.egg-info directory for their metadata, but the egg_info block in setup.cfg looks like it's related to old d2to1 functionality related to some sphinx extension for reporting version information (which make sense | |
| 17:12:11 | fungi | given the names of the options in it) | |
| 17:12:50 | fungi | that has appeared in nova's setup.cfg since 2010 according to git, and seems to have been cargo-culted into numerous projects who copied their setup.cfg from nova's | |
| 17:13:41 | efried | shocking | |
| 17:13:43 | fungi | it seems to be entirely vestigial at this stage, and a lot of newer projects don't include it (those newer projects which do include it have, i suspect, just copied it from older projects not knowing any better) | |
| 17:14:18 | efried | okay, well, I suppose it's probably an easy enough thing to revert & backport if we find out in four years that we broke somebody. | |
| 17:14:25 | efried | Thanks fungi | |
| 17:14:46 | fungi | yeah, if all the copying started from nova, then maybe the undoing also has to start there ;) | |
| 17:15:12 | fungi | mordred and/or dhellmann may have sufficient context to explain what that block was actually for once upon a time | |
| 17:15:35 | fungi | the fact that it references svn dates it nicely | |
| 17:18:38 | mordred | fungi: what did I do? | |
| 17:19:13 | mordred | oh that. I'd love if it went away | |
| 17:19:18 | fungi | mordred: discussing the necessity of the [egg_info] block in setup.cfg | |
| 17:19:25 | fungi | (or lack of necessity) | |
| 17:19:58 | mordred | oh - wait - I thought it was the boilerplate in setup.py for eventlet ... one sec | |
| 17:20:45 | fungi | mordred: egg_info.{tag_build,tag_date,tag_svn_revision} was something to do with a sphinx extension? | |
| 17:21:12 | fungi | i tried piecing together old commit messages and changelog entries from d2to1 to work out why it was there | |
| 17:21:46 | mordred | ah. I believe that is some text that setuptools would write to setup.cfg if it wasn't there | |
| 17:22:03 | mordred | so we added it to avoid it being added and being a diff when running jobs | |
| 17:22:15 | dansmith | efried: did you mean +2? | |
| 17:22:22 | mordred | I'm guessing setuptools has since stopped writing random crap to setup.cfg senselessly :) | |
| 17:22:32 | fungi | fwiw i've not seen setuptools write anything into setup.cfg files in modern usage | |
| 17:22:38 | mordred | yeah | |
| 17:22:45 | mordred | it's possible it was a distutils thing even | |
| 17:22:52 | efried | dansmith: I mean I'm putting a -2 to hold the series, so that you can review and +2 it without fear of it accidentally merging like last time. | |
| 17:22:52 | mordred | like - that's some OLD cruft | |
| 17:23:08 | dansmith | efried: ah, cool | |
| 17:23:21 | fungi | thanks mordred! | |
| 17:26:24 | efried | mriedem: would I be correct in surmising that "cold migration" existed in the world before "live migration" | |
| 17:26:50 | sean-k-mooney | efried: for most hyperviors yes | |
| 17:27:09 | sean-k-mooney | when nova was recreate both where a thing | |
| 17:27:16 | efried | I can't think of any other reason why "migration" means "cold migration" in the API. | |
| 17:27:18 | sean-k-mooney | but i think live migration was added later | |
| 17:27:59 | sean-k-mooney | cold migration works in more cases the live migration can | |
| 17:28:16 | sean-k-mooney | for excampl you could cold migreate and ironic server or rsd system in principal | |
| 17:28:40 | sean-k-mooney | live migrating either would be much harder even if it was possibel which it is not today | |
| 17:28:57 | sean-k-mooney | so haveing cold migration which is a lower barrier to entry be the default makes sense | |
| 17:31:06 | sean-k-mooney | efried: i dont have a linke but extra is inteded to optional dependcies or tools that are need for building but not using a project. we do not need postgress or mysql to run our tests we can use sqlite to execute all the db test | |
| 17:31:28 | sean-k-mooney | we opertunistalcly try to use mysql or postgerss if they are installed but its not a requirement | |