| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-11 | |||
| 20:35:49 | mriedem | you don't need depends-on within the same repo | |
| 20:36:13 | mriedem | so just remove the resize test from the top patch, then commit it into a new change at the top of the series, | |
| 20:36:14 | sean-k-mooney | claudiub: ya the way os-vif is written it should work with or without the patch but it annoys me that it works just because we happened ot make the functions not fail if there preconditions were violated | |
| 20:36:19 | mriedem | and then git review -R -y | |
| 20:36:24 | stvnoyes | ok | |
| 20:36:51 | mriedem | if you need to go back within the series to change something, you do an interactive rebase (git rebase -i HEAD~4) or whatever | |
| 20:39:44 | efried | git restack is your friend | |
| 20:40:00 | efried | Well, *my* friend, anyway. | |
| 20:40:13 | sean-k-mooney | efried: git restack? | |
| 20:41:00 | sean-k-mooney | i use git rebase -i often but never heard of git restack | |
| 20:41:01 | efried | sean-k-mooney It rolls up rebase -i and some of the other paperwork associated with managing a big series. | |
| 20:41:24 | efried | sean-k-mooney I think it was developed by stackers. Separate plugin. pip install git-restack (I think) | |
| 20:41:36 | sean-k-mooney | https://github.com/openstack-infra/git-restack so yes | |
| 20:41:56 | sean-k-mooney | git review is also an infra tool | |
| 20:42:16 | efried | no kidding, really? TIL | |
| 20:42:40 | efried | Was using that long before I got involved with openstack. | |
| 20:42:43 | sean-k-mooney | yep https://github.com/openstack-infra/git-review | |
| 20:44:17 | sean-k-mooney | efried: stephenfin: also intoduced me to gerrithub.io which allows you to add gerrit to any github project so you can use the openstack/gerrit workflow with it insteadk of pull requests | |
| 20:44:41 | efried | oooooooo | |
| 20:44:44 | bauzas | I don't like git restack | |
| 20:44:51 | bauzas | I prefer the old but clean way | |
| 20:45:12 | efried | Would you like it better if it was git restaque? | |
| 20:45:16 | bauzas | because something you can be messed up in a long series and then git reflog is your friend | |
| 20:45:52 | dansmith | bauzas: you never started talking | |
| 20:46:07 | bauzas | dansmith: I was holding my breath till your email :p | |
| 20:46:08 | dansmith | bauzas: I was expecting you to start asking your question and I'd get to it when I was done | |
| 20:46:18 | dansmith | bauzas: but then I got distracted with no ping :) | |
| 20:46:23 | bauzas | my bad, misunderstanding | |
| 20:46:24 | dansmith | anyway, shoot | |
| 20:46:26 | bauzas | so | |
| 20:47:13 | bauzas | dansmith: in case you don't know yet, we're attaching a mdev device based on the allocation | |
| 20:47:22 | bauzas | that works fine | |
| 20:47:57 | bauzas | but the issue we have is that there are a shit ton of other methods that rebuild that guest XML, and those don't have the allocations passed yet | |
| 20:48:20 | dansmith | 1.0 shit tons?\ | |
| 20:49:25 | bauzas | I mean, spawn() is calling it, but also for example hard_reboot | |
| 20:49:52 | bauzas | which itself is called by power_on()... | |
| 20:49:53 | bauzas | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L2658 | |
| 20:49:54 | dansmith | yeah, I get it | |
| 20:50:30 | bauzas | so, I tested on my node, and indeed, if you stop/start your instance, you're not missing the mdev | |
| 20:50:39 | dansmith | not missing? | |
| 20:50:39 | bauzas | you're missing* | |
| 20:50:42 | dansmith | heh yeah okay | |
| 20:50:58 | bauzas | so there are 2 options | |
| 20:51:18 | bauzas | #1 pass the allocations down to where we need, exactly like we did for spawn() | |
| 20:51:33 | bauzas | that means we need to modify every caller | |
| 20:51:53 | bauzas | #2 introspect the flavor rather than allocations | |
| 20:52:05 | dansmith | #2 is cheating | |
| 20:52:24 | dansmith | the reason I asked about the fraction of shit tons is... how many actual calls are we talking about here? | |
| 20:53:43 | bauzas | lemme grep that | |
| 20:54:37 | bauzas | my grep tells me 6 times | |
| 20:54:54 | dansmith | that doesn't seem untenable to me | |
| 20:55:13 | dansmith | granted you have to do it in multiple drivers, but.. it's not a billion | |
| 20:55:19 | dansmith | that's like 0.0001 shit tons :) | |
| 20:55:24 | bauzas | resume, unrescue, finish_migration, power_on and so far | |
| 20:55:43 | dansmith | I guess the other thing we need to do is cache those in compute manager for when we need them because spawn brings them, but the other calls won't | |
| 20:55:53 | dansmith | so the other way to look at this is, | |
| 20:56:00 | bauzas | dansmith: 'those' == allocs ? | |
| 20:56:06 | dansmith | that maybe this is a libvirt problem | |
| 20:56:09 | dansmith | yes those == allocs | |
| 20:56:45 | dansmith | like, I wonder if the other drivers would have this problem, or if they would just not blindly regenerate the vm definition | |
| 20:57:02 | dansmith | you could just read the xml to get the mdev info before you blow it away, right? | |
| 20:57:07 | bauzas | dansmith: that's something I wondered earlier in the day | |
| 20:57:07 | dansmith | and you wouldn't need the full allocation | |
| 20:57:29 | bauzas | I mean I was surprised to see we were generating the XML every time | |
| 20:57:35 | dansmith | well, | |
| 20:57:44 | dansmith | I don't think that refreshing it all the time is bad, | |
| 20:58:00 | dansmith | because it gives us a chance to update things like the ceph monitor ip address and such | |
| 20:58:07 | dansmith | but we don't have to do it blindly | |
| 20:58:14 | bauzas | yeah I can understand that reasoning | |
| 20:58:43 | bauzas | but yeah, instead of just amending what's needed, we're just wiping out the whole config | |
| 20:58:49 | bauzas | anyway | |
| 20:58:53 | bauzas | this is what it is | |
| 20:58:56 | dansmith | you could probably write a clean utility method that is "get me the mdev(s) from $guest" and just call that everywhere | |
| 20:59:18 | dansmith | everywhere you need it that is.. don't call it EVERYWHERE :P | |
| 20:59:28 | dansmith | and don't call it everywhere() because that won't be very obvious | |
| 21:00:07 | bauzas | I need to look at the driver code | |
| 21:00:08 | openstackgerrit | Matt Riedemann proposed openstack/python-novaclient master: Remove deprecated MigrationManager.list cell_name kwarg https://review.openstack.org/532972 | |
| 21:00:08 | openstackgerrit | Matt Riedemann proposed openstack/python-novaclient master: Fix being able to create a reno using tox -e venv https://review.openstack.org/532971 | |
| 21:02:25 | cfriesen | Is it documented anywhere that deleting a quota-set for a tenant will also delete it for any users within that tenant? (This seems to be how it behaves, but I don't see it in the API ref.) | |
| 21:03:41 | melwitt | cfriesen: user quota is a subset of tenant quota so I think it would behave that way. and I doubt it's documented | |
| 21:04:36 | cfriesen | melwitt: I can see the rationale, was just a bit unexpected. | |
| 21:05:05 | bauzas | dansmith: mmm, your last approach could work, except for migrations (but I think it's a silverlining) | |
| 21:05:27 | dansmith | bauzas: we should have allocations passed for migrations I imagine | |
| 21:05:30 | bauzas | dansmith: I mean, we can get the guest, hence its mdevs, before we regenerate it | |
| 21:05:31 | dansmith | we have to look them up anyway | |
| 21:05:40 | melwitt | cfriesen: yeah. user quota was an afterthought. originally it was tenant quota only (and I think it might go back that way eventually). so to tack on user quota, all they did was add a user column and filter on project AND user if user quota | |
| 21:06:04 | bauzas | dansmith: right, I just feel migrations are off the scope for queens | |
| 21:06:44 | dansmith | yeah, but it's good to think about how it'd be handled | |
| 21:06:49 | dansmith | we don't even know if libvirt could do that I guess | |
| 21:06:49 | stvnoyes | mriedem: how exactly do I remove the test from the top patch? I don't want to make things worse. (git is not my strong point). | |
| 21:07:25 | mriedem | stvnoyes: you just undo your change | |
| 21:07:33 | mriedem | stvnoyes: want me to do it? | |
| 21:07:56 | stvnoyes | that might be easier, just let me know what you did, for future reference. | |
| 21:08:27 | stvnoyes | once you do that I'll submit a new change | |
| 21:09:23 | edmondsw | mriedem sdague the PowerVM vSCSI patch is also ready for review now, which is the last one for that bp | |
| 21:09:36 | sean-k-mooney | dansmith: bauzas just reading back im not that surprised to seee the xml being regenerated on hard reboots its requried for some config changes | |
| 21:09:49 | dansmith | sean-k-mooney: yup | |
| 21:10:15 | dansmith | I've argued for doing it more often than we need even so we can freshen it, since some instances run for years without us getting to tweak things | |
| 21:10:19 | sean-k-mooney | bauzas: you will need to be careful not to miss livemigration too as we regenerate part of the xml there also + resize | |
| 21:10:22 | dansmith | we just don't need to do it blindly | |