| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-04 | |||
| 12:33:13 | kashyap | Okido; should also probably add a constant for QEMU | |
| 12:36:01 | johnthetubaguy | yeah, I thought there was one already, but yeah | |
| 12:37:42 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Bump MIN_{LIBVIRT,QEMU}_VERSION for "Rocky" https://review.openstack.org/558783 | |
| 12:38:18 | kashyap | johnthetubaguy: Seems like (from a `git grep`) I'm the first user of the constant FAKE_LIBVIRT_VERSION in tests | |
| 12:41:49 | johnthetubaguy | hmm, that is curious | |
| 12:58:45 | efried | johnthetubaguy: Thanks for merging the error codes spec. Do you know who's authorized to approve the blueprint (other than mriedem, who doesn't seem to be around)? | |
| 13:05:47 | johnthetubaguy | efried: I can I think, in theory anyone with nova-specs core should be able to | |
| 13:06:16 | efried | johnthetubaguy: Okay, thanks. | |
| 13:06:41 | johnthetubaguy | that should look better now | |
| 13:07:01 | efried | johnthetubaguy: Nice, thanks! In case you felt like looking at the code while the spec is still fresh, it's ready: https://review.openstack.org/#/c/546177/ | |
| 13:07:33 | efried | be nice to have this code in place for edleafe's work on consumer generations. | |
| 13:07:58 | johnthetubaguy | ah, I see your +2 there already, I will try take a look at that today | |
| 13:08:13 | efried | cool, thank you. | |
| 13:14:16 | mriedem | artom_: can you create https://blueprints.launchpad.net/nova/+spec/numa-aware-live-migration please? | |
| 13:22:22 | artom_ | mriedem, ack | |
| 13:24:26 | artom | mriedem, done | |
| 13:24:46 | mriedem | thanks | |
| 13:35:30 | edleafe | jaypipes: around? | |
| 13:36:04 | edleafe | efried: cdent: maybe you guys could help with a question | |
| 13:36:28 | efried | edleafe: I'm listening. | |
| 13:36:59 | edleafe | ok, <1.8, allocations didn't have project_id/user_id | |
| 13:37:09 | edleafe | in those cases, they are None on the object | |
| 13:37:44 | edleafe | the code I'm doing for consumer generations tries to insert a record in the consumer table if one doesn't yet exist | |
| 13:38:15 | edleafe | But there is a NOT NULL constraint on the project_id and user_id columns, so in that case (and many of our tests) the insert fails | |
| 13:38:46 | edleafe | Should I just punt and say that for <1.8, no consumer record can be created? | |
| 13:39:20 | efried | edleafe: tbc, this is because you wanted to be able to put the generation into the consumers table even for earlier microversions, yah? | |
| 13:40:15 | dansmith | mriedem: I was holding this for your approval: https://review.openstack.org/#/c/558059/4 | |
| 13:40:25 | edleafe | efried: well, normally I'd put a conditional and skip for <1.8. But that means that later calls can and will overwrite | |
| 13:40:38 | edleafe | efried: It does seem that that's the best we can do here | |
| 13:40:44 | edleafe | Just wanted to get a second opinion | |
| 13:41:09 | efried | edleafe: So backing up for a second: today (before your code) if you try to create a consumer record with microversion <1.8, what happens? | |
| 13:41:39 | efried | ...because presumably the request payload doesn't require the proj/user IDs at <1.8 | |
| 13:41:46 | edleafe | efried: the code checks for the presence of those two fields on the allocation object, and if they aren't there, or are None, it skips the creation | |
| 13:42:04 | efried | But how does it ultimately close the loop, then? | |
| 13:42:15 | edleafe | So my feeling is we should continue that behavior, even if it sucks | |
| 13:42:15 | efried | The allocation gets created with... no consumer? | |
| 13:42:25 | edleafe | efried: yep | |
| 13:42:49 | efried | So the consumer in that case is represented only by its UUID in the allocations table; it doesn't truly exist otherwise | |
| 13:42:55 | edleafe | TBC, no consumer *record* | |
| 13:43:10 | edleafe | it still has the instance UUID in the consumer_id field | |
| 13:43:27 | efried | ...of the allocations table | |
| 13:43:33 | edleafe | yes | |
| 13:44:46 | efried | so presumably if you GET an allocation record at 1.8 that was created at <1.8, the proj/user doesn't show up in the response. | |
| 13:44:54 | efried | or it shows up, but with null? | |
| 13:45:28 | edleafe | yeah, they are None >=1.8 | |
| 13:45:42 | efried | Seems to me like the right thing would be to make proj/user be nullable. | |
| 13:45:55 | efried | and always create the record, even <1.8 | |
| 13:46:09 | efried | ...which isn't a behavior change, because the API will still be doing exactly the same thing. | |
| 13:46:43 | edleafe | It's a teeny behavior change, but a real edge of an edge case | |
| 13:47:28 | efried | How would it be a behavior change? You're just changing the condition from "if the record doesn't exist, assume null/null" to "use what's in the record, which will always be there (oh, and btw, might be null)" | |
| 13:47:28 | edleafe | here's the case: | |
| 13:47:41 | mriedem | dansmith: i saw, but haven't dug into the changes yet | |
| 13:47:50 | dansmith | mriedem: okay just wanted to make sure you did | |
| 13:47:56 | edleafe | allocations are created <1.8. No consumer record in the past; now there is w/generation=0 | |
| 13:48:15 | efried | dansmith, mriedem: You'll be delighted to know that I checked our OOT driver, and you don't break it with that change. | |
| 13:48:33 | efried | ...because we use the tuple form already. | |
| 13:48:58 | efried | edleafe: Sorry, still listening. | |
| 13:49:01 | edleafe | np | |
| 13:49:19 | mriedem | efried: i don't know why your out of tree driver would care about that change | |
| 13:49:52 | edleafe | A later attempt to allocate that provides the generation may or may not succeed, depending on the gen. If there were no consumer record, though, it would always succeed | |
| 13:50:02 | edleafe | IMO, that's not a bad change, but it is a change | |
| 13:51:27 | openstack | Launchpad bug 1761197 in OpenStack Compute (nova) "Not defined keypairs in instance_extra cellsV1 DBs" [Undecided,New] - Assigned to Surya Seetharaman (tssurya) | |
| 13:51:27 | belmoreira | dansmith mriedem can you have a look into https://bugs.launchpad.net/nova/+bug/1761197 and give me your opinion? | |
| 13:51:31 | efried | mriedem: https://github.com/openstack/nova-powervm/blob/master/nova_powervm/virt/powervm/tasks/network.py#L183-L186 where _get_vif_events uses the tuple form: https://github.com/openstack/nova-powervm/blob/master/nova_powervm/virt/powervm/tasks/network.py#L226 | |
| 13:52:16 | mriedem | efried: that's how it's used in tree today too | |
| 13:52:18 | efried | edleafe: But the "later attempt to allocate that provides the generation" *should not* "always succeed" | |
| 13:52:33 | efried | mriedem: Right, I guess we've ported that code over already. | |
| 13:52:44 | kashyap | dansmith: Hi there, if you get time today, I too (like johnthetubaguy, who +2ed) am going blind on this: https://review.openstack.org/#/c/534384/ | |
| 13:52:57 | efried | edleafe: Besides, there's not a behavior change wrt generation handling cause we don't *have* generation handling yet. | |
| 13:53:03 | efried | edleafe: IMO this is all tasty goodness. | |
| 13:53:10 | edleafe | efried: sure, but if we preserve the behavior with allocations <1.8 (who even uses that anymore?), it will always succeed | |
| 13:53:19 | edleafe | As I said, an edge of an edge case | |
| 13:53:20 | mriedem | efried: i guess you're referring to how event_names in that interface can be a list of names, rather than tuples https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L447 | |
| 13:53:38 | efried | mriedem: right. Was allowable before dansmith's patch on your change. | |
| 13:54:07 | dansmith | belmoreira: hmm, are you saying that back when you ran the keypair migration they weren't sync'd down to the v1 cell databases and thus nothing happened? | |
| 13:55:26 | dansmith | efried: everyone copied the first implementation in libvirt I imagine, which was luckily done the more explicit way..I checked all the in-tree drivers | |
| 13:55:31 | efried | edleafe: Still not sure I follow. If you're using <1.8, you'll get generations incremented under the covers and you'll never know about it. As soon as you start using the new microversion (the one you're writing), you'll see the generation (whatever it is) when you GET, and you'll push it back when you PUT/POST. That's as it should be. But that behavior ought to be the same regardless of what we do with the proj/user | |
| 13:55:37 | efried | dansmith: nod | |
| 13:56:06 | efried | edleafe: The proj/user isn't required even at 1.8, is it? | |
| 13:56:33 | efried | hm, looks like the user_id isn't marked as optional. | |
| 13:56:59 | efried | sorry, neither is marked as optional. | |
| 13:57:36 | efried | but are they in fact required? (That would seem weird, tbh) | |
| 13:58:27 | efried | edleafe: But my point is that you would already have this weirdness if you straddled microversions like that. You would have to deal with the fact that proj/user were null before (whether it's because the record doesn't exist or because the record exists with nulls in it) and now are required (or whatever). | |
| 13:58:51 | edleafe | efried: they are required >=1.8 | |
| 13:59:09 | efried | edleafe: We're probably talking in circles around an issue that's insignificant if it even exists at all. | |
| 13:59:23 | efried | Is there any reason we can't make those fields nullable? | |
| 13:59:43 | purplerbot | <edleafe> It's a teeny behavior change, but a real edge of an edge case [2018-04-04 13:46:43.828800] [n 2tEe] | |
| 13:59:43 | edleafe | efried: [t 2tEe] | |
| 14:00:02 | edleafe | Like, if you're a purist... | |
| 14:00:06 | efried | and make the logic that returns them tolerate <record absent> and <record present with nulls> with the same result. | |
| 14:00:52 | edleafe | sure, nulls would have to work in all cases | |
| 14:01:13 | edleafe | ok, let me make that change to the consumers table and see how that works | |
| 14:02:05 | efried | ++ | |
| 14:08:57 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Cleanup RP and HM records while deleting a compute service. https://review.openstack.org/554920 | |
| 14:13:56 | openstackgerrit | Merged openstack/nova master: Teardown networking when rolling back live migration even if shared disk https://review.openstack.org/555481 | |
| 14:25:08 | belmoreira | dansmith: sorry was away... yes, the migration was only correctly done in top DB | |
| 14:25:46 | dansmith | belmoreira: but you mean the top api cell db, not the api_db, yes? | |
| 14:26:24 | belmoreira | yes, the top DB in cellsV1 | |
| 14:26:35 | dansmith | belmoreira: we just removed the code that did that migration, but even still, it would be out of the ordinary to make nova-manage reach across cellsv1 DBs | |
| 14:28:48 | belmoreira | dansmith: yes I saw that the code was removed. But when running nova-manage in cells DBs it should go to nova_api for the keypair | |