Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-01-10
09:24:29 ameeda gmann: I am not sure. I still beginner with nova and openstack, so you guess that I need to restore the original code ?
09:25:14 gmann ameeda: np!. let's not change that and see whether bug is fixed or not. you can check locally also and by adding functional tests also
09:26:03 gmann ameeda: if something compute/api.py change needs to do we can do later. but i am sure changing DB field and util function should work
09:27:42 ameeda gmann: what about functional tests. its important ? if so, how I can do that ?
09:31:37 gmann ameeda: yea it is imp to see bug is fixed and it does not regress . example is like these tests - https://github.com/openstack/nova/tree/master/nova/tests/functional/regressions
09:32:01 ameeda gmann: thanks for your help and your time.
09:32:09 gmann ameeda: with that we can get to know whether bug is actually fixed and no more hidden restriction/truncation etc
09:32:16 gmann ameeda: np!
09:35:47 openstackgerrit sahid proposed openstack/nova master: libvirt: slow live-migration to ensure network is ready https://review.openstack.org/497457
09:53:10 openstackgerrit caishan proposed openstack/nova master: Unit testing test_driver.py indent issue https://review.openstack.org/532473
09:57:07 openstackgerrit caishan proposed openstack/nova master: Unit testing test_driver.py indent issue https://review.openstack.org/532473
10:07:18 openstackgerrit Stephen Finucane proposed openstack/nova master: Fix typo in release note https://review.openstack.org/531854
10:24:45 mdbooth lyarwood: Could you take a look at https://review.openstack.org/#/c/531524/ ? I'd like to encourage the author to resurrect your stable rescue series instead.
10:26:55 lyarwood mdbooth: sure, there's a spec up for review for this, might provide the feedback there - https://review.openstack.org/#/c/532410/3/specs/rocky/approved/volume-backed-server-rescue.rst
10:27:39 mdbooth Hmm. I didn't see that linked from the bp.
10:28:13 lyarwood mdbooth: it isn't, it's on the gerrit topic.
10:28:28 mdbooth Eurgh.
10:29:00 stephenfin lyarwood: Small question here https://review.openstack.org/#/c/460243/13/nova/virt/libvirt/driver.py@1273
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()

Earlier   Later