| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-04 | |||
| 11:35:27 | johnthetubaguy | yaawang: ah... thank you, I forgot about the VM needing to reboot if you loose network connectivity with Post Copy | |
| 11:35:31 | gmann | johnthetubaguy: ^^ updated for current comments. i will check other review comments tomorrow. | |
| 11:37:52 | johnthetubaguy | gmann: thanks, sorry for the delay, too much multi-tasking... and I need to get some lunch | |
| 11:38:29 | gmann | johnthetubaguy: i know, how many time you need to switch the context :) | |
| 11:38:41 | johnthetubaguy | gmann: I am wondering if we need to split the details into two... | |
| 11:39:34 | johnthetubaguy | gmann: maybe this should be two specs (...ducks) | |
| 11:40:13 | johnthetubaguy | gmann: so first bit is admin_only and admin_or_owner with tests and better default check_str and scope_types | |
| 11:40:16 | gmann | johnthetubaguy: hummm you mean separate for scope and default roles ? | |
| 11:40:26 | gmann | ok | |
| 11:40:29 | johnthetubaguy | gmann: second spec is adding the Reader role support? | |
| 11:40:56 | johnthetubaguy | now... I know the reader role support is why we are doing it really | |
| 11:41:27 | johnthetubaguy | I just think we need to separate the two bits of work, as they have two different sets of thinking around how we keep it backwards compatible | |
| 11:41:35 | yaawang | johnthetubaguy: Pleasure, can you remove -1 on the gerrit? :) | |
| 11:41:39 | johnthetubaguy | I think the Reader thing is easy, once you have the other stuff in place | |
| 11:42:43 | johnthetubaguy | yaawang: I am still -1 the current approach though, because it talks about the specific implementation, we need to talk about what the workload needs in a way that is hypervisor agnostic... ideally that is. | |
| 11:42:44 | gmann | johnthetubaguy: so in first bit we will keep all GET with system_admin or project_member etc and in second we change them to reader roles | |
| 11:42:55 | johnthetubaguy | yaawang: I will come back with a better suggestion after I have had lunch | |
| 11:43:21 | johnthetubaguy | gmann: yeah, I think that is what we agreed at the PTG in terms of splitting up the patches as we improve the coverage | |
| 11:43:45 | johnthetubaguy | gmann: the second reader roles spec is where we need the extra granular roles too, I guess | |
| 11:44:17 | gmann | yeah, granular rules is needed mainly for reader capability | |
| 11:44:54 | johnthetubaguy | gmann: do you think that makes sense? I don't want to drag it out too much, but I think we need that split due to all test coverage and the change to the DB level check | |
| 11:45:15 | johnthetubaguy | gmann: so the DB level check would go with the reader change I think | |
| 11:46:17 | johnthetubaguy | now the customers I have really want the reader role, they don't care about the other bits, they are "just a dependency" to make things work :( | |
| 11:46:18 | gmann | but DB check needs to adjust with scope_type right say when we will change current admin to system_admin | |
| 11:46:36 | johnthetubaguy | well the DB check is only needed by the Reader rule | |
| 11:46:42 | johnthetubaguy | I mean... | |
| 11:46:42 | gmann | i am still thinking if split can end up with 2 upgrade impact for operator. | |
| 11:47:20 | gmann | ohk, yeah mostly GET thing | |
| 11:47:27 | johnthetubaguy | so I think the reader role addition has no upgrade impact... unless you happen you use the role "reader" instead of "member" | |
| 11:48:28 | johnthetubaguy | well, there granularity change is an impact I guess, but it only affects folks who have changed those policy rules, which should be quite a smaller number of folks | |
| 11:49:03 | johnthetubaguy | gmann: so maybe the best thing is keep it as one spec, but make that clear split in the spec description? | |
| 11:49:27 | gmann | yeah that looks much better to me. | |
| 11:50:28 | gmann | current spec is split for scope_type and roles. i can make it to 1. admin, admin-or-owner -> system scope, project member 2. reader role and DB checks change | |
| 11:52:55 | gmann | basically first bit goes more for adding scope type only with adjusted check_str and second goes for adopting default roles. hummm | |
| 11:53:52 | johnthetubaguy | gmann: I think the first does scope_types and check_str optionally checking for Member, and scope:project, etc | |
| 11:54:53 | johnthetubaguy | gmann: I think the first does scope_types and check_str optionally checking for Member, and scope:project, etc | |
| 11:55:00 | johnthetubaguy | hmm, maybe that's not right either | |
| 11:55:17 | johnthetubaguy | so I really should get some food, my brain is failing me | |
| 11:58:07 | gmann | it mainly separate the current admin from project operation. i mean current project admin would not be able to perform new system level admin operation unless token is scoped with system. | |
| 11:58:31 | gmann | if deployment choose to enforce the scope_type | |
| 12:00:37 | gmann | i still feel reader ability makes these changes more useful otherwise operator can still add project member with system scope and lie to nova/oslo. | |
| 12:06:07 | johnthetubaguy | gmann: yeah, maybe we just do this in one shot... going to think on that over lunch | |
| 12:06:47 | johnthetubaguy | yaawang: thank you for explaining your use case, I think if we rename the image properties and flavor extra specs slightly, I am happy. | |
| 12:07:01 | johnthetubaguy | yaawang: I have added a suggestion on your spec | |
| 12:07:32 | gmann | johnthetubaguy: ok, will catch with you ( or on gerrit) tomorrow. thanks for review and detail discussion. | |
| 12:08:49 | yaawang | johnthetubaguy: Thanks, will look later... | |
| 12:48:16 | amyltsev | Hello, could someone advice, can I have name of volumes which were created during creation instances with volume creation, like the instance name? | |
| 12:59:55 | kashyap | sean-k-mooney: On 'virtio-blk' vs. 'virtio-scsi', I'd say you got it the other way round: 'virtio-scsi' was designed to address the limitations of 'virtio-blk' | |
| 13:01:16 | sean-k-mooney | kashyap: virtio-scsi is generally slower as it has to emulate a scsi contoller | |
| 13:01:55 | kashyap | sean-k-mooney: In _some_ workloads 'virtio-scsi' is slower, in others, it outperforms 'virtio-blk' | |
| 13:02:29 | yonglihe | sean-k-mooney: Hi | |
| 13:03:02 | sean-k-mooney | yonglihe: hi | |
| 13:03:25 | sean-k-mooney | kashyap: in anycase we are defaulting to sata so its not really that relevent | |
| 13:03:42 | yonglihe | I'm finding you that NUMA stuff api spec | |
| 13:03:44 | sean-k-mooney | kashyap: i normally only use virtio-scsi when im using ceph | |
| 13:03:59 | kashyap | sean-k-mooney: Sure, but I wanted to point out that correction | |
| 13:04:02 | kashyap | Yep, noted | |
| 13:04:23 | kashyap | sean-k-mooney: Also most new features are implemented on 'virtio-scsi'-only; due to the difficulty to extend 'virtio-blk' | |
| 13:05:18 | yonglihe | sean-k-mooney: Hope you have time ,thanks. https://review.opendev.org/#/c/658716/ spec "show-server-numa-topology" | |
| 13:05:52 | sean-k-mooney | kashyap: yes i know. i do follow the qemu/kvm development too not quite as closely as you but enought to know where to look this up when i need too | |
| 13:06:10 | kashyap | (Nod) | |
| 13:09:15 | sean-k-mooney | the storage subsystem in qemu/kvm is one i have looked into a few times but its also the first thing i swap out of memory :) | |
| 13:10:09 | kashyap | sean-k-mooney: Hehe, I spent far too much time following and playing with the Block Layer | |
| 13:10:18 | kashyap | And _still_ I swap out routinely | |
| 13:11:40 | yonglihe | Paste "clean up orphan instances" here, need review : https://review.opendev.org/#/c/627765/ | |
| 13:12:42 | sean-k-mooney | yonglihe: i just responded to your question on v2 and ill review v4 after i grab a cup of coffee. | |
| 13:13:04 | lyarwood | kashyap: https://github.com/openstack/nova/blob/2ea6e6f8db9fc6cecf389cacdd0d82d8226b99fb/nova/conf/libvirt.py#L714 - Any idea why we don't set defaults for hw_machine_type? | |
| 13:13:32 | kashyap | lyarwood: We set for AArch64 and s390x; but not for x86_64 | |
| 13:13:40 | sean-k-mooney | yonglihe: claning up instance not listed in the db can be dangous in some cases | |
| 13:14:04 | sean-k-mooney | ill try to review that also but how are you validiating that openstack created the instnce | |
| 13:14:19 | sean-k-mooney | are you checking for the metadata we add in the domian xml? | |
| 13:14:22 | yonglihe | I create a out of band vm | |
| 13:14:35 | kashyap | lyarwood: We don't set for x86_64, because we don't have enough information for that. | |
| 13:15:01 | kashyap | lyarwood: That's what this spec is supposed to handle: https://review.openstack.org/#/c/631154/ ("WIP: Gracefully handle QEMU machine types for guests") | |
| 13:15:03 | lyarwood | kashyap: I don't even see a default listed in that section | |
| 13:15:04 | sean-k-mooney | yonglihe: to be clear my concern is we allow people to run non openstack manged vms on openstack compute nodes. so your orpahn patch should not break that usecase | |
| 13:15:25 | lyarwood | kashyap: ah right | |
| 13:15:35 | kashyap | lyarwood: We choose the defaults for non-x86_64 in the method you just moved: get_machine_type() | |
| 13:15:47 | kashyap | I even added a NOTE there :-) | |
| 13:15:48 | kashyap | if caps.host.cpu.arch in (obj_fields.Architecture.ARMV7, | |
| 13:15:48 | kashyap | obj_fields.Architecture.AARCH64): | |
| 13:15:48 | kashyap | mach_type = "virt" | |
| 13:15:58 | kashyap | Likewise, for s390x | |
| 13:16:29 | yonglihe | sean-k-mooney: if it's dangers in any way, we should address that. for the none OS vms, some ones need this to clean up and some one does not. the default action now is noop. | |
| 13:17:24 | lyarwood | kashyap: right sorry I'm specifically talking about the hw_machine_type option itself | |
| 13:17:33 | lyarwood | kashyap: and the fact that option doesn't actually have a default | |
| 13:17:36 | sean-k-mooney | yonglihe: right but what im suggesting is we shoudl chacke the domain xml to confim if it contains the metadata we add with the instacne uuid/flavor/image and other info were we say this vm was created by nova | |
| 13:17:59 | lyarwood | kashyap: I assumed it did and that caused loads of tests to fail as the final lookup would use that config option and return None | |
| 13:18:01 | sean-k-mooney | yonglihe: if that info is not present then we should not reap the vm as it was not create by nova | |
| 13:18:32 | kashyap | lyarwood: Even for the config option *itself*, we don't set default as we don't have enough information about the guest OS | |
| 13:18:35 | yonglihe | sean-k-mooney: seems like a good point and we need that. | |
| 13:19:04 | lyarwood | kashyap: ack understood | |
| 13:19:05 | kashyap | lyarwood: That's why we delegate setting default machine type to orchestrators. | |
| 13:19:41 | yonglihe | sean-k-mooney: you may want to comment this to the patch and i gonna address that. -:) | |
| 13:20:06 | sean-k-mooney | yonglihe: that will driver dependent but i think it makes sense to have a call to the driver to retrun the set of possible ophean instances and then for the compute manager to ask the driver to reap them. | |
| 13:20:12 | sean-k-mooney | sure ill add it to the review | |
| 13:22:45 | mriedem | lyarwood: bauzas: dansmith: can we get these stein backports in so i can do a release? https://review.opendev.org/#/q/topic:bug/1830747+status:open+branch:stable/stein | |
| 13:23:10 | bauzas | mriedem: ack I can take a look | |
| 13:23:18 | yonglihe | sean-k-mooney: driver specific is spitted out, there is 2 patches on set. find the orphan seems like logic belong to compute but checking the metadata is definitely driver's scope. | |
| 13:25:50 | sean-k-mooney | yonglihe: well i woudl expect the driver to be the thing that check what intances are running on the host and provide a kwarg to allow filtering to just vms it created. | |