| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-10 | |||
| 14:49:00 | cdent | sean-k-mooney: yeah, it's the sort of thing where you should keep _me_ in drinks in dublin to not raise again | |
| 14:49:33 | sean-k-mooney | efried: actully you could have several per compute node. e.g. sriov + ovs on the same node though in practice yes | |
| 14:50:06 | efried | sean-k-mooney But not one agent for multiple computes. | |
| 14:50:40 | efried | sean-k-mooney So my point is, you can ask for providers (having CUSTOM_MANAGED_BY_NEUTRON) && (in tree <compute RP UUID>) | |
| 14:50:50 | sean-k-mooney | efried: :( well for agent based neutron no. but this is odl.... | |
| 14:50:59 | sean-k-mooney | * there is | |
| 14:51:29 | efried | cdent I thought GET /resource_providers had a queryparam for "having traits". I don't see it at a glance. | |
| 14:52:38 | cdent | i'm not sure that got merged (yet) | |
| 14:52:46 | cdent | I do think code to do it somewhere though | |
| 14:52:55 | efried | cdent Oh, okay, in flight | |
| 14:53:26 | efried | It would be somewhere in that fabulous placement update email summary... | |
| 14:54:08 | mriedem | alex_xu: i'm still +2 on https://review.openstack.org/#/c/330406/ - i think the migration_links thing is correct; it's consistent with other APIs that support paging. you have a good point about the changes-since before 2.59 though, but that could be addressed in a follow up. | |
| 14:54:18 | mriedem | since it assumes people would actually do that | |
| 14:54:34 | sean-k-mooney | efried: worst comes worse neuron can jsut get teh whole tree and walk it to see if the inventries it creates exist or not. | |
| 14:54:51 | cdent | efried: I think it is something that alex_xu was working on but a lot of his trait related stuff got abandoned | |
| 14:55:31 | efried | cdent Yah, I can't find any such thing in open state (assuming it would have 'trait' somewhere in the title/description) | |
| 14:56:30 | efried | mgoddard So at a glance, it looks like what you've done here is modeled after the inventory updating stuff. | |
| 14:56:55 | mgoddard | efried: Correct | |
| 14:57:00 | cdent | efriend I suspect that quite a few query style things, on /resource_providers, got dropped when /allocation_candidates took the focus, especially if the use cases on /resource_providers werent yet fully formed | |
| 14:57:42 | efried | mgoddard I haven't fully synthesized this stance yet, but I *think* I'm going to come to the conclusion that that's unnecessarily complicated (even for inventory) - and even incorrect in that it does retries at this low level rather than at the consumer level. | |
| 14:59:31 | efried | cdent GET /resource_providers?having_all=T1,T2 and/or ?having_any=T3,T4 seems like a fairly natural thing to expect, but of course there needs to be a real use case for it. What sean-k-mooney described could count as such. | |
| 14:59:52 | sean-k-mooney | efried: one of the other issue is that nova uses the nova compute node uuid of the host_id(hostname by default) which is sotre in the name filed of the compute node RP i think so we cant uses in_tree in this case and need to use name | |
| 15:00:23 | efried | I believe in_tree accepts name or UUID, doesn't it? | |
| 15:00:37 | efried | no, never mind. | |
| 15:01:03 | sean-k-mooney | got to run to a meeting but efried did you not have a systax for this discribed also in your generic device management proposal. | |
| 15:01:30 | sean-k-mooney | be back in 30 mins | |
| 15:02:00 | efried | sean-k-mooney Syntax for what? And I doubt it, I don't recall getting to a 'syntax' level of detail in the generic device management discussions. | |
| 15:03:04 | mriedem | alex_xu: makes me wonder if we've added other query strings in higher microversions to apis that allowed additionalProperties before :) | |
| 15:04:16 | efried | mgoddard I think we got away with retries at the report client level for inventory because at the time inventory was the only thing that could affect generation, AND we were guaranteed to be the only thing messing with that provider, AND there were no trees or sharing providers. dansmith cdent and Jay should check me on this, but I think we're going to want to pull those retries outta there (at least for 409s) and subsume t | |
| 15:04:17 | efried | hem in the wholesale retries from the resource tracker level. | |
| 15:04:48 | cdent | that's probably right and aligns with what was said monday | |
| 15:05:20 | kashyap | mriedem: A heads-up: Given your Nova commit 8075797, https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg00845.html -- [PATCH 0/2] qemu-img: Let "info" warn and go ahead without -U ['--force-share'] | |
| 15:05:52 | kashyap | I (& DanPB too) pointed out that Nova already added support to it | |
| 15:06:14 | kashyap | Where the QEMU folks were asking if Nova / other management tools use it -- https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg01816.html | |
| 15:09:27 | mriedem | kashyap: so they are talking about deprecating and removing the locking thing because everyone is just bypassing it to get their code working again? | |
| 15:10:15 | alex_xu | efried: cdent anything I can help on trait? | |
| 15:10:18 | mriedem | i think nova hits qemu-info from a lot of places | |
| 15:10:30 | kashyap | mriedem: The discussion is still in flux. I don't think they're going to _remove_ it. | |
| 15:10:37 | mriedem | so auditing when we can just ignore it and bypass the lock would be difficult | |
| 15:10:47 | kashyap | The aim of the locking change is to not let users shoot themselves in the foot | |
| 15:10:55 | mriedem | yeah i realize | |
| 15:10:56 | kashyap | But that WILl cause some inconvenience, in terms of usage behaviour | |
| 15:11:07 | kashyap | Trying to get a sense of what is the behaviour across versions | |
| 15:11:07 | cdent | alex_xu: I don't think we need to do anything immediately but we were discussing needing to be able to get a list of resource providers that have a particular trait | |
| 15:11:08 | alex_xu | mriedem: so...after that patch merge, we have a window the order version API is broken | |
| 15:11:19 | mriedem | kashyap: so for the shareable disk thing in libvirt 3.10, does that just bypass the lock in qemu 2.10? | |
| 15:11:29 | kashyap | mriedem: Also, I was just adding a TODO item is that, we should investigate using the run-time command 'query-block' | |
| 15:11:33 | mriedem | kashyap: or is it telling qemu, 'this is intentionally a shared thing, so be cool with it'? | |
| 15:11:39 | cdent | it occurs to me now, after thinking about it a bit, that we can probably use the 'resources' param for that, and pass in the one single trait we care about ( <- efried ) | |
| 15:11:45 | kashyap | Instead of 'qemu-img' in a loop every few seconds; as 'query-block' will give more consistent results | |
| 15:11:48 | kashyap | mriedem: Yep | |
| 15:12:00 | mriedem | alex_xu: i wouldn't say the older API version is broken | |
| 15:12:21 | mriedem | alex_xu: it's assuming someone actually passes changes_since, something that wasn't supported before the new microversion | |
| 15:12:26 | efried | cdent How do you specify traits to ?resources ? | |
| 15:12:31 | kashyap | mriedem: So the upcoming behaviour (not set in stone) is that: *even* if you _don't_ specify '--force-share', it'll go ahead with the run, but will print a warning, so as to prime your brain | |
| 15:12:45 | mriedem | kashyap: we won't see those warnings most likely | |
| 15:12:45 | efried | cdent I should know that answer, shouldn't I | |
| 15:12:47 | kashyap | mriedem: Just noticed your other question about shareable thing | |
| 15:12:48 | alex_xu | mriedem: yes.... | |
| 15:12:55 | cdent | efried: i'm not certain, and i'm not certain we do, yet | |
| 15:13:01 | efried | cdent It's in flight, yeah. | |
| 15:13:02 | kashyap | mriedem: Did you see Peter's comment here, to your question: https://bugzilla.redhat.com/show_bug.cgi?id=1378242#c21 | |
| 15:13:03 | openstack | bugzilla.redhat.com bug 1378242 in libvirt "QEMU image file locking (libvirt)" [Unspecified,On_qa] - Assigned to pkrempa | |
| 15:13:04 | cdent | was generalizing that that's how it _should_ work | |
| 15:13:16 | mriedem | alex_xu: if you're really concerned about it, i can make the quick change to check for it if version<2.59 and just pop it off the req.GET | |
| 15:13:22 | cdent | and it should work for both /rp and /ac | |
| 15:13:26 | kashyap | mriedem: Yep, we won't see, because Nova already baked in (correctly so) the '--force-share' with your commit | |
| 15:13:36 | mriedem | alex_xu: i'm just trying to get done as much as i can before i'm out next week | |
| 15:13:54 | mriedem | kashyap: i meant nova won't see b/c it would be in the qemu/libvirtd logs, | |
| 15:13:58 | mriedem | and we don't look there unless it's an error | |
| 15:14:00 | efried | cdent Ah, that's it, actually the code I have up is only applying it to /ac. https://review.openstack.org/#/c/517757/1 | |
| 15:14:06 | kashyap | Ah, like that. | |
| 15:14:32 | cdent | efried: thus my comment on line 23 on https://etherpad.openstack.org/p/nova-ptg-rocky | |
| 15:14:52 | alex_xu | mriedem: got it, you can have my promise to review that patch again tomorrow | |
| 15:15:02 | mriedem | alex_xu: ok i'll update it today then | |
| 15:15:06 | alex_xu | mriedem: thanks | |
| 15:15:06 | mriedem | thanks for the solid review as always | |
| 15:15:56 | simondodsley | Any idea when it became a valid option to add to the ```live_migration_flags``` parameter in ```nova.conf``` and since this parameter was deprecated in Mitaka does Nova now automatically use ‘unsafe’ or is there something else that needs to be set to force the ‘unsafe’ switch? | |
| 15:15:56 | simondodsley | I can see that this switch was added in libvirt 0.9.11 back in 2012, but I’m struggling in finding references to it or VIR_MIGRATE_UNSAFE as valid options in Kilo or later releases of OpenStack (other than just comments) | |
| 15:15:56 | simondodsley | What I’m trying to find out is when and if Nova supported/supports the use of the ```virsh –-unsafe``` switch when ```cachemode != none```? | |
| 15:15:56 | simondodsley | Hi - hope I'm on the correct channel to ask these questions... | |
| 15:16:15 | mriedem | cdent: efried: dansmith: klindgren_ pinged me last night about the number of REST calls from the compute to placement during the update_available_resource periodic task which runs by default every minute, | |
| 15:16:25 | simondodsley | sorry about the format there :) | |
| 15:16:25 | alex_xu | cdent: efried, for the trait, the only left thing is expose 'required' parameter intthe 'GET /allocation_candidates' API | |
| 15:16:28 | mriedem | from his pike deployment it's 5 calls https://paste.ubuntu.com/26356656/ | |
| 15:16:32 | mriedem | at least | |
| 15:16:33 | mriedem | per compute | |
| 15:16:43 | mriedem | cdent: efried: dansmith: the thing i noted was the 2 calls for aggregates, | |
| 15:16:59 | mriedem | which if you look at the code, the provider aggregate map is there in the report client but not used, | |
| 15:16:59 | cdent | mriedem: yes, you remember that post i made mid year about such things ? | |
| 15:17:04 | mriedem | b/c we don't support shared providers yet | |
| 15:17:11 | mriedem | cdent: not the detalis no | |
| 15:17:24 | mriedem | cdent: can you summarize? | |
| 15:17:27 | mriedem | we might be on the same page | |
| 15:17:37 | dansmith | two hits to inventories? | |
| 15:17:51 | mriedem | dansmith: i wondered about that too | |
| 15:18:05 | mriedem | for the aggregates ones, i told him the obvious thing to do is just comment out that code as it's totally unused | |
| 15:18:16 | cdent | it was also five, iirc, and I was able to do some tricks to trim it but they were deemed risky. agree that one way to cut is to reduce is not make the agg map | |
| 15:18:34 | cdent | let me find the message, because I think it had something to say about the double inventory | |
| 15:19:57 | mriedem | _get_inventory is only called by _get_inventory_and_update_provider_generation which is only called to check if we need to update inventory (if things changed), or delete inventory | |