| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-14 | |||
| 16:14:21 | kashyap | 17:06 < kashyap> cfriesen: After image conversion completes successfully, QEMU issues fsync(), thus flushing the content down to disk. | |
| 16:14:38 | kashyap | cfriesen: So, the numbers are correct, FWIW. The data _is_ on disk. | |
| 16:15:05 | kashyap | cfriesen: I even audited QEMU's code (with call trac) to track down 'qemu-img convert' ends up calling fsync(). | |
| 16:15:35 | kashyap | cfriesen: There is the call trace (since I sometimes see you pop into #qemu on OFTC :-)) -- https://kashyapc.fedorapeople.org/qemu-img-call-trace-to-fsync.txt | |
| 16:16:13 | kashyap | dansmith: Hehe, saw your "church retreat" remark on the channel archives (my IRC proxy kicked me, thanks to the IDENTIFY) | |
| 16:16:30 | kashyap | (I commented on that weshay asked) | |
| 16:16:34 | kashyap | s/on that/on that review/ | |
| 16:20:51 | cfriesen | kashyap: I just did an strace of qemu-img convert, and I see it calling fdatasync() rather than fsync(). But that should still be sufficient. | |
| 16:21:28 | kashyap | Yes, if fdatasync() is available it is used, otherwise its cousin fsync() | |
| 16:21:46 | kashyap | See the QEMU function call trace I posted earlier :-) | |
| 16:21:59 | kashyap | cfriesen: Shall I respin for the s/multiple orders/multiple times/? | |
| 16:23:03 | cfriesen | I did make it a nit. :) | |
| 16:24:42 | kashyap | cfriesen: Saw that; that's what I was referring to | |
| 16:25:00 | cfriesen | I mean, if I really thought it required a respin I'd have done -1 | |
| 16:25:25 | cfriesen | but if you're respinning for any other reason you could fix that up too | |
| 16:27:01 | cfriesen | do we need four separate commits for this? I mean, if we make the change one place it makes sense to make it everywhere, right? | |
| 16:28:01 | cfriesen | I've gotten flack for too-big patches though, so maybe my opinion is biased. :) | |
| 16:31:56 | kashyap | cfriesen: See the discussion in the scrollback here in the channel with mdbooth (who asked for separte patches) | |
| 16:32:12 | kashyap | Because: each one is touching a completely different driver (SMBFS vs. Virtuozzo, etc) | |
| 16:32:32 | kashyap | (It is the "one logical change per patch" rule.) | |
| 16:33:06 | mdbooth | cfriesen: Just MHO, but the potential for fallout and therefore reverts seems slightly different in each case. | |
| 16:33:22 | cfriesen | okay, works for me. | |
| 16:33:44 | mdbooth | But they all definitely have the same theme. | |
| 16:34:27 | mdbooth | cfriesen: And we can't test virtuozzo | |
| 16:34:38 | kashyap | Exactly, the "potential for reverts" is the thing | |
| 16:35:25 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Use 'writeback' QEMU cache mode for Nova instances https://review.openstack.org/641981 | |
| 16:35:25 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: qemu: Make disk image conversion dramatically faster https://review.openstack.org/640781 | |
| 16:35:26 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: smbfs: Use 'writeback' QEMU cache mode https://review.openstack.org/643377 | |
| 16:35:26 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: vzstorage: Use 'writeback' QEMU cache mode https://review.openstack.org/643376 | |
| 16:35:34 | kashyap | cfriesen: ^ Mind re-acking? | |
| 16:36:16 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP/PoC: Bypass ironicclient for node.get https://review.openstack.org/642899 | |
| 16:37:42 | cfriesen | I suspect the biggest impact will be I/O usage patterns on the host, since now we'll be queueing up a bunch of writes and flushing them all at once. | |
| 16:37:48 | cfriesen | kashyap: done | |
| 16:38:57 | kashyap | cfriesen: Thank you | |
| 16:45:34 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP/PoC: Bypass ironicclient for node.get https://review.openstack.org/642899 | |
| 16:50:00 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP/PoC: Bypass ironicclient for node.get https://review.openstack.org/642899 | |
| 17:03:57 | jackding | Could cores look at https://review.openstack.org/#/c/621646/? It's been too long. | |
| 17:18:15 | kashyap | cfriesen: Fancy having a gander at: https://review.openstack.org/#/c/631154/ (WIP: Gracefully handle QEMU machine types for guests) | |
| 17:27:53 | dansmith | mriedem: who was asking about the infinite recursion ssl thing yesterday? | |
| 17:28:04 | dansmith | oh EmilienM | |
| 17:28:05 | dansmith | https://review.openstack.org/#/c/626952/ | |
| 17:29:21 | mdbooth | dansmith: Yeah, so I was previously going on faith, but it didn't appear to fix the test. | |
| 17:29:35 | mdbooth | But it *did* just fix the reproducer system I had direct access to. | |
| 17:30:01 | dansmith | hrm, this looks weird, I'll have to do some poking | |
| 17:30:23 | mdbooth | It's very weird. If you look in the BZ I have a minimal non-nova python reproducer. | |
| 17:31:13 | mdbooth | Based on my understanding of the eventlet bug, the issue is that we need to do monkey patching really early before importing most things. | |
| 17:31:29 | mdbooth | ...as some code runs at import time. | |
| 17:31:32 | dansmith | yeah | |
| 17:31:40 | mdbooth | So that was the goal of my patch. | |
| 17:31:49 | cfriesen | kashyap: I think the blueprint name is supposed to match the spec filename | |
| 17:31:59 | mdbooth | Have one thing that you import first that does the monkey patching without importing anything else. | |
| 17:32:45 | openstack | bugzilla.redhat.com bug 1686817 in openstack-nova "[osp15] nova is broken on the undercloud with ssl" [Urgent,Assigned] - Assigned to mbooth | |
| 17:32:45 | mdbooth | OSP15 bz is here: https://bugzilla.redhat.com/show_bug.cgi?id=1686817 | |
| 17:33:17 | dansmith | mdbooth: if you're patching now before importing oslo.context, I imagine that reload is no longer necessary | |
| 17:33:24 | kashyap | cfriesen: Yeah, noticed it. Can I still update the BP URL? | |
| 17:33:39 | kashyap | cfriesen: I wrote that BP while still the discussions were still hot on QEMU and libvirt lists on the default | |
| 17:34:02 | cfriesen | kashyap: no idea | |
| 17:34:08 | kashyap | cfriesen: Where they were on the verge to change the default to 'q35', but backed out due to "maybe we're breaking backwards compat" | |
| 17:34:34 | mdbooth | dansmith: Quite possibly, but I just moved it. | |
| 17:34:39 | dansmith | yeah I know | |
| 17:34:47 | dansmith | just answering your TODO | |
| 17:35:18 | dansmith | er, note | |
| 17:35:35 | kashyap | cfriesen: Yep, I could change it. | |
| 17:37:30 | dansmith | yikun: FYI https://review.openstack.org/#/c/592285/ | |
| 17:37:39 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova-specs master: WIP: Gracefully handle QEMU machine types for guests https://review.openstack.org/631154 | |
| 17:39:53 | dansmith | mdbooth: why are there two identical LP bugs? | |
| 17:39:58 | dansmith | or am I confusing something? | |
| 17:40:18 | kashyap | cfriesen: Fixed. | |
| 17:40:36 | mdbooth | dansmith: Unclear. | |
| 17:41:21 | mdbooth | dansmith: Is the other one a tripleo tracker? | |
| 17:41:33 | dansmith | shouldn't be necessary with LP | |
| 17:42:16 | dansmith | mdbooth: so, the cmd/__init__.py one.. does that get run by the entrypoint before anything in something like cmd/compute ? because at first glance, it seems to not move it for those commands, if it comes in the first time they import cmd.common | |
| 17:42:27 | dansmith | I guess it must.. | |
| 17:42:54 | mdbooth | dansmith: Yes, it does. The BZ is against nova compute in the undercloud, which isn't wsgi. | |
| 17:43:49 | dansmith | right, okay so this ends up actually running sooner in the compute case because we've done the patching before we import utils, which imported oslo stuff? | |
| 17:44:15 | mdbooth | Yes. | |
| 17:44:39 | dansmith | because you changed things to run at import time, it's maybe a little more obscure, but really the fix here is detaching the monkeypatch from the utils module (or anything else that has a bunch of stuff in it) | |
| 17:45:40 | mdbooth | Yeah. I don't think there's any way to avoid running anything at import time, tbh, because that's the fundamental underlying issue | |
| 17:45:50 | mdbooth | you need to run monkey_patch() before importing stuff | |
| 17:46:29 | dansmith | no, I know.. I get that part | |
| 17:47:18 | mdbooth | Yes. The purpose of the monkey_patch module is to do monkey patching with minimal dependencies. | |
| 17:55:33 | mdbooth | dansmith: Thanks | |
| 17:55:41 | dansmith | aye | |
| 17:56:49 | kashyap | dansmith: If you want some fresh change of scenery, a simple catch: https://review.openstack.org/#/q/topic:bug/1818847+(status:open+OR+status:merged) | |
| 17:57:08 | dansmith | I really don't | |
| 17:57:11 | kashyap | Hehe | |
| 17:58:18 | kashyap | dansmith: It brings 15 times faster performance for image conversion :D -- https://review.openstack.org/#/c/640781/ | |
| 17:58:51 | kashyap | I ran it through the QEMU Block Layer maintainer, who also reviewed the patches over e-mail. And gave their: "Looks-good-to-me'd-by", FWIW | |
| 18:46:05 | mriedem | someone want to approve these for gate goodness? https://review.openstack.org/#/q/topic:bug/1819944+(status:open+OR+status:merged)+project:openstack/nova | |
| 18:46:17 | mriedem | gets the ceph portion of our live migration (+grenade) jobs running again | |
| 18:46:55 | mriedem | efried: maybe you want to hit https://review.openstack.org/#/c/643150/ and melwitt can get the one below | |
| 18:47:47 | efried | mriedem: is that reverting something? | |
| 18:47:56 | mriedem | yeah in part | |
| 18:47:58 | mriedem | revert + fix | |
| 18:48:09 | mriedem | it's reverting this https://review.openstack.org/#/c/643122/ | |
| 18:48:29 | efried | k, Ima update commit message accordingly | |
| 18:48:58 | openstackgerrit | Eric Fried proposed openstack/nova master: Re-enable Ceph in live migration testing https://review.openstack.org/643150 | |
| 18:50:07 | efried | mriedem: done | |
| 18:52:34 | mriedem | thanks | |
| 19:01:47 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP/PoC: Bypass ironicclient for node.get https://review.openstack.org/642899 | |