| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-08-09 | |||
| 12:49:57 | lyarwood | true | |
| 12:50:25 | mdbooth | And then you've also got 1 less code path to test | |
| 12:50:42 | lyarwood | well you still need it if it isn't cached | |
| 12:50:47 | lyarwood | for now | |
| 12:50:55 | lyarwood | but longer term we can remove it | |
| 12:51:05 | mdbooth | Right, but you can put that in a utility call with separate tests | |
| 12:51:08 | lyarwood | mdbooth: are there util methods for reading/writing to disk.info btw? | |
| 12:51:18 | mdbooth | No, we'd need to pull it out of imagebackend | |
| 12:51:34 | mdbooth | (A good thing) | |
| 12:54:37 | mdbooth | lyarwood: But that's conditional on us continuing to need this stuff. If it has a limited shelf life it's not worth it. | |
| 12:55:17 | mdbooth | Although I still don't understand why we don't need allocated disk any more. | |
| 12:57:00 | lyarwood | mdbooth: well we still need it for LM | |
| 12:57:42 | lyarwood | mdbooth: tbh I'd rather land something simple like this as a bugfix and then work to switch to disk.info outside of this in a bp or something | |
| 12:59:12 | mdbooth | lyarwood: Sure. I'd prefer the workaround over the extra code paths for sure. | |
| 12:59:31 | lyarwood | mdbooth: wait, getting confused now, which workaround? | |
| 12:59:36 | lyarwood | mdbooth: os.stat? | |
| 12:59:43 | mdbooth | lyarwood: No, your original one. | |
| 12:59:50 | mdbooth | os.stat() doesn't fix it, we established that | |
| 13:01:05 | mdbooth | lyarwood: So... land your original workaround, with the disk.info thing in reserve. | |
| 13:01:34 | lyarwood | mdbooth: that workaround breaks LM | |
| 13:01:59 | lyarwood | mdbooth: that's why I'm suggesting using os.stat and os.path.getsize for RAW at least | |
| 13:02:28 | mdbooth | How does it break LM, btw? | |
| 13:03:00 | lyarwood | mdbooth: see my comment, LM with non-shared storage where we are creating images on the dest in pre_live_migration | |
| 13:03:42 | lyarwood | mdbooth: if we don't get an accurate virtual size we end up creating images that are too small | |
| 13:04:05 | lyarwood | https://review.openstack.org/#/c/589567/3 - my comment there sorry | |
| 13:04:19 | lyarwood | https://bugs.launchpad.net/nova/+bug/1770640 | |
| 13:04:19 | openstack | Launchpad bug 1770640 in nova (Ubuntu Bionic) "live block migration of instance with vfat config drive fails" [High,Fix committed] | |
| 13:10:41 | mdbooth | lyarwood: We shouldn't be live migrating a config disk anyway | |
| 13:10:46 | mdbooth | That sounds like a different bug | |
| 13:10:58 | mdbooth | We should just host->host copy it | |
| 13:11:35 | lyarwood | mdbooth: yeah the issue wasn't with the config disk but the main instance disk iirc | |
| 13:12:17 | mdbooth | Ok. | |
| 13:12:32 | lyarwood | hmm actually that's vdb | |
| 13:12:43 | lyarwood | but you can see we are mirroring | |
| 13:17:41 | odyssey4me | Hi folks. Is there a conf entry for the number of workers nova-scheduler fires up? | |
| 13:17:43 | mdbooth | lyarwood: Ok, now I understand the interaction. | |
| 13:17:53 | odyssey4me | I can't seem to find one in the references. | |
| 13:19:39 | odyssey4me | ok, it would appear that there is one: https://github.com/openstack/nova/blob/master/nova/cmd/scheduler.py#L49 | |
| 13:28:33 | mriedem | dansmith: i'm +2 on https://review.openstack.org/#/c/582413/ if you want to re-apply your +2 | |
| 13:29:12 | mriedem | bauzas: can you go through these backports? https://review.openstack.org/#/q/topic:bug/1784705+status:open | |
| 13:29:34 | mriedem | mel said she was looking to cut stable releases today | |
| 13:29:40 | mriedem | so i'm going to try and flush some of these out | |
| 13:30:01 | dansmith | okay | |
| 13:40:40 | mriedem | stephenfin_: e. gads. https://review.openstack.org/#/q/topic:bug/1746393+status:open | |
| 13:41:43 | mriedem | feels like a feature as a bug fix | |
| 13:42:49 | mriedem | especially nervous when we have 0 CI of any of this stuff | |
| 13:46:59 | mriedem | sean-k-mooney[m]: i guess the intel 3rd party PCI/NFV CI must be dead huh? | |
| 13:47:58 | mriedem | dansmith: here are the queens backports with a +2 ready to go https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:stable/queens+label:Code-Review=2 - the rest are in stephen's series, which i'd want to hold off on for a bit | |
| 13:49:18 | mriedem | oh and https://review.openstack.org/#/c/590062/ would be nice - that fix sat for over a year | |
| 13:50:14 | melwitt | nova meeting in 10 minutes | |
| 13:53:08 | melwitt | cdent: is this bug considered closed/fixed now that both patches have landed? neither patch used Closes-Bug in the commit message https://bugs.launchpad.net/nova/+bug/1786055 | |
| 13:53:08 | openstack | Launchpad bug 1786055 in OpenStack Compute (nova) "performance degradation in placement with large number of resource providers" [High,In progress] - Assigned to Chris Dent (cdent) | |
| 13:54:05 | cdent | melwitt: hmmm. There is more than can be done, but not likely that more will be done _now_, so I would guess closed is probably a reasonable state. The major factor has been addressed. Fixing the rest will involve considerable refactoring | |
| 13:56:05 | melwitt | cdent: I see, thanks | |
| 14:01:09 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Use os.stat and os.path.getsize for RAW disk inspection https://review.openstack.org/589567 | |
| 14:17:09 | mriedem | i guess we don't need to wait for translations https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:zanata/translations | |
| 14:20:20 | melwitt | yeah, I was thinking it seems like openstack doesn't do translations anymore but I didn't know how to check. I'll add that link to the release checklist wiki | |
| 14:20:34 | melwitt | I know we don't translate log messages | |
| 14:20:49 | melwitt | but other user-facing message, still translate? I wasn't sure | |
| 14:21:15 | efried | afaik we're still supposed to _() for exception messages. | |
| 14:21:37 | lyarwood | mdbooth: remind me again where the compute code was that deleted and recreated an attachment? | |
| 14:21:44 | melwitt | aye, I have seen that | |
| 14:22:57 | mdbooth | lyarwood: _terminate_volume_connections | |
| 14:24:17 | lyarwood | mdbooth: urgh was looking at remove_volume_connection | |
| 14:25:01 | mdbooth | lyarwood: IIRC it was triggering a bug in the cinder fixture, which assumed only 1 attachment | |
| 14:25:13 | mdbooth | But with this we've briefly got 2 attachments | |
| 14:26:30 | mdbooth | lyarwood: I don't love the raw-only fix, tbh, because I think it increases the test and maintenance burden. If that code needs to live on I'd prefer to bring it together somehow. | |
| 14:27:39 | mdbooth | I'll abandon the stat thing, though, because as you point out it's not a solution | |
| 14:28:36 | lyarwood | mdbooth: kk, well it improves performance for the raw images user that reported the issue in the short term until we start using disk.info to store the virtual size | |
| 14:29:00 | lyarwood | mdbooth: and given that means we also need to refactor code out of imagebackend I'd rather land something simple first then focus on that | |
| 14:30:28 | mdbooth | I don't think it's a refactor, btw. Just code motion really iirc. | |
| 14:30:50 | mdbooth | Would just be moving it elsewhere to make it easier to call. | |
| 14:31:23 | mriedem | GET /kashyap returns me a 404 | |
| 14:31:29 | mriedem | is his nick not registered? | |
| 14:31:41 | mdbooth | mriedem: Yep. He's out for a few more days yet, I think | |
| 14:31:46 | mriedem | blarg | |
| 14:31:51 | mriedem | wanted him to read the "nova-compute choosing incorrect qemu binary when scheduling 'alternate' (ppc64, armv7l) architectures?" thread in the ML | |
| 14:32:15 | mriedem | wondering if there is a good reason why we don't set guest.arch in the libvirt domain xml based on the hw_architecture image property | |
| 14:32:22 | mriedem | maybe someone could ask danpb? | |
| 14:33:05 | mdbooth | stephenfin_: might have an opinion | |
| 14:33:57 | mdbooth | mriedem: Is it tagged [nova]? | |
| 14:34:23 | openstackgerrit | Lee Yarwood proposed openstack/nova master: Add regression test for bug#1784353 https://review.openstack.org/587014 | |
| 14:34:24 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP compute: Terminate volume connections during _shutdown_instance https://review.openstack.org/590348 | |
| 14:34:37 | lyarwood | mdbooth: ^ that's the terminate_volume_connections alternative btw, without unit test changes | |
| 14:34:53 | mdbooth | lyarwood: That was fast! Looking. | |
| 14:35:49 | mdbooth | mriedem: Do you have an opinion on ^^^, btw? | |
| 14:36:51 | mdbooth | mriedem: Basically leaves us with a blank attachment when calling _shutdown_instance | |
| 14:36:58 | lyarwood | hmm that removes the call to detach with cinderv2 | |
| 14:37:05 | lyarwood | why would we do that during shutdown? | |
| 14:38:35 | mdbooth | lyarwood: remind me what v2 detach() does | |
| 14:38:52 | mriedem | mdbooth: seems fun at first since it's very much the same code, | |
| 14:38:55 | mriedem | but see inline comments | |
| 14:39:13 | mdbooth | terminate_connection() causes the storage backend to kill the connection | |
| 14:39:25 | mriedem | mdbooth: v2 detach is just the os-detach volume action which changes the volume status to 'available' | |
| 14:39:31 | mriedem | does nothing on the volume backend | |
| 14:39:33 | mdbooth | mriedem: Got it. | |
| 14:40:32 | mriedem | so if you did this, | |
| 14:41:20 | mriedem | you'd have to have volume attachment cleanup code in both compute manager if we don't reschedule (max_attempts=1 or force_hosts/nodes is set) or if we do reschedule and conductor build_instances hits MaxRetriesExceeded | |
| 14:41:43 | mriedem | which is essentially what we have for cleaning up ports | |
| 14:41:49 | sean-k-mooney | mdbooth: stephenfin_ is on vaction for the next week and a half just fyi | |