| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-08-09 | |||
| 12:39:20 | mdbooth | lyarwood: An alternate (but not necessarily 'better'): write virtual size into disk.info. It's possible to do this in a backwards compatible way. We would just read it out for virtual size, update it automatically if it's missing so we don't have to fetch it again, and use state for allocated size. | |
| 12:39:25 | mdbooth | s/state/stat/ | |
| 12:39:55 | mdbooth | lyarwood: It would be fast, accurate, and secure. | |
| 12:40:27 | mdbooth | It would also be a bit more complex, so only worth it if we continue to need the data. | |
| 12:40:57 | mdbooth | If a fast get_disk_size() is required ongoing, I think we should do ^^^. If not, I think we should go with the workaround. | |
| 12:47:34 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: rewrite _get_instance_disk_info_from_config https://review.openstack.org/589567 | |
| 12:47:36 | lyarwood | mdbooth: I'd rather keep this simple if possible, what about ^ | |
| 12:49:14 | mdbooth | lyarwood: That doesn't eliminate the qemu-img call, though | |
| 12:49:17 | mdbooth | For qcow2 | |
| 12:49:23 | lyarwood | mdbooth: yeah I don't think we can | |
| 12:49:35 | lyarwood | mdbooth: the issue was reported against RAW FWIW | |
| 12:49:43 | mdbooth | You can if you cache it | |
| 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 | openstack | Launchpad bug 1770640 in nova (Ubuntu Bionic) "live block migration of instance with vfat config drive fails" [High,Fix committed] | |
| 13:04:19 | lyarwood | https://bugs.launchpad.net/nova/+bug/1770640 | |
| 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 | 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: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: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 | |