Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-01-10
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
15:21:12 mriedem i wonder if he's on baremetal
15:21:28 mriedem because there are cases where the driver.get_inventory call for ironic will return an empty dict which is an indication to delete the inventory for the provider
15:21:38 mriedem klindgren_: ^
15:21:51 mriedem were you seeing those inventory calls to placement on libvirt or ironic computes?
15:27:28 openstackgerrit Andrey Volkov proposed openstack/nova master: [placement] Fix resource provider delete https://review.openstack.org/529519
15:27:34 cdent sigh, took me forever to find http://lists.openstack.org/pipermail/openstack-dev/2017-January/110953.html
15:27:35 mriedem oh i see,
15:27:38 mriedem _update_inventory_attempt is called in a loop
15:27:42 cdent not yet clear if it will be any use
15:28:06 mriedem so if we get a 409 trying to update inventory we try again
15:28:12 mriedem that's why there are multiple GETs for inventory
15:28:30 sean-k-mooney efried: syntaks for traits to resouces. there was discusstion of a resouce_1=<class x>,required1=<trait y>,<trait z> query arg syntaks for get allocation candiates
15:28:40 cdent mriedem: that may not be right, have a look at point B1 in the list posting above
15:29:29 openstackgerrit Balazs Gibizer proposed openstack/nova master: Deduplicate service status notification samples https://review.openstack.org/531381
15:29:39 cdent (it maybe also be right, though, but without logs, hard to say)
15:29:53 sean-k-mooney efried: i dont think we have a similar fuctionality for the resouce providres api however.
15:31:17 mriedem cdent: yeah, you might be right, because if we hit an inventory conflict, we delete the rp_uuid from the cache and then get the resource provider again to update the generation, but i don't see a GET to just /resource_providers in klindgren_'s output
15:31:51 mriedem so likely hitting something that's not a 409, but would need logs

Earlier   Later