| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-10 | |||
| 12:53:18 | lyarwood | kashyap: I'm all for helping first time readers through code but it makes the entire thing more confusing IMHO | |
| 12:55:18 | kashyap | lyarwood: Hmm, if you think it'll confuse more, I'll defer to you. | |
| 12:55:35 | kashyap | lyarwood: I noticed what you said is missing in your review comment | |
| 12:56:35 | kashyap | lyarwood: Then maybe you'd want to note that the call to detach the encryptor is elsewhere :-) | |
| 13:01:43 | takashin | alex_xu: Are you aroud? | |
| 13:02:06 | alex_xu | takashin: yea | |
| 13:02:23 | takashin | alex_xu: Is there API meeting today? | |
| 13:03:16 | alex_xu | takashin: yes, but passed few weeks, there is no people show up, then I didn't run it, is there anything you want to discuss, we can discussed at here I think | |
| 13:03:55 | takashin | alex_xu: Okay. I have 2 patches for reviews. | |
| 13:04:07 | takashin | api-ref: Parameter verification for servers.inc: https://review.openstack.org/#/c/528201/ | |
| 13:04:13 | gmann | alex_xu: takashin i was away too since 2-3 weeks. we can resume from next week may be | |
| 13:04:16 | takashin | api-ref: Example verification for servers.inc: https://review.openstack.org/#/c/529520/ | |
| 13:04:38 | takashin | gmann: thanks. | |
| 13:05:00 | takashin | alex_xu: gmann: Would you review the patches? | |
| 13:05:03 | alex_xu | gmann: cool, we will run it | |
| 13:05:20 | alex_xu | takashin: I add them to my review list, will try to reach them | |
| 13:05:40 | gmann | takashin: ywa, i remember to review those half way last week, ll do tomorrow for sure | |
| 13:05:55 | takashin | alex_xu: gmann: Thank you. | |
| 13:06:00 | takashin | That's all. | |
| 13:06:33 | alex_xu | takashin: did you see my comment https://review.openstack.org/#/c/459483/, I think that is thing we should keep consistent, and I think that isn't worth another microversion | |
| 13:07:22 | takashin | alex_xu: I saw your comment. I will fix it tomorrow. | |
| 13:07:34 | alex_xu | takashin: thanks | |
| 13:08:17 | alex_xu | takashin: gmann btw, there is API patch closed to merge https://review.openstack.org/#/c/330406, I'm reviewing it, but still still found something, it will be great you guys can help review it also, the API patch is really huge :) | |
| 13:08:18 | mdbooth | lyarwood: Anyway, like I said the patch is a huge improvement, I don't see any issues in the code. It's simpler and it fixes at least 3 bugs. I'm just cautious about removing context from a driver which is already plenty opaque in places. | |
| 13:09:00 | gmann | alex_xu: sure, added in my tomorrow list | |
| 13:09:24 | takashin | alex_xu: okay. I will review it tomorrow. | |
| 13:10:33 | alex_xu | gmann: takashin thanks! | |
| 13:14:40 | kashyap | mdbooth: Do you recall top off your head, in what scenarios Nova calls 'qemu-img info' for _running_ guests? | |
| 13:14:50 | kashyap | If not, don't worry, I'll go look into code | |
| 13:15:30 | mdbooth | kashyap: Not off the top of my head, but pretty sure there are some. | |
| 13:16:02 | mdbooth | Look at live migration. Maybe imagecache reaper. | |
| 13:19:47 | kashyap | mdbooth: Okido, I'm in a discussion w/ the QEMU Block folks, and they're asking this. | |
| 13:19:54 | efried | mgoddard Where's set_traits_for_provider (https://review.openstack.org/#/c/532290/1/nova/compute/resource_tracker.py@890) defined? I can't find it in master or in your series. | |
| 13:19:54 | kashyap | I'm sure we do, just have to audit | |
| 13:20:09 | efried | mgoddard I ask because I'm actually in the process of implementing that method right now. | |
| 13:20:21 | efried | mgoddard Want to avoid duplication of effort if possible. | |
| 13:31:43 | mgoddard | efried: hi. I'm still implementing that one. I can submit what I have for review if you'd like to see it | |
| 13:32:25 | efried | mgoddard I would, yes. I'll show you mine if you show me yours :) | |
| 13:33:16 | mgoddard | efried: well I don't usually do this, but go on then | |
| 13:34:06 | efried | mgoddard You're probably a little further along BUT one of the key things I'm doing there is exposing a new exception base class for placement API conflicts and raising subclasses thereof from this method and its brethren (e.g. set_aggregates_for_provider, tbd) when they encounter 409s. | |
| 13:34:52 | efried | mgoddard It's in the middle of a rather messy restack, won't be ready to show for a little while yet. | |
| 13:35:22 | efried | mgoddard But now that I know you're also wanting it for the ironic traits bp, seems it needs to be peeled out of that series. | |
| 13:36:21 | efried | which is probably not super hard. Though at the moment it's based on a change that raises a conflict exception for RP creation, which I think *is* tied pretty heavily into that series. | |
| 13:44:24 | kashyap | mdbooth: Just noting for the record, looked for the past few minutes: | |
| 13:44:26 | kashyap | _rebase_with_qemu_img() , _live_snapshot() and _get_instance_disk_info_from_config() [from nova/virt/libvirt/driver.py] | |
| 13:44:42 | kashyap | nova/virt/libvirt/imagebackend.py: | |
| 13:44:59 | kashyap | - cache() --> fetch_func_sync() --> get_disk_size() --> qemu_img_info() | |
| 13:45:04 | sean-k-mooney | efried: QQ is there a top level api for placement aggregates? e.g. can i list all aggregates or list all resocue providers in an aggregate given the aggregate uuid? | |
| 13:45:21 | efried | sean-k-mooney sec... | |
| 13:46:04 | efried | sean-k-mooney Hum, I thought there was, cause I'm gonna need it. Still looking... | |
| 13:46:46 | sean-k-mooney | efried: no rush. you can get teh aggregates a resouce provider is part of but at least looking at the master docs the recprical api does not appear to exist | |
| 13:46:56 | cdent | you can use member_of to get all rps in a given aggregate | |
| 13:47:07 | efried | Ahh, that's it, thanks cdent | |
| 13:47:17 | cdent | https://developer.openstack.org/api-ref/placement/#list-resource-providers | |
| 13:47:57 | cdent | It's not clear how you're supposed to discovery an aggregate, though, other than by looking at https://developer.openstack.org/api-ref/placement/#list-resource-provider-aggregates | |
| 13:48:06 | cdent | or knowing the uuid prior | |
| 13:48:12 | mdbooth | kashyap: I'm not convinced cache() would be called on a running instance. As always with that code, though, it's far from obvious without checking carefully. | |
| 13:48:28 | mdbooth | kashyap: cache() mostly means 'create' | |
| 13:48:29 | kashyap | mdbooth: Yeah, I should've been careful in pointing that out | |
| 13:48:37 | efried | cdent Yeah. That said, is there a use case for that? | |
| 13:48:50 | sean-k-mooney | cdent: ah yes cool. that still leaves me with one question. how do i create the aggreate in the first place with out a top level aggregates api? | |
| 13:49:18 | cdent | efried: not that I'm aware, but just as I was thinking of it I stumbled on "How do I know the aggregates" | |
| 13:49:27 | cdent | sean-k-mooney: it gets created when you use it | |
| 13:49:40 | kashyap | mdbooth: I didn't do a thorough audit, though. Taking notes as I find instances of it & then see where they're called on a running guest | |
| 13:49:47 | kashyap | For live snapshot we do for sure | |
| 13:50:00 | cdent | so if you PUT to /resource_provider/{uuid}/aggregates with a new uuid there ya go | |
| 13:50:01 | efried | sean-k-mooney You "create" it by assigning it to a provider via PUT /rp/{uuid}/aggs | |
| 13:50:02 | sean-k-mooney | cdent: so the first time i add a resouce provider to an agregate it creates the uuid | |
| 13:50:04 | efried | yeah, what he said. | |
| 13:50:12 | mdbooth | kashyap: Did you look in the imagecache periodic task? | |
| 13:50:12 | cdent | yes | |
| 13:50:43 | kashyap | mdbooth: Not yet; so far I'm just noting down instances where it's called. E.g: | |
| 13:50:46 | kashyap | nova/virt/libvirt/imagebackend.py | |
| 13:50:49 | kashyap | - cache() --> fetch_func_sync() | |
| 13:50:51 | kashyap | --> get_disk_size() [from nova/virt/libvirt/driver.py] | |
| 13:50:54 | kashyap | --> qemu_img_info() [from nova/virt/images.py] | |
| 13:50:56 | kashyap | - verify_base_size() --> get_disk_size() | |
| 13:50:59 | kashyap | - class LVM() --> create_image() --> create_lvm_image() --> get_disk_size() --> qemu_img_info() | |
| 13:51:00 | sean-k-mooney | cdent: so the problem of determining an un used uuid for the aggregate is left to the client | |
| 13:51:09 | cdent | yes | |
| 13:51:12 | kashyap | (And then see the calls for the live guest.) | |
| 13:51:12 | efried | sean-k-mooney I knew I wrote that code: https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L431 | |
| 13:51:44 | sean-k-mooney | cdent: ok cool. more reading for me to do :) | |
| 13:51:48 | cdent | sean-k-mooney: there's some stuff in the very early rp specs about such things | |
| 13:51:49 | efried | sean-k-mooney There shouldn't be any such thing as an "unused" aggregate UUID. Unless you mean "used only for one provider", which is pointless. | |
| 13:52:14 | cdent | but if I remember right, the thinking was that the client would already have some identifier in min | |
| 13:52:15 | cdent | d | |
| 13:52:30 | efried | ...because when you remove the last association, placement ought to get rid of that agg ID. | |
| 13:53:02 | efried | cdent Yeah, that's a weird one. In the PowerVM SSP case, the only thing that makes sense is to give the agg the same UUID as the shared storage pool | |
| 13:53:22 | efried | Though I *suppose* I could give the agg the UUID of the cluster instead. It's effectively the same thing in powervm land. | |
| 13:53:43 | sean-k-mooney | efried: well what i mean is if i am createing a set of recouse providers and i want to make them part of a aggregate to group them i need to choose a uuid that is not useed by anyone else. | |
| 13:54:30 | cdent | placement does not clean up the unused agg uuids. that was an early design decision, sort of resulting from how the tables were being normalized to not use uuid keys (and a few others things, it's so long ago) | |
| 13:54:32 | efried | sean-k-mooney Yes, which one would normally do by generating one randomly, BUT that breaks down quickly if you have more than one point of control for that agg. | |
| 13:54:45 | efried | cdent Whoah, so how do they get cleaned up? | |
| 13:54:58 | cdent | they don't | |
| 13:55:02 | cdent | at least not last I checked | |
| 13:55:03 | efried | They just... leak? | |
| 13:55:04 | mgoddard | efried: I have some concerns with the storage of the generation in the provider tree | |
| 13:55:06 | mgoddard | efried: there is a single generation per-provider that covers inventory, traits, and aggregates | |
| 13:55:10 | mgoddard | efried: but if we call e.g. pt.update_inventory() with a new generation, the traits or aggregates for that provider may not necessarily correspond to the generation that gets set | |