| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-08-01 | |||
| 18:18:24 | mriedem | well there is a json formatter, and a context formatter | |
| 18:18:33 | sdague | yes, we're using the context formatter | |
| 18:18:38 | mriedem | and a color handler, and journal and syslog handler | |
| 18:18:43 | mriedem | but how would someone know? | |
| 18:18:47 | mriedem | config? | |
| 18:19:27 | sdague | it's config, but I think those are the defaults | |
| 18:20:55 | sdague | oh, you know what it is, it's actually python logging config | |
| 18:20:59 | sdague | outside of our config | |
| 18:21:10 | sdague | so use_syslog / use_journal will tweak those things | |
| 18:21:14 | sdague | https://docs.openstack.org/oslo.log/latest/configuration/index.html | |
| 18:21:27 | sdague | but on the formatter side it's all done as python logger setup | |
| 18:23:14 | mriedem | ok, because what i think we're seeing with a bunch of these random stacktraces related to ComputeHostNotFound_Remote is that we're logging with an exception context, | |
| 18:23:18 | mriedem | and oslo.log is logging the traceback | |
| 18:23:30 | mriedem | automagically | |
| 18:24:20 | mriedem | https://www.youtube.com/watch?v=iExgnVXSAuE | |
| 18:24:24 | mriedem | thanks oslo.log | |
| 18:26:11 | mriedem | gonna push a patch to test that theory | |
| 18:27:54 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: See if we can squash ComputeHostNotFound_Remote on startup https://review.openstack.org/489683 | |
| 18:27:55 | mriedem | sdague: dansmith: ^ is the canary | |
| 18:29:23 | mriedem | i think it's something to do with this: https://github.com/openstack/oslo.log/blob/3.30.0/oslo_log/formatters.py#L144 | |
| 18:36:45 | openstackgerrit | Merged openstack/nova master: Test resize with placement api https://review.openstack.org/487958 | |
| 18:36:58 | mriedem | yay ^ | |
| 18:37:13 | openstackgerrit | Jackie Truong proposed openstack/nova master: Add trusted_certs to instance_extra https://review.openstack.org/457711 | |
| 18:54:24 | jaypipes | gibi: where do I change the notification sample test JSON files? | |
| 18:54:58 | jaypipes | gibi: nm, found it. | |
| 19:02:15 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: remove existing allocs when set allocs https://review.openstack.org/489273 | |
| 19:02:15 | openstackgerrit | Jay Pipes proposed openstack/nova master: remove provider allocs in confirm/revert resize https://review.openstack.org/488510 | |
| 19:02:16 | openstackgerrit | Jay Pipes proposed openstack/nova master: Additional assertions to resize tests https://review.openstack.org/489714 | |
| 19:05:41 | melwitt | this is a bug fix (data corruption possible) I think we'll need for rc1, if anyone can review please https://review.openstack.org/#/c/488545/ | |
| 19:07:33 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285 | |
| 19:22:38 | openstackgerrit | Eric Fried proposed openstack/nova master: nova.utils.get_endpoint_data() https://review.openstack.org/488137 | |
| 19:23:09 | mriedem | melwitt: question in there | |
| 19:23:16 | mriedem | the persistent vs live stuff always confuses me | |
| 19:25:25 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Cleanup unnecessary logic in os-volume_attachments controller code https://review.openstack.org/485823 | |
| 19:25:36 | cfriesen | mriedem: isn't "persisten" whether there is a persistent domain (unrelated to the device)? | |
| 19:26:14 | melwitt | mriedem: I'm not sure I understand the question. in my comment I tried to say "if DeviceNotFound was raised when we passed persistent=True, that means it wasn't found, so we should continue on to try a live detach". though I do notice I should have used "if persistent and live" | |
| 19:26:33 | melwitt | because if live isn't True then there's no need to continue | |
| 19:26:42 | mriedem | well there you go | |
| 19:27:08 | melwitt | is that what you were pointing out? sorry :) | |
| 19:27:15 | mriedem | no | |
| 19:27:39 | mriedem | i just wanted to ask a question to sound like i know 2 sh*ts about this code before +2ing it | |
| 19:27:59 | mriedem | <- professional | |
| 19:28:15 | melwitt | cfriesen: yeah, from what I understand there's effectively two copies of the VM config. one is "persistent" to affect upon next boot and the other is "live" which affects only the live VM | |
| 19:28:19 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285 | |
| 19:28:28 | cdent | jmlowe: I’m going to break the bourbon pattern to give you some early for the good advice of the day award | |
| 19:28:41 | mriedem | melwitt: the comment makes more sense now | |
| 19:29:13 | melwitt | so if the VM even has a persistent config, you have to detach the device from both configs to make sure the device is detached live and also won't show up again after a reboot | |
| 19:29:25 | melwitt | it's confusing | |
| 19:29:48 | mriedem | melwitt: that would be good doc to put in that code | |
| 19:29:57 | melwitt | will do | |
| 19:31:02 | cfriesen | melwitt: so we try first calling detach_device() with both persistant and live but libvirt raises an exception since it doesn't find it in the persistent config? | |
| 19:31:50 | mriedem | so the bug is we tried to detach from persistent config and it wasn't there, so we gave up, but it was in the live config, and then the user reboots the guest and now the volume is attached to both? | |
| 19:32:00 | cfriesen | melwitt: so we have to retry just the live detach without the persistant | |
| 19:32:05 | jmlowe | cdent: what advise was that? "never shit in your hat" "money talks and bullshit walks" "shit in one hand wish in the other and see which one fills up first" | |
| 19:32:19 | melwitt | cfriesen: right. this can happen in the scenario where the guest is busy (e.g. file open) and the guest ignores the ACPI request to detach from live. so what happens there is the detach from the persistent config succeeds but the live fails and so the overall detach fails | |
| 19:32:54 | mriedem | can qemu / libvirt just add a "seriously_please_detach_this_thing_at_some_point" API? | |
| 19:33:02 | jmlowe | cdent: it appears that most of my advise is scatological in nature | |
| 19:33:06 | cdent | jmlowe: ceph your glance and your disk | |
| 19:33:11 | cfriesen | melwitt: I take it libvirt chokes if you specify both VIR_DOMAIN_AFFECT_CONFIG and VIR_DOMAIN_AFFECT_LIVE and it's not in the persistant? | |
| 19:33:12 | melwitt | cfriesen: later on, if the guest is all done and the file is closed, the user wants to detach the volume again, they will issue the detach command and we'll need to detach it from only the live config bc it's already gone from the persistent config | |
| 19:33:16 | cdent | which may be scat | |
| 19:33:20 | melwitt | cfriesen: tes | |
| 19:33:22 | melwitt | *yes | |
| 19:33:56 | melwitt | libvirt will raise a "no device found" type error due to the AFFECT_CONFIG flag | |
| 19:34:21 | cfriesen | melwitt: almost seems safer to just handle persistent and live separately rather than trying to do them both and have to clean up if it fails | |
| 19:34:35 | cfriesen | but I suppose it's probably more efficient to do them both at the same time | |
| 19:34:43 | jmlowe | cdent: I do what I can, I was practically frothing at the mouth to go from Liberty to Mitaka just for the ceph glance nova stuff | |
| 19:35:16 | melwitt | cfriesen: yeah, I have wondered similar. in a normal scenario you'd only need one call to do the whole thing | |
| 19:37:01 | cfriesen | melwitt: looks okay to me with the caveat of adding that check for "live" | |
| 19:37:36 | melwitt | cfriesen: cool, thanks. I'm working on adding that and beefing up the test to match | |
| 19:37:44 | melwitt | and adding more code comments | |
| 19:38:32 | cfriesen | seems like we could have run into problems with the second call currently if live was false | |
| 19:39:27 | melwitt | yeah, possibly. I'm not sure what it does if you pass no flags, maybe a no-op one would hope | |
| 19:42:14 | melwitt | okay, 0 is VIR_DOMAIN_AFFECT_CURRENT=0 | |
| 19:42:15 | melwitt | Affect current domain state. so it would do something, hopefully raising one of the "not found" we handle and then it would bubble up to compute which would ignore it | |
| 19:43:37 | mriedem | jaypipes: want to -2 this so someone doesn't get confused it's not for pike? https://review.openstack.org/#/c/488595/ | |
| 19:44:17 | openstackgerrit | Matthew Edmonds proposed openstack/nova master: update policy UT fixtures https://review.openstack.org/398610 | |
| 19:47:32 | cfriesen | melwitt: looks like libvirt virDomainDetachDeviceFlags() will error if "flags" is not set. | |
| 19:47:49 | cfriesen | based on a quick check of the code | |
| 19:47:59 | mriedem | dims: i'm beating my head against some weird traceback logging we're seeing but don't know where the traceback is actually coming from https://review.openstack.org/#/c/489683/2 | |
| 19:48:05 | mriedem | dims: i assume it's something in oslo | |
| 19:48:39 | melwitt | cfriesen: okay. well, we'd pass 0 for flags if neither persistent nor live, and I thought 0 was a valid flag | |
| 19:49:58 | cfriesen | melwitt: wait, I think I misread. | |
| 19:50:01 | cfriesen | oops | |
| 19:50:11 | cfriesen | was looking at the function pointer, not the variable | |
| 19:54:11 | cfriesen | melwitt: the common code does not check for empty flags, but passes it on down to the specific sub-driver (ie the qemu one). | |
| 20:01:58 | cfriesen | melwitt: based on virDomainObjUpdateModificationImpact() it looks like if flags is empty it will set either VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG based on whether or not the domain is currently active | |
| 20:02:37 | melwitt | cfriesen: ah, cool. so that's what they mean by VIR_DOMAIN_AFFECT_CURRENT (which is 0) | |
| 20:02:49 | cfriesen | yes | |
| 20:18:11 | openstackgerrit | melanie witt proposed openstack/nova master: Detach device from live domain even if not found on persistent https://review.openstack.org/488545 | |
| 20:20:56 | cdent | mriedem: if you’re in a docs way here’s a bit of placement docs: https://review.openstack.org/#/c/469048/ | |
| 20:23:59 | mriedem | ack | |
| 20:24:46 | mriedem | dansmith: whilst reviewing your cells v2 topology docaroo, i realized we could/should disable CONF.filter_scheduler.track_instance_changes in the superconductor mode for devstack, | |
| 20:24:55 | mriedem | since the computes are basically rpc casting into the ether | |
| 20:24:59 | dansmith | ah yeah | |
| 20:25:29 | sdague | speaking of docs, if anyone wants to fix a whole lot of our 404s... https://review.openstack.org/#/c/489650/ one more +2 | |
| 20:25:36 | mriedem | so i'm +2 on the doc https://review.openstack.org/#/c/487183/ but what did you have in mind for documenting the upcall limitations? | |
| 20:26:40 | dansmith | mriedem: I can throw those into the bottom set of things if you want | |
| 20:27:02 | mriedem | ok, i mentioned them in ps5 to go in there, but you didn't add them so i didn't know what you had planned | |
| 20:29:02 | dansmith | unintentional | |