Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-14
16:00:28 kashyap Some simple catch above, if anyone wants a change of scenery.
16:00:47 kashyap (The core change and documentation is in: libvirt: Use 'writeback' QEMU cache mode for Nova instances https://review.openstack.org/641981)
16:02:14 cdent edleafe: α ω
16:02:57 edleafe cdent: make up your mind: greek or roman??
16:04:48 cfriesen kashyap: quick question. did you try running a "sync" after the writeback image conversion test to ensure the data had actually hit the disk?
16:05:17 cfriesen or do we even care if it did or not?
16:05:49 kashyap cfriesen: No, I didn't do. But, 'sync' is not required at all in this case -- why?
16:06:59 cfriesen well, your numbers look a lot like it just wrote to the cache. If the host crashes at that point, it might think it has valid data on-disk but actually have a corrupt image.
16:09:35 cfriesen now this may not actually be a problem in practice, just thought I'd raise it.
16:11:10 kashyap cfriesen: I got dropped off my IRC proxy; what was the last message you got from me?
16:11:31 kashyap (And if you wrote anything else after your "quick question ...", please re-post)
16:12:35 cfriesen I just said... your numbers look a lot like it just wrote to the cache. If the host crashes at that point, it might think it has valid data on-disk but actually have a corrupt image. Now this might not be a problem in practice, but I thought I'd mention it.
16:14:08 kashyap cfriesen: Noted, but ... I spent careful 4 days talking to QEMU Block Layer maint folks.
16:14:18 kashyap 17:05 < kashyap> cfriesen: No, I didn't do. But, 'sync' is not required at all in this case -- why?
16:14:18 kashyap And not sure if my messages got through:
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

Earlier   Later