| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-08-09 | |||
| 12:13:11 | lyarwood | mdbooth: well before you rm -rf it | |
| 12:13:39 | lyarwood | mdbooth: I still think we could use the getsize approach for raw disks | |
| 12:13:47 | lyarwood | mdbooth: and your stat call | |
| 12:14:22 | lyarwood | mdbooth: that way, no workaround, just an additional bugfix for RAW disks | |
| 12:14:58 | mdbooth | Honestly, I'd prefer to avoid doing anything complicated for a legacy code path. Adding if <raw>, elif <qcow2>, elif <lvm>... | |
| 12:15:10 | mdbooth | Doesn't seem like a good plan. I'll take your hack over that. | |
| 12:16:05 | lyarwood | mdbooth: well we already do that here anyway tbh | |
| 12:16:28 | lyarwood | disk_type == file driver_type == ploop etc | |
| 12:23:35 | mdbooth | lyarwood: Right, but we'd be adding at least 1 new code path. Also, we'd still be slow for qcow2 at least sometimes. | |
| 12:25:21 | mdbooth | lyarwood: I'm ambivalent here in case you hadn't picked up :) I'm not necessarily against your hack, just thinking if it's worth the effort to do better. | |
| 12:26:08 | lyarwood | mdbooth: yeah I really don't enjoy touching this stuff tbh as something always comes up but I think the stat/getsize approach for RAW is the best we can offer | |
| 12:27:25 | mdbooth | Incidentally, what scheme do we now have which isn't interested in actual disk usage? | |
| 12:27:30 | mdbooth | That seems odd. | |
| 12:27:55 | s10 | update_available_resource() with this call to get_allocated_disk_size is blocking some other operations only in post_live_migration: https://github.com/openstack/nova/commit/ab1e48f4683315db631be3f0995be6258edf6997 ? Do we really need this call here now? | |
| 12:29:31 | mdbooth | s10: Intuitively I'd say yes, but based on the same assumptions which tell me we should still be interested in actual disk usage. | |
| 12:29:46 | mdbooth | ...which we're apparently not. | |
| 12:30:10 | lyarwood | yeah placement should handle that now, so I think we can actually remove this? | |
| 12:30:23 | mdbooth | lyarwood: But how does placement handle it? | |
| 12:30:41 | mdbooth | Placement doesn't know anything about actual disk usage which the hypervisor didn't tell it. | |
| 12:31:11 | lyarwood | mdbooth: yeah but I didn't think that was coming from the RT but I'm likely wrong. | |
| 12:32:01 | mdbooth | Unless we deprecated disk overcommit? | |
| 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. | |