| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-10 | |||
| 08:37:09 | gmann | need to check whether it fix the reported bug or not | |
| 08:52:48 | gmann | ameeda: left comment there | |
| 08:55:30 | openstackgerrit | Jie Li proposed openstack/nova master: Support volume-backed server rescue https://review.openstack.org/531524 | |
| 08:59:25 | bauzas | morning Novaers | |
| 09:01:15 | gmann | morning | |
| 09:01:22 | ameeda | gmann: Thank you for review, so I need to return the master code, this restrict the length of the value, I removed the limitations for that ? I will change the reno. also add a functional regression test. | |
| 09:03:56 | gmann | ameeda: you mean this? - https://review.openstack.org/#/c/526900/17/nova/compute/api.py | |
| 09:04:14 | ameeda | gmann: yes | |
| 09:04:35 | gmann | ameeda: i checked and it should not have impact but let me confirm again. | |
| 09:05:22 | ameeda | gmann: thanks :) | |
| 09:05:30 | gmann | ameeda: np! | |
| 09:22:30 | gmann | ameeda: i still not find if that code truncated the value in case of system metadata | |
| 09:22:38 | gmann | ameeda: any failure link etc? | |
| 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 | |