| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-18 | |||
| 14:00:59 | mriedem | i know writeback is recommended for rbd users, so is there a bias here? | |
| 14:01:02 | kashyap | mriedem: On that topic, it is. I ran this past the QEMU Block Layer folks over a lengthy thread | |
| 14:01:16 | mriedem | kashyap: yeah i trust you've done your homework | |
| 14:01:19 | kashyap | See the commit message for on the question of NFS | |
| 14:01:20 | mriedem | given the commit message | |
| 14:01:37 | mriedem | well, which commit message? | |
| 14:01:44 | kashyap | 1 sec | |
| 14:01:52 | mriedem | i guess you mean https://review.openstack.org/#/c/640781/9//COMMIT_MSG | |
| 14:01:55 | mriedem | which i also commented on | |
| 14:02:03 | mriedem | but again, those are different patches | |
| 14:02:34 | mriedem | one is image conversion and one is the disk config for local disks yeah? | |
| 14:02:36 | kashyap | mriedem: Ah, just saw your comment. So, that applies to both the patches | |
| 14:02:56 | kashyap | So I should add remark in the patch that changes it for the Nova instances? | |
| 14:03:18 | kashyap | s/?// | |
| 14:03:51 | mriedem | probably, but it's saying that if the host crashes during writes to NFS, it doesn't matter - but with writethrough each write gets an fsync yeah? | |
| 14:03:53 | kashyap | I checked with Kevin Wolf, he said in _both_ cases: (a) for image conversion; and (b) for Nova instances | |
| 14:03:54 | mriedem | and with writeback they wouldn't | |
| 14:03:59 | mriedem | so you'd lose those writes on NFS right? | |
| 14:04:04 | kashyap | ... 'writeback' is fine even in NFS. | |
| 14:04:21 | kashyap | mriedem: Yes, with 'writethrough', each write gets an fsync(). | |
| 14:04:21 | mriedem | does this all assume some minimum version of qemu for that safety behavior? | |
| 14:04:28 | mriedem | b/c as noted, danpb wrote this code | |
| 14:05:18 | kashyap | mriedem: Yes, danpb also incidentally saw and said: "Nova is using the wrong cache mode in both cases, conversion & for guests" | |
| 14:05:23 | kashyap | On minimum version -- | |
| 14:05:56 | bauzas | mriedem: apologies if you felt I trampled your -1 by a new PS, it was just for stopping the gate run, i should have added my own -1 with it | |
| 14:06:02 | kashyap | mriedem: We have it since QEMU 1.1 (released in 2012). I noted it in the commit message with references: see lines 67-71 | |
| 14:07:50 | mriedem | kashyap: ack yeah ok i glossed over that | |
| 14:08:16 | mriedem | but that is also true in the non-image conversion case? | |
| 14:09:03 | kashyap | mriedem: Yest that's what the QEMU Block Layer folks said. Let me confirm once more. | |
| 14:10:09 | kashyap | mriedem: Okay, yes. even for non-image conversion case, 'writeback' is correct. | |
| 14:10:29 | mriedem | so let me ask another way, in what case would anyone use writethrough? | |
| 14:10:38 | kashyap | Good question | |
| 14:11:20 | kashyap | mriedem: You'd use it if your guest OS really really sucks and doesn't issue fluses where data must be safe on disk. | |
| 14:11:25 | kashyap | But note: | |
| 14:12:46 | kashyap | Such "suck OSes" are things RHEL-4 or even older versions of Windows | |
| 14:13:17 | kashyap | FWIW, the commit message and the text I've written is also reviewed by QEMU folks; I wasn't riffing on it :-) | |
| 14:15:53 | mriedem | i'm not sure there is a way to make this configuration per-guest in nova, | |
| 14:15:54 | mriedem | but ok | |
| 14:16:10 | mriedem | i.e. i'm not aware of an image property to override the configuration | |
| 14:16:24 | kashyap | Me neither | |
| 14:24:57 | kashyap | mriedem: Replied on the commit on your note about DanPB's 2012 commit. | |
| 14:27:53 | openstackgerrit | Lance Bragstad proposed openstack/nova master: Clarify policy short-comings in policy enforcement doc https://review.openstack.org/643960 | |
| 14:30:12 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/os-vif master: Add functional test to check the addition of an interface to a linux bridge https://review.openstack.org/643964 | |
| 14:37:31 | mriedem | lbragstad: replied in https://review.openstack.org/#/c/643949/1/doc/source/reference/policy-enforcement.rst@117 - that's a good question for which i'm not sure i have a good answer, | |
| 14:37:39 | mriedem | i'm inclined to say we could just remove that section | |
| 14:37:46 | mriedem | but maybe run it by the patrole/ops people first | |
| 14:41:35 | gmann | tssurya: sure, i will check today. | |
| 14:41:57 | lbragstad | mriedem sweet | |
| 14:42:20 | openstackgerrit | OpenStack Release Bot proposed openstack/os-resource-classes stable/stein: Update .gitreview for stable/stein https://review.openstack.org/644028 | |
| 14:42:22 | openstackgerrit | OpenStack Release Bot proposed openstack/os-resource-classes stable/stein: Update UPPER_CONSTRAINTS_FILE for stable/stein https://review.openstack.org/644029 | |
| 14:42:24 | openstackgerrit | OpenStack Release Bot proposed openstack/os-resource-classes master: Update master for stable/stein https://review.openstack.org/644030 | |
| 14:42:33 | openstackgerrit | OpenStack Release Bot proposed openstack/os-traits stable/stein: Update .gitreview for stable/stein https://review.openstack.org/644031 | |
| 14:42:35 | openstackgerrit | OpenStack Release Bot proposed openstack/os-traits stable/stein: Update UPPER_CONSTRAINTS_FILE for stable/stein https://review.openstack.org/644032 | |
| 14:42:37 | openstackgerrit | OpenStack Release Bot proposed openstack/os-traits master: Update master for stable/stein https://review.openstack.org/644033 | |
| 14:42:46 | openstackgerrit | OpenStack Release Bot proposed openstack/os-vif stable/stein: Update .gitreview for stable/stein https://review.openstack.org/644034 | |
| 14:42:48 | openstackgerrit | OpenStack Release Bot proposed openstack/os-vif stable/stein: Update UPPER_CONSTRAINTS_FILE for stable/stein https://review.openstack.org/644035 | |
| 14:42:51 | openstackgerrit | OpenStack Release Bot proposed openstack/os-vif master: Update master for stable/stein https://review.openstack.org/644036 | |
| 14:43:38 | lbragstad | mriedem i'm not sure how much review spam you want - but i was planning on following up each section in 643960 with it's own patch | |
| 14:44:28 | mriedem | context in greater detail." | |
| 14:44:28 | mriedem | has a section dedicated to describing the underlying cause and historical | |
| 14:44:28 | mriedem | yeah i assumed that was coming since it says "Each issue | |
| 14:47:25 | lbragstad | if that's too verbose - i can propose it in one patch, too | |
| 14:49:55 | mriedem | lbragstad: a question and some nits in https://review.openstack.org/#/c/643960/ | |
| 14:50:57 | lbragstad | awesome - i'll take a look | |
| 14:53:35 | kashyap | mriedem: So, on cache mode change for Nova guests: (a) The existing Nova help text is broken: it claims "writethrough" as the default, it is not the case; (b) My patch doesn't change the _default_ -- see my last comment here: https://review.openstack.org/#/c/641981/7 | |
| 15:00:15 | kashyap | It seems you've added that help text from an IBM help doc (commit: 3d9a76bd71159a3e8ce0acdb27ec76381b1024fc) | |
| 15:00:47 | mriedem | kashyap: yes, i fleshed out the help from libvirt and ibm help docs because before that it was a one-line unhelpful help string | |
| 15:01:11 | kashyap | mriedem: Yeah, I see your intention was good. I'm trying to find the new URL of that IBM doc | |
| 15:01:36 | kashyap | Because QEMU's default has been 'writeback' (for guests) for ages; not sure why the IBM doc claimed it to be 'writethrough' | |
| 15:02:05 | mdbooth | Can anybody tell me which package provides the 'openssl' binary on Ubuntu? | |
| 15:02:54 | mriedem | umm, openssl? | |
| 15:03:23 | mdbooth | mriedem: That's what it's called on Fedora. Just wanted to ensure it's not called something else in foreign parts. | |
| 15:03:29 | mriedem | http://lmgtfy.com/?q=ubuntu+openssl+package | |
| 15:04:00 | kashyap | mriedem: Hehe, I seem to have already debunked the IBM doc 2014! -- https://www.redhat.com/archives/libvirt-users/2014-August/msg00060.html (Behavior of disk caching with qcow2 disks | |
| 15:04:46 | mdbooth | Or however that translates when you make it brown | |
| 15:07:14 | openstackgerrit | Matthew Booth proposed openstack/nova master: Add missing test bindep on openssl https://review.openstack.org/644238 | |
| 15:07:14 | openstackgerrit | Matthew Booth proposed openstack/nova master: Remove eventlet hub workaround for monotonic clock https://review.openstack.org/644237 | |
| 15:16:56 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Use 'writeback' QEMU cache mode for Nova instances https://review.openstack.org/641981 | |
| 15:16:56 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: smbfs: Use 'writeback' QEMU cache mode https://review.openstack.org/643377 | |
| 15:16:56 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: vzstorage: Use 'writeback' QEMU cache mode https://review.openstack.org/643376 | |
| 15:19:13 | openstackgerrit | Gorka Eguileor proposed openstack/nova master: Fix unnecessary locking when connecting volumes https://review.openstack.org/614190 | |
| 15:20:18 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Use 'writeback' QEMU cache mode when 'none' is not viable https://review.openstack.org/641981 | |
| 15:20:35 | kashyap | bauzas: mriedem: efried: The above patch doesn't need a rel eng; we aren't changing the default - I updated the commit message to reflect that. | |
| 15:21:27 | kashyap | (And fixed the wrong info in the Nova conf doc, which came in via another misinfo from IBMs's doc, which disappeared) | |
| 15:23:37 | kashyap | Huh, let me reorder them patches correctly. Sorry for the noise. | |
| 15:25:29 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: vzstorage: Use 'writeback' QEMU cache mode https://review.openstack.org/643376 | |
| 15:25:29 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Use 'writeback' QEMU cache mode when 'none' is not viable https://review.openstack.org/641981 | |
| 15:25:30 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: smbfs: Use 'writeback' QEMU cache mode https://review.openstack.org/643377 | |
| 15:27:26 | gibi | melwitt, mriedem: the bandwidth documentation patch for neutron is ready to review https://review.openstack.org/#/c/640390 | |
| 15:30:04 | mriedem | gibi: ok, should probably get mlavalle to review that since he worked on it | |
| 15:32:12 | gibi | mriedem: sure | |
| 15:32:26 | gibi | I just wanted to note that it is not wip any more | |
| 15:54:18 | tssurya | thanks gmann | |
| 16:16:52 | kashyap | efried: Just to tie-up the loose end: your observation was correct earlier -- self._disk_cachemode does _not_ get fed in by the CONF.libvirt.disk_cachemodes | |
| 16:20:24 | kashyap | mdbooth: Thanks for the review, again. It's like a whack-a-mole. Just when I think I fixed all occurrences... | |
| 16:21:09 | mriedem | aspiers: efried: i'm doing some testing in devstack with the compute capabilites as traits stuff and i'm not seeing where / when a sighup is needed https://review.openstack.org/#/c/538498/22/releasenotes/notes/driver-capabilities-to-traits-152eb851cd016f4d.yaml | |
| 16:21:46 | mdbooth | kashyap: Hehe. | |
| 16:22:14 | mdbooth | kashyap: Incidentally, if at all possible can you avoid rebasing? | |
| 16:23:09 | kashyap | mdbooth: Sure. I was fixing commit messages by re-ordering patches, and forgot to put them back in the same order one of the times | |
| 16:23:28 | mdbooth | Ack | |