| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-01-21 | |||
| 15:38:43 | sean-k-mooney | regardless of what the hw_rng_model is set to | |
| 15:40:51 | kashyap | gibi: sean-k-mooney: Yes: | |
| 15:40:51 | kashyap | - if not rng_is_virtio and not rng_allowed: | |
| 15:40:52 | kashyap | + if not rng_allowed: | |
| 15:42:01 | kashyap | Then 'hw_rng_model' will be "useless" (but we want to enhance the prop to allow two more values), as it isn't used outside this method. | |
| 15:43:20 | gibi | kashyap: if the value of hw_rng_model is not need to decide to add the device that is fine. As soon as we (you) add support the new model values then such change will make hw_rng_model used again, in the device parameters (I guess) | |
| 15:44:34 | kashyap | gibi: Yes, I'll go with the above 'diff' and document the TODO the image props to extend it for new values | |
| 15:44:56 | kashyap | gibi: Sounds okay? | |
| 15:45:37 | gibi | kashyap: yes, sounds good | |
| 15:45:39 | kashyap | (The extended two values will also be virtio variants.) | |
| 15:45:42 | sean-k-mooney | we have not added new values in several cycle and like will not in the future | |
| 15:45:50 | sean-k-mooney | so we might just want to deprecate the model | |
| 15:46:03 | sean-k-mooney | is there support in qemu for other RNGs? | |
| 15:46:04 | gibi | sean-k-mooney: I guess kashyap disagrees ^^ | |
| 15:46:34 | kashyap | sean-k-mooney: Wait: how do you know "not in the future"? The new values were added to fix certain bugs with guest compat with old operating systems | |
| 15:46:39 | kashyap | As I mentioned on the change. | |
| 15:46:41 | sean-k-mooney | well im just asking is can it have other value form a qemu point of view | |
| 15:47:22 | kashyap | sean-k-mooney: Yes: | |
| 15:47:25 | kashyap | https://libvirt.org/formatdomain.html#elementsVirtioTransitional | |
| 15:47:54 | sean-k-mooney | the model has noting to do with tanstional devices | |
| 15:48:09 | kashyap | sean-k-mooney: Hang on. | |
| 15:48:31 | kashyap | sean-k-mooney: 'virtio-transitional' is another possible value, instead of 'virtio' | |
| 15:48:48 | kashyap | Please take two minutes to read what the page is saying. | |
| 15:48:57 | sean-k-mooney | oh i see they aded that in 5.2 | |
| 15:49:07 | kashyap | I also quoted *specific* bits in the Gerrit change! | |
| 15:49:29 | sean-k-mooney | so im not sure we should be exposing that to end users | |
| 15:50:02 | kashyap | So you haven't read the comment that clearly explains it here: https://review.opendev.org/#/c/616603/10/nova/virt/libvirt/driver.py@5114 | |
| 15:50:35 | kashyap | In short: we should _not_ deprecate it. | |
| 15:50:43 | kashyap | (Where "it" == hw_rng_model) | |
| 15:51:26 | kashyap | sean-k-mooney: I'm sure we should expose them as possible values for the user | |
| 15:51:27 | sean-k-mooney | im wondering if nova can just make the chioce itself without exposeing it to the user | |
| 15:51:37 | kashyap | Hmm, I don't think we have enough info about it. | |
| 15:51:46 | sean-k-mooney | sure we do | |
| 15:52:23 | sean-k-mooney | if qemu/libvirt support the virtio-non-transitional version shoudl we not always use it | |
| 15:52:57 | kashyap | I'm of course not saying we should unconditionally enable anything what QEMU/libvirt support. | |
| 15:53:24 | kashyap | Anyway, we're going into the weeds here. The point being: we should not be in a hurry to deprecate that prop | |
| 15:53:36 | sean-k-mooney | ok | |
| 15:53:45 | sean-k-mooney | we also shoudl not be in a hurry to extend it | |
| 15:53:55 | sean-k-mooney | that is not relevent to your patch however | |
| 15:54:10 | kashyap | Yes, I'm not hurrying to extend it; I wanted to keep it a potential option. | |
| 15:54:30 | sean-k-mooney | i guess we might need to expose it for legacy os support | |
| 15:54:55 | kashyap | sean-k-mooney: gibi: To conclude the discussion: my previous comment still stands: will go with the `diff` noted earlier w/ a TODO in code | |
| 15:54:57 | sean-k-mooney | but they could jsut disbael the RNG | |
| 15:55:04 | kashyap | sean-k-mooney: Yeah, indeed on the legacy OS | |
| 15:55:19 | sean-k-mooney | ya the diff is fine with me | |
| 15:55:23 | kashyap | Okido | |
| 15:55:33 | kashyap | Thanks for bearing with me | |
| 15:56:40 | kashyap | Completely unrelated: I can't even do Ctrl-c in Grrrrrit-- anymore. Is it just me? | |
| 15:57:25 | sean-k-mooney | i got it working but i know other have had issues | |
| 15:57:41 | sean-k-mooney | its a browser thing i think | |
| 15:57:52 | sean-k-mooney | i dont think our gerrit was updated | |
| 16:00:38 | kashyap | sean-k-mooney: I tried it in both FF and Chromium :-( | |
| 16:00:49 | kashyap | (To no effect) | |
| 16:12:48 | sean-k-mooney | kashyap: so in chrome i has the "webkit-user-select: none" css property applied | |
| 16:12:56 | sean-k-mooney | to the comment | |
| 16:14:33 | sean-k-mooney | in firfox 67 it does not | |
| 16:14:57 | kashyap | sean-k-mooney: I see, will fiddle. Thank you for the tip | |
| 16:15:30 | kashyap | sean-k-mooney: Unrelated: do we have any examples of this: | |
| 16:15:33 | kashyap | "* Some properties recognized by Nova may have no effect unless a corresponding property is enabled in the server flavor." | |
| 16:16:19 | kashyap | (The 'hw_rng_model' / 'hw_rng:allowed' used to be one; but now we're changing it.) | |
| 16:16:25 | kashyap | I quoted it from Glance docs; as I'm updating them | |
| 16:21:44 | sean-k-mooney | am hw:mem_page_size in the image will only have an effect if hw:mem_page_size=any or hw:mem_page_size=large | |
| 16:22:31 | sean-k-mooney | i think the realtime mask in the image will also be ignored if you dont enable in in the flaovr but we normally try to make it an irror rather then ignore it | |
| 16:22:44 | sean-k-mooney | this is slightly different however | |
| 16:23:29 | sean-k-mooney | in that its more qulatitive. i.e. it is saying if i have a random number generateor make it of this type | |
| 16:29:19 | kashyap | sean-k-mooney: Yeah, the hw:mem_page_size is not dependent on any other property; so it's different indeed | |
| 16:29:59 | sean-k-mooney | i think the realtime mask is the best example | |
| 16:30:08 | sean-k-mooney | realtiem cpus can only be enabled in the falvor | |
| 16:30:15 | sean-k-mooney | but you can set the mask in the flavor or image | |
| 16:30:29 | sean-k-mooney | and the image mask only works if its enabled in the flavor | |
| 16:30:42 | sean-k-mooney | also http://paste.openstack.org/show/788651/ is what i think is going on with gerrit | |
| 16:30:47 | kashyap | sean-k-mooney: Got the exact property names, please? So I can fix the Gerrit document right away | |
| 16:32:24 | sean-k-mooney | i should have added https://developer.mozilla.org/en-US/docs/Web/CSS/initial | |
| 16:32:39 | kashyap | sean-k-mooney: Thank you :-) | |
| 16:32:40 | sean-k-mooney | so ya inial restores a property to its defaul state which may have changed | |
| 16:33:38 | sean-k-mooney | hehe i like that note " On inherited properties, the initial value may be unexpected. " | |
| 16:33:49 | sean-k-mooney | in other words if you use it it will break | |
| 16:33:54 | sean-k-mooney | eventually | |
| 16:35:30 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Add a default VirtIO-RNG device to guests https://review.opendev.org/616603 | |
| 16:36:13 | kashyap | sean-k-mooney: When you can, does the TODO & NOTE added above make sense to you? | |
| 16:38:36 | sean-k-mooney | im not sure i agree with likely to be extended :P but yes they make sense although i proably woudl make them both NOTE | |
| 16:39:34 | sean-k-mooney | mainlly because virtion 1.0 was standarised in 2016 | |
| 16:40:43 | sean-k-mooney | so im not sure how relevent support of explecitly running operating systems that predate that is given libvirt we will default to tansitional device model if you are using the pc machien type | |
| 16:41:52 | sean-k-mooney | rhel 6 or windows server 2013 are the two main things that come to mind but they are not going to be deploy on q35 anyway | |
| 16:52:57 | kashyap | sean-k-mooney: :-) I don't want to be overly confident and remove it yet | |
| 16:53:18 | kashyap | Hence I just wanted to make triple-sure before we remove it (and then later realize, "oh, we need to re-introduce" :-)) | |
| 16:56:32 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Add a default VirtIO-RNG device to guests https://review.opendev.org/616603 | |
| 16:56:37 | kashyap | sean-k-mooney: Okay, satisfied your request :-) --^ | |
| 16:56:45 | kashyap | sean-k-mooney: I even added "if a good reason shows up" :D | |
| 17:14:39 | artom | Has the XMLMatcher utility for unit tests not moved to oslo? | |
| 17:14:45 | artom | Keystone has one, we have one... | |
| 17:14:49 | artom | Can't find it in oslo anywhere | |
| 17:15:30 | sean-k-mooney | kashyap: lol ok didnt i mark it as a nit | |
| 17:15:43 | sean-k-mooney | kashyap: e.g. you dont need to respin but sure | |
| 17:16:22 | sean-k-mooney | we have one in nova | |
| 17:16:39 | sean-k-mooney | at least we used too | |
| 17:17:26 | artom | sean-k-mooney, yeah, but I'd like to suggest amodi use it in his whitebox patch | |
| 17:17:37 | sean-k-mooney | you can do self.assertXmlEqual | |
| 17:18:01 | artom | Not in tempest... | |
| 17:18:49 | sean-k-mooney | this is the implementaion https://github.com/openstack/nova/blob/e80300ac20388890539a7f709e526a0a5ba8e63d/nova/test.py#L534-L535 | |