| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-06-17 | |||
| 18:34:33 | sean-k-mooney | but it is the default we shoudl use | |
| 18:34:43 | sean-k-mooney | qxl works for linux and windows guests | |
| 18:34:47 | melwitt | does that mean qxl doesn't work on windows? | |
| 18:35:00 | melwitt | ok. I was thinking maybe the default should be qxl | |
| 18:35:12 | sean-k-mooney | but while its the best of the non virto option im not sure we should default to it | |
| 18:35:24 | sean-k-mooney | qxl was developed to have a better gpu for spice | |
| 18:35:25 | melwitt | but defaulting to a deprecated thing? | |
| 18:35:33 | sean-k-mooney | it work with novnc too | |
| 18:35:53 | sean-k-mooney | melwitt: without recored the current gpu we could break people on upgrade | |
| 18:36:06 | sean-k-mooney | so if we want to chang eit we need to record the current value | |
| 18:36:29 | sean-k-mooney | were recordign the current value is just popluating that value in the system metadata table | |
| 18:37:16 | melwitt | guh, I don't know enough about this stuff to understand what needs to be done here. all I know is the cirrus default messes up novnc | |
| 18:37:28 | melwitt | on rhel 8.2 | |
| 18:37:32 | sean-k-mooney | melwitt: as of when because it used to work | |
| 18:37:52 | melwitt | it works, it's just greyscale and low res | |
| 18:37:53 | sean-k-mooney | ah so its a rhel 8.2 issue with what the qemu version we use or no vnc | |
| 18:38:09 | melwitt | yeah | |
| 18:38:16 | sean-k-mooney | that sound like a qemu bug | |
| 18:38:43 | sean-k-mooney | basically if we change the default and you upgrade then hard reboot | |
| 18:38:51 | sean-k-mooney | the gpu pci device would change | |
| 18:39:11 | sean-k-mooney | if you dont have the qxl driver then the console would not work right | |
| 18:39:39 | sean-k-mooney | qxl has a vga comapatible interface to the generic window vga driver will work with it an dit should work on linux out of the box | |
| 18:39:47 | melwitt | I see | |
| 18:44:36 | sean-k-mooney | melwitt: are we setting a default graphic model if you dont request one? | |
| 18:44:54 | sean-k-mooney | or are we currently leaving it to libvirt/qemu | |
| 18:45:18 | melwitt | yeah, the link I posted earlier https://github.com/openstack/nova/blob/6ed556f4adfd8a98fae2e91204f1ce0484f5dd9d/nova/virt/libvirt/config.py#L1949 we set cirrus | |
| 18:50:59 | sean-k-mooney | melwitt: ya we proably should just remove that line | |
| 18:51:25 | melwitt | ok. I'll do some local tests to see what happens when I remove it | |
| 18:52:17 | sean-k-mooney | well we need to set it to none i guess and only populate the xml atribute if its set | |
| 18:52:51 | sean-k-mooney | we also have another place we defualt it i think | |
| 18:53:14 | sean-k-mooney | we should not be defaulting it in that file in anycase | |
| 18:53:26 | melwitt | in driver.py we set it special based on things like virt_type or guest arch or spice | |
| 18:53:42 | sean-k-mooney | yep that is what im thinking of | |
| 18:53:52 | melwitt | but in the customer's case it's cirrus because of what's in the default config.py | |
| 18:54:09 | sean-k-mooney | we should only be adding this if you have a vnc/spice console too | |
| 18:54:32 | melwitt | "this" meaning what, qxl? | |
| 18:55:27 | sean-k-mooney | the graphics device | |
| 18:55:43 | melwitt | oh that config section altogether you mean | |
| 18:55:59 | sean-k-mooney | ya if you use the serial console you should not have a graphics device | |
| 18:56:25 | melwitt | ah yeah, we are actually only doing it for certain consoles in driver.py | |
| 18:56:31 | sean-k-mooney | if you use novnc or spice then we auto add a graphic element to provide the frame buffer | |
| 18:56:33 | melwitt | I just didn't see that earlier | |
| 18:56:52 | melwitt | yeah we're only doing for novnc and spice | |
| 18:57:03 | sean-k-mooney | yep which is correct | |
| 18:57:05 | melwitt | that we add that xml section for the video device | |
| 18:57:11 | sean-k-mooney | well i guess we would do it for RDP | |
| 18:57:13 | sean-k-mooney | maybe? | |
| 18:57:23 | sean-k-mooney | oh RDP is only supported with hyperv | |
| 18:57:23 | melwitt | I don't see rdp in here | |
| 18:57:31 | melwitt | yeah it's hyperv | |
| 18:57:50 | melwitt | ok, so sounds like everything is good except we shouldn't be setting a default and just let qemu/kvm handle it | |
| 18:58:07 | sean-k-mooney | https://github.com/openstack/nova/blob/6ed556f4adfd8a98fae2e91204f1ce0484f5dd9d/nova/virt/libvirt/driver.py#L5989-L5998 | |
| 18:58:26 | sean-k-mooney | melwitt: that would alow them to contol the default and or deprecation | |
| 18:58:35 | melwitt | make sense | |
| 18:58:36 | sean-k-mooney | but the downside is we loose contol over that | |
| 18:58:45 | sean-k-mooney | so our guest abi will alter with theres | |
| 18:59:03 | sean-k-mooney | that is ok i guess since you can use the image property if you care | |
| 18:59:16 | melwitt | and yeah that's the code I was looking and the helper function being called there | |
| 19:00:21 | sean-k-mooney | ya so this https://github.com/openstack/nova/blob/6ed556f4adfd8a98fae2e91204f1ce0484f5dd9d/nova/virt/libvirt/driver.py#L5351-L5367 is basically working around libvit/qemu not doing the right thing by default | |
| 19:01:43 | sean-k-mooney | well and the fact we set a default in the xml generation class | |
| 19:01:51 | sean-k-mooney | so if we remove that default | |
| 19:02:02 | sean-k-mooney | then we might not need all those specal cases | |
| 19:02:37 | melwitt | ok, well one step at a time I guess :P | |
| 19:02:52 | melwitt | I do some local tests to see what happens when I remove the default | |
| 19:02:57 | melwitt | *I'll | |
| 19:03:45 | sean-k-mooney | ok let me know an i can review. https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/ is a review of some to the option and recomendations | |
| 19:04:21 | sean-k-mooney | i need to remove the gop device i added by mistake last cycle and add https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/#bochs-display | |
| 19:04:21 | melwitt | thanks | |
| 19:04:45 | sean-k-mooney | apparently gop is only for the bhve hypervior but the libvirt doc did not mention that | |
| 19:06:40 | sean-k-mooney | bochs implements a uefi graphic output protocol (gop) device for kvm | |
| 19:07:03 | sean-k-mooney | so that is the one i should have added | |
| 19:10:06 | sean-k-mooney | melwitt: during the ptg we disucssed breifly the idea of a resize for something other then a flavor change | |
| 19:10:58 | sean-k-mooney | maybe this could be another usecause for that | |
| 19:12:19 | sean-k-mooney | i think this was in the context of changing machine_types where i was suggeign a new retype server action but perhaps something more generic that would update the embed flavor and image with the latest would make more sense | |
| 19:12:40 | sean-k-mooney | e.g. a recreate action but that preserved all ports, volume and disks | |
| 19:14:13 | melwitt | how was that suggestion received, I'm curious heh | |
| 19:14:57 | melwitt | like, I know lots of people would like to do that. in my case, I'm trying to recover from what I consider a bug, the default being set to a deprecated thing and glitching out the vnc console | |
| 19:15:20 | sean-k-mooney | reusing resize was dansmith idea i was thinking it shoudl be its onw server action. not sure if dansmith actully was in favor or just saying if it was to be done i would start there | |
| 19:15:45 | melwitt | ah, cool | |
| 19:16:08 | sean-k-mooney | melwitt: the main think would be that recreate would have to be a move operation like resize so we can validate the new requirement | |
| 19:16:19 | sean-k-mooney | since we cant assume the current host is valid anymore | |
| 19:16:36 | melwitt | yeah. well, we validate the new stuff on a rebuild too since a few releases ago | |
| 19:16:51 | sean-k-mooney | but i think it would fix a lot of the thing that hard reboot fix but which require the flaovr or image properties to be modified | |
| 19:17:06 | sean-k-mooney | melwitt: only if the image changes | |
| 19:17:13 | melwitt | yeah, it would make life a lot easier for a lot of people | |
| 19:17:46 | sean-k-mooney | yeah so i think it has merit but not sure where it sits in the priortiy queue | |
| 19:17:49 | melwitt | yeah, I'm just saying that we validate even during rebuild so I think it makes sense to validate on a recreate/resize if we're consuming new image props etc | |
| 19:18:03 | melwitt | we're already doing it | |
| 19:18:10 | melwitt | *doing similar | |
| 19:18:21 | sean-k-mooney | for me i think it would be a good way to opt into changing the machine type for example | |
| 19:19:59 | sean-k-mooney | anyway im going to go buy icecream and i guess other food before the stores start to close | |
| 19:22:53 | sean-k-mooney | hum not sure if good pr or pr blunder https://www.benjerry.ie/flavours/netflix-and-chillld-ice-cream | |
| 19:28:03 | melwitt | thanks for the video model chat o/ | |
| 19:36:30 | mnaser | is there reasoning behind what has ended up being validated in the api layer and what doesn't get validated in the api layer? | |
| 19:37:03 | mnaser | i haven't replicated this yet (but in theory i can imagine it's possible): end user tried to create a vm where image size > disk size when doing bfv | |
| 19:37:16 | mnaser | and they don't really get much more of a helpful error (and their request gets refused too obviously) | |
| 19:37:35 | mnaser | but only after it fails, in what i assume nova-compute trying to ask cinder to create a disk that's impossible to create | |
| 19:53:43 | melwitt | mnaser: generally we validate whatever we can in the api layer to fail fast and avoid the behavior you're describing (fail later on nova-compute). within reason though, as some checks would involve rpc call to the compute host and we avoid that to some degree | |
| 19:54:24 | melwitt | mnaser: I found this part where we validate the image_min_disk against the volume disk size, but that might be different than just the image size as in your example? https://github.com/openstack/nova/blob/master/nova/compute/api.py#L681 | |
| 19:56:15 | mnaser | melwitt: oh interesting, you're right, i wonder if this is a case where the user did not have min_disk specified.. | |