Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-01-10
10:29:41 mdbooth I reviewed the patch anyway, which had issues. I pointed out your series as a much more thorough alternative which was also previously nearly across the line.
10:36:30 mdbooth stephenfin: I answered for him :)
10:38:29 stephenfin mdbooth: Ta!
10:38:39 stephenfin That's done now, as promised
10:45:37 openstackgerrit sahid proposed openstack/nova master: hardware: only take into account small pages https://review.openstack.org/532168
10:50:24 lyarwood stephenfin / mdbooth ; yup thanks, context is still required for attach
11:07:41 openstackgerrit Jie Li proposed openstack/nova-specs master: Support volume-backed server rescue https://review.openstack.org/532410
11:11:01 openstackgerrit Liam Young proposed openstack/nova master: Add exception to no-upcall note of cells doc https://review.openstack.org/532491
11:14:20 sean-k-mooney did they upgrade gerrit recently? i just commented on a patch set 15 of someting and it included old draft comment i had on patchset 4 also...
11:15:32 sean-k-mooney granted it did at least include them on the patchset 4 version but still that annoying when you cant see that they are there
11:17:43 gibi sean-k-mooney: yeah, this is a new feature from the last gerrit upgrade
11:17:53 gibi sean-k-mooney: but the upgrade happened couple of months ago
11:30:01 sean-k-mooney gibi: oh really i guess i have just been lucky enough not to hit it till now. if i start a review and there are new revions in between i normally start again and copy the comments but dont always delete the old ones since they were ignored
11:30:29 openstackgerrit Jie Li proposed openstack/nova-specs master: Support volume-backed server rebuild https://review.openstack.org/532407
11:42:13 openstackgerrit Marcin Juszkiewicz proposed openstack/nova stable/pike: libvirt: use 'host-passthrough' as default on AArch64 https://review.openstack.org/532504
11:42:31 hrw as it went to master it would be good to have it in pike too ;D
11:44:34 openstackgerrit Jie Li proposed openstack/nova master: Support volume-backed server rescue https://review.openstack.org/531524
11:53:41 mdbooth lyarwood: Sorry :/ https://review.openstack.org/#/c/460243/
11:54:28 openstackgerrit Jie Li proposed openstack/nova master: Support volume-backed server rescue https://review.openstack.org/531524
12:24:54 Rambo Hi,everyone ,Can you help me to review the spec ?The link is https://review.openstack.org/#/c/532410/
12:25:03 Rambo Another is :https://review.openstack.org/#/c/532407/
12:25:09 Rambo Thank you very much
12:29:09 sean-k-mooney melwitt: stephenfin bauzas o/ do any of ye know the url to the ptg etherpad matt started. ill grab it from the irc logs if not so noworries if you dont have it to hand.
12:30:30 sean-k-mooney melwitt: stephenfin bauzas found it https://etherpad.openstack.org/p/nova-ptg-rocky
12:31:54 lyarwood mdbooth: np, fired back, I really don't like that comment with the encryptor.detach_volume call removed
12:32:15 lyarwood mdbooth: it should be pretty obvious that you can't disconnect a volume before detaching it from the guest tbh
12:32:30 lyarwood mdbooth: and the tests should catch anyone trying to do this
12:33:54 kashyap lyarwood: Sometimes explicit is better than implicit. What is obvious to you would be a far cry from it for a fresh pair of eyes trying to get up2speed.
12:34:20 kashyap I'm personally a fan of documenting in comments, even if sometimes it's a bit obvious if you're in the know
12:36:28 ygl hi all
12:36:35 ygl can someone help me with my issue
12:43:13 openstackgerrit sahid proposed openstack/nova master: hardware: only take into account small pages https://review.openstack.org/532168
12:51:59 lyarwood *sigh*
12:52:33 lyarwood kashyap: so how would the comment help someone reviewing that method for the first time?
12:52:49 lyarwood kashyap: given that the call to detach the encryptor is now hidden from them in _disconnect_volume
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

Earlier   Later