| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-24 | |||
| 19:04:07 | cdent | presumably there could be a situation where a compute node isn’t involved? | |
| 19:04:07 | dansmith | well, I haven't really needed to refer to anything other than a compute node | |
| 19:04:07 | edleafe | "radicle" is similar to root | |
| 19:04:22 | edleafe | That'll get the dictionaries dusted off | |
| 19:04:25 | dansmith | I guess I'm missing the need for a name | |
| 19:04:25 | efried | edleafe Far out. | |
| 19:05:00 | efried | dansmith Generically in placement, it's not always a compute node. Or at least, we want to code up placement so that's the case. At least that's what I thought the vision was. | |
| 19:05:22 | dansmith | sure, I'm just saying I don't think I've had a need to refer to this distinction | |
| 19:05:34 | dansmith | like, I'd call it "RP" and "Sharing RP" I guess | |
| 19:05:42 | dansmith | and prefix it with "compute node" when I'm talking about that | |
| 19:05:44 | dansmith | because, you know, | |
| 19:05:54 | dansmith | it's possible that a compute node would be a sharing one :) | |
| 19:07:18 | cdent | dansmith: if you want some context, patchset 6 on https://review.openstack.org/#/c/480379/ | |
| 19:09:12 | cdent | see yas | |
| 19:09:20 | dansmith | the three sentences you wrote on that PS? | |
| 19:09:22 | dansmith | oh well | |
| 19:11:00 | efried | dansmith Not the only time it has come up, though. I just hit it again: https://review.openstack.org/#/c/514199/2/nova/objects/resource_provider.py@2459 | |
| 19:11:29 | dansmith | right there it calls it a sharing provider, right? | |
| 19:12:11 | dansmith | or you're complaining about "root providers" I guess | |
| 19:12:40 | dansmith | how about "selfish providers" :P | |
| 19:12:59 | edleafe | dansmith: I already suggested that :) | |
| 19:13:10 | dansmith | they're all providers, some are sharing, so it seems like that's the distinction | |
| 19:13:22 | dansmith | edleafe: oh sorry | |
| 19:14:41 | edleafe | heehee | |
| 19:15:44 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM Driver: config drive https://review.openstack.org/409404 | |
| 19:17:19 | efried | edleafe dansmith https://doodle.com/poll/9fkqidtfzwh7y9tr | |
| 19:18:32 | efried | Added "selfish". | |
| 19:18:42 | edleafe | voted for "selfish" | |
| 19:18:48 | edleafe | 'cause I'm selfish | |
| 19:20:00 | dansmith | why is "compute node" even in there? | |
| 19:20:57 | efried | dansmith You seemed attached to it :) | |
| 19:21:19 | dansmith | huh? I said that in my referring to it, I was always referring to an actual compute node and thus had no need for another term | |
| 19:21:32 | dansmith | obviously we wouldn't want to call it a compute node in placementy code | |
| 19:21:57 | efried | Okay, I'm good with that. It wasn't clear from your discourse above. I'll remove 'root' too. | |
| 19:22:22 | dansmith | efried: [12:04:06] <dansmith>well, I haven't really needed to refer to anything other than a compute node | |
| 19:22:29 | edleafe | You don't have to remove them. | |
| 19:22:37 | efried | too late | |
| 19:22:38 | edleafe | Just don't vote for them | |
| 19:22:59 | dansmith | we're not going to call it compute node, so unless edleafe just wants to trap flies, it might as well not be there :) | |
| 19:23:11 | edleafe | if you put "doughnut" in there I wouldn't mind, I just wouldn't vote for it | |
| 19:23:14 | efried | I was trying to figure out a way to settle on something without bikeshedding | |
| 19:23:22 | efried | Now we're bikeshedding about the poll itself. | |
| 19:23:26 | efried | Meta-bikeshedding. | |
| 19:39:51 | mriedem | dansmith: ok https://review.openstack.org/#/c/506420/ | |
| 19:42:43 | dansmith | mriedem: so, let me tell you why some of that is the way it is | |
| 19:42:52 | dansmith | you're going to tell me to clean it up | |
| 19:43:11 | dansmith | but, it's because in the next patch I move this out of the actual task and share it from live migration | |
| 19:43:24 | dansmith | any time I change something here it causes the next patch to conflict out like a mofo | |
| 19:43:34 | dansmith | so I've just been trying to avoid a big refactor | |
| 19:43:45 | dansmith | but I guess that's just not gonna fly | |
| 19:44:28 | mriedem | i hadn't powered ahead | |
| 19:44:33 | mriedem | i gave it my all on this one | |
| 19:44:44 | dansmith | yeah, I know | |
| 19:44:54 | dansmith | I'm just really burned out on this | |
| 19:45:32 | mriedem | couldn't we do a patch in between that handles the refactor to move it into a utils? | |
| 19:45:56 | mriedem | that's what i did when i moved the force=true claim in conductor stuff to scheduler_utils so evacuate and live migration could both use that | |
| 19:46:12 | dansmith | I should just squash out the refactor into this one since it adds stuff and then just removes it | |
| 19:46:33 | dansmith | I don't think there's any reason not to have it in one of the tasks, it just has to be accessible | |
| 19:46:51 | mriedem | i put that other thing here https://github.com/openstack/nova/blob/91f436d6c095573594f660438fe2c72e721d97ce/nova/scheduler/utils.py#L222 | |
| 19:46:57 | mriedem | but sure, it could live in the migrate task | |
| 19:47:14 | mriedem | we kind of end up with this allocationy utility stuff everywhere though | |
| 19:47:57 | dansmith | yeah but unless they're going to actually be shared, putting it in utils just makes that a big bundle of indirected methods | |
| 19:48:01 | dansmith | which doesn't help, IMHO | |
| 19:48:32 | mriedem | i'm not really following what you mean | |
| 19:48:38 | mriedem | either way is fine | |
| 19:48:52 | dansmith | I'd rather it be closer to the user, i.e. in one of the tasks | |
| 19:49:01 | mriedem | but yeah it seems the base patch should then have the utility code | |
| 19:49:38 | mriedem | either way i'd rather not deal with reviewing both the refactor and the live migration task using it in the same change, | |
| 19:49:41 | mriedem | we could split that | |
| 19:49:52 | dansmith | I'm squashing it into the first patch, | |
| 19:49:56 | mriedem | ok | |
| 19:49:57 | dansmith | so it just adds the utility methods and uses them | |
| 19:50:17 | dansmith | and either I'll push it up that way and try to apply your concerns to the refactored code, | |
| 19:50:27 | dansmith | or you won't have heard from me by friday and should pick this up | |
| 19:50:32 | dansmith | and give my cellsv2 talk at SYD | |
| 19:50:50 | mriedem | i'm happy to pick up addressing my comments on this, | |
| 19:50:56 | mriedem | none of them are hard to handle | |
| 19:50:59 | dansmith | no | |
| 19:51:02 | dansmith | I'll do it | |
| 19:51:21 | mriedem | i mean after you rev | |
| 19:51:33 | mriedem | if there is something missed i'll just do it myself, or on top | |
| 19:51:44 | dansmith | okay | |
| 20:02:11 | openstackgerrit | Hongbin Lu proposed openstack/nova master: placement: add API reference for create inventory https://review.openstack.org/511342 | |
| 20:22:56 | mriedem | this trap has been laid since 2014 https://bugs.launchpad.net/nova/+bug/1726871 | |
| 20:22:58 | openstack | Launchpad bug 1726871 in OpenStack Compute (nova) "AttributeError: 'BlockDeviceMapping' object has no attribute 'uuid'" [High,Triaged] - Assigned to Matt Riedemann (mriedem) | |
| 20:48:31 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix AttributeError in BlockDeviceMapping.obj_load_attr https://review.openstack.org/514825 | |
| 20:49:41 | mriedem | dansmith: melwitt: ^ easy fix for a regression i introduced in pike | |
| 20:50:54 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix AttributeError in BlockDeviceMapping.obj_load_attr https://review.openstack.org/514825 | |
| 20:53:54 | openstackgerrit | Dan Smith proposed openstack/nova master: Make migration uuid hold allocations for migrating instances https://review.openstack.org/506420 | |
| 20:53:55 | openstackgerrit | Dan Smith proposed openstack/nova master: Make live migration hold resources with a migration allocation https://review.openstack.org/507638 | |
| 20:54:06 | dansmith | mriedem: seams were cleaner than I thought ^ | |
| 20:57:23 | melwitt | mriedem, dansmith: as a general rule, if we do obj_reset_changes on an ObjectField, we should use recursive=True? | |
| 20:57:36 | melwitt | just trying to understand that bit | |
| 20:58:09 | mriedem | melwitt: not sure honestly, because i found a similar problem in the instance object yesterday | |
| 20:58:48 | mriedem | this https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L563 | |
| 20:59:06 | mriedem | after you create an instance object, ec2_ids is still marked as changd | |
| 20:59:09 | mriedem | *changed | |
| 20:59:26 | mriedem | however, a later call to instance.save() won't care https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L650 | |
| 21:00:23 | melwitt | yeah, I'm guessing the "what_changed" logic recurses, checking | |
| 21:00:24 | mriedem | melwitt: in my bdm patch, if you remove recursive=True and replace https://review.openstack.org/#/c/514825/2/nova/tests/unit/objects/test_block_device.py@391 with bdm.save(), you'll see the failure | |