| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-22 | |||
| 17:35:29 | figleaf | this == host state dicts | |
| 17:35:57 | leakypipes | figleaf: that's cool with me. just make a note to address in a later patch is fine. | |
| 17:36:15 | openstackgerrit | Merged openstack/nova master: Fix 500 if list servers called with empty regex pattern https://review.openstack.org/506585 | |
| 17:36:20 | figleaf | leakypipes: okie dokie | |
| 17:38:11 | superdan | figleaf: leakypipes: best way to do that is throw a patch up with the removal at the end | |
| 17:38:18 | superdan | clearly broken, but a tombstone reminder :) | |
| 17:38:28 | leakypipes | superdan: yep | |
| 17:38:44 | superdan | otherwise I doubt we'll come back to it | |
| 17:43:27 | figleaf | superdan: not sure that's necessary, since we'll be changing the return value from a host (dict or HostState) to a Selection object. All the code that touches select_destinations() will be affected | |
| 17:45:08 | superdan | figleaf: okay I guess I thought this was in the middle before the point at which we'd be forming out Selection objects | |
| 17:45:41 | superdan | I misread your "knowing it will be removed in a later patch" as "I'll clean it up later" | |
| 17:46:23 | sdague | mriedem: ok, the qemu patch, who else should take a look at it - https://review.openstack.org/#/c/505673/ ? | |
| 17:46:29 | figleaf | superdan: this was a small change to accomodate a list of objects instead of a single, because alternates | |
| 17:46:41 | sdague | as it would be good to unblock new qemu | |
| 17:46:43 | figleaf | later will be a list of Selection | |
| 17:57:25 | superdan | leakypipes: just a +W needed on a test add: https://review.openstack.org/#/c/505392/7 | |
| 17:57:58 | openstackgerrit | Jackie Truong proposed openstack/nova master: Add trusted_certs to instance_extra https://review.openstack.org/457711 | |
| 18:22:07 | sdague | cburgess: responded on https://review.openstack.org/#/c/505673 | |
| 18:22:19 | leakypipes | superdan: done | |
| 18:23:30 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: set/check if inventory change in tree https://review.openstack.org/470575 | |
| 18:23:30 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: integrate ProviderTree to report client https://review.openstack.org/415921 | |
| 18:23:31 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: add nested resource providers https://review.openstack.org/377138 | |
| 18:23:31 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: allow filter providers in tree https://review.openstack.org/377215 | |
| 18:23:32 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: adds REST API for nested providers https://review.openstack.org/384807 | |
| 18:23:32 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: update client to set parent provider https://review.openstack.org/385693 | |
| 18:36:46 | cburgess | sdague Re-responded | |
| 18:37:22 | sdague | cburgess: you all really mix / match qemu runtime and tooling versions? | |
| 18:37:30 | sdague | Because that kind of seems dangerous | |
| 18:37:31 | cburgess | In the past yes. | |
| 18:37:58 | cburgess | sdague Used to be required to support qemu upgrades and live migration cross versions. In theory we now claim its simply Red Hat's problem. | |
| 18:38:26 | cburgess | sdague I'm not saying its even a reasonable thing to do anymore. I'm simply pointing out that we are assuming something there that might not be obvious to some folks. | |
| 18:38:38 | sdague | cburgess: what was the sequence of changing things you would do there? | |
| 18:39:31 | cburgess | sdague Test the version directly.. not just ask libvirt the emulate version (assuming there is some kind of --version arguement to qemu-img). | |
| 18:40:03 | sdague | cburgess: there is, but then you are text parsing vs. getting a binary number, which is a lot less robust | |
| 18:40:11 | cburgess | sdague Maybe we should need a comment above that block of code that outlines our assumption. | |
| 18:40:37 | sdague | cburgess: I don't mean for this fix, I mean, lets pretend you had a 2.8 based environment, and you wanted to live migrate to a 2.10 one | |
| 18:40:48 | sdague | what was the series of steps metacloud would do to dot hat | |
| 18:40:55 | cburgess | sdague Oh you mean how did we handle this in the past? | |
| 18:40:58 | sdague | yes | |
| 18:41:28 | openstackgerrit | Merged openstack/nova master: Add fault-filling into instance_get_all_by_filters_sort() https://review.openstack.org/505391 | |
| 18:43:50 | openstackgerrit | Merged openstack/nova master: Add a regression test for bug 1718455 https://review.openstack.org/506092 | |
| 18:43:51 | openstack | bug 1718455 in OpenStack Compute (nova) "[pike] Nova host disable and Live Migrate all instances fail." [Medium,In progress] https://launchpad.net/bugs/1718455 - Assigned to Matt Riedemann (mriedem) | |
| 18:44:34 | cburgess | sdague QEMU was installed in /opt/qemu/version_numer, We had a wrapper script that lived at /usr/sbin that libvirt would find. When libvirt propped it, it would report its latest version. When it was used to launch a VM it would look at various CLI flags to determine the original version that was used to launch that VM (in the case of an incoming live migration) and launch the original version of the emulator to ensure compat. | |
| 18:45:57 | sdague | cburgess: and qemu-img was at what version? | |
| 18:46:23 | sdague | because that's in the $PATH so you do only get one there | |
| 18:46:42 | cburgess | sdague qemu-img was always the latest. along with the proped version from libvirt. In our case this assumption is fine. I'm not saying we (Metacloud) need this. I'm just pointing out that there is an implied assumption here. | |
| 18:46:43 | sdague | assuming you had 2.6, 2.8, 2.10 installed | |
| 18:46:55 | sdague | qemu-img was 2.10 | |
| 18:47:03 | cburgess | Correct | |
| 18:47:14 | sdague | and if you queried libvirt it generically it told you 2.10 | |
| 18:47:16 | sdague | ? | |
| 18:47:28 | sdague | but it would do magic for guests that had been booted with old versions? | |
| 18:47:32 | cburgess | Correct | |
| 18:47:36 | cburgess | To all of the above. | |
| 18:47:42 | sdague | ok, in which case, this code would totally work for you | |
| 18:47:42 | mriedem | i think it's a pretty safe assumption. i think most of the code assumes that anything we do with qemu on the host is the same version that libvirt is using. | |
| 18:48:25 | sdague | and, honestly, anything more esoteric than what you are doing I would expect people to have to hack the code | |
| 18:48:34 | sdague | s/are/were/ | |
| 18:49:42 | sdague | I want to be careful here making the main path potentially more fragile for an edge world that we aren't really sure exists anywhere | |
| 18:49:53 | cburgess | OK | |
| 18:49:59 | cburgess | Just felt... wrong. | |
| 18:50:29 | sdague | cburgess: I did have a pause until I looked up that qemu-img and qemu come from the same source tarball | |
| 18:50:48 | sdague | in which case I'm not going to assume you can mix / match | |
| 18:50:54 | cburgess | You can.. | |
| 18:50:58 | cburgess | Its perfectly legal to do so. | |
| 18:51:07 | cburgess | It just happens to come in the same source build. | |
| 18:51:12 | sdague | the qemu team tells you you can? | |
| 18:51:13 | cburgess | Well legal-ish. | |
| 18:51:17 | sdague | and that they'll support it? | |
| 18:51:33 | sdague | like "it didn't blow up for us" | |
| 18:51:37 | cburgess | This is the first time I'm aware of a flag like this that requires synced version. | |
| 18:51:38 | sdague | is not the same as upstream supported | |
| 18:51:51 | sdague | cburgess: sure, that caught us off guard for sure | |
| 18:52:09 | cburgess | sdague I don't know what the "official" support policy is from upstream. | |
| 18:52:28 | cburgess | sdague I only know what the "make it work" policy is. :P | |
| 18:52:36 | sdague | if you find a piece of evidence that the qemu team says "+1 we support mix and match" I'd change my tune :) | |
| 18:52:57 | openstackgerrit | Merged openstack/nova master: Remove method "_get_host_ref_from_name" https://review.openstack.org/504796 | |
| 18:52:58 | sdague | but until then, this is pretty robust version detection, a lot more than parsing strings of stderr | |
| 18:53:37 | openstackgerrit | Merged openstack/nova master: enhance api-ref for os-server-external-events https://review.openstack.org/504263 | |
| 18:53:38 | sdague | and we could always change it later if people really needed the mix & match | |
| 18:53:44 | sdague | and came forward | |
| 18:54:10 | cburgess | sdague I removed my objection. | |
| 18:54:11 | sdague | cburgess: while you are at terminal - https://review.openstack.org/#/c/454323/ - live snapshot by default | |
| 18:54:15 | sdague | cburgess: cool | |
| 18:54:24 | mriedem | we could parse the error and retry the command with the new flag, but... | |
| 18:54:29 | mriedem | error message parsing isn't fun either | |
| 18:54:31 | sdague | mriedem: yeh | |
| 18:54:36 | cburgess | I agree... its ugly. | |
| 18:55:05 | sdague | and given that we got a new required flag we were not expecting, I'm really wary of considering error messages or even version string dumps contract | |
| 18:55:43 | cburgess | sdague Yeah so to be clear.. we never deployed libvirt 1.2.2 because it was bad. We just skipped that version which is why we never hit that live snapshot bug. But yeah we have run with it since we merged the code upstream. | |
| 18:56:01 | sdague | cburgess: ok, cool | |
| 18:56:39 | cburgess | sdague rmk added that code... umm... 4 or 5 years ago and we have been running with it enabled ever since. | |
| 18:57:30 | sdague | cburgess: yeh, I knew you all had been using that for a long time, I just realized at PTG we hadn't come around and flipped that back since getting past 1.2.2 | |
| 18:58:13 | cburgess | sdague cool.. its +1 from me. | |
| 19:00:16 | mriedem | leakypipes: superdan: https://review.openstack.org/#/c/505417/6 | |
| 19:00:18 | sdague | for the land of fun... qemu-img --version | head -1 on centos7, ubuntu 16.04, ubuntu 17.04 | |
| 19:00:24 | sdague | qemu-img version 1.5.3, Copyright (c) 2004-2008 Fabrice Bellard | |
| 19:00:32 | sdague | qemu-img version 2.5.0 (Debian 1:2.5+dfsg-5ubuntu10.16), Copyright (c) 2004-2008 Fabrice Bellard | |
| 19:00:39 | sdague | qemu-img version 2.8.0(Debian 1:2.8+dfsg-3ubuntu2.4) | |
| 19:00:47 | cburgess | Yeah you will break RHEL with that change. | |