| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-28 | |||
| 13:06:08 | alex_xu | in the end, we said use req filter or not... | |
| 13:24:08 | stephenfin | alex_xu: I thought we were going to copy the approach from https://review.opendev.org/#/c/671801/32/nova/scheduler/utils.py | |
| 13:24:38 | stephenfin | so that's why I dragged https://review.opendev.org/#/c/674894/ to the front of the series | |
| 13:25:25 | stephenfin | mriedem, dansmith: You have been summoned by efried https://review.opendev.org/#/c/675338/ | |
| 13:26:45 | stephenfin | alex_xu: Yeah, that's what efried's comments are saying too. I guess aspiers is waiting to make sure that gets in first | |
| 13:27:09 | mriedem | stephenfin: why? | |
| 13:27:20 | stephenfin | Haven't a notion | |
| 13:27:52 | mriedem | the extra spec in those api samples could just as well be {'fart': 'sniffer'} | |
| 13:28:02 | stephenfin | Maybe because it's microversion related | |
| 13:28:11 | stephenfin | Yup, that's what I thought | |
| 13:28:27 | mriedem | it's not really, | |
| 13:28:36 | mriedem | but i see a good reason to -1 the shit out of your patch | |
| 13:28:47 | mriedem | oh drat https://review.opendev.org/#/c/675338/11/nova/tests/fixtures.py | |
| 13:28:48 | mriedem | you already got it | |
| 13:29:16 | stephenfin | #winning | |
| 13:30:15 | mriedem | +2 | |
| 13:38:42 | mriedem | mdbooth: for https://review.opendev.org/#/c/677736/ i think we're just waiting for your customer to test it and ack it right? | |
| 13:38:44 | aspiers | stephenfin, alex_xu, efried: my understanding was that we were going to keep it in req filter for now and then change it later | |
| 13:39:24 | aspiers | kashyap: I don't have the slightest issue with features being split into two *meaningful* sections. But a section entitled "type" is totally meaningless. | |
| 13:39:42 | mdbooth | mriedem: So... they did test it but they didn't provide the same logs as last time. I've asked them for them, but they haven't been provided yet. | |
| 13:39:44 | mriedem | gibi: can i get a re-+W on https://review.opendev.org/#/c/637605/37 ? | |
| 13:40:00 | mriedem | mdbooth: did they notice the slow down? | |
| 13:40:18 | aspiers | kashyap: Let me take another tack. Please explain to me what this feature section entitled "type" is supposed to contain. | |
| 13:40:49 | aspiers | kashyap: And while you're at it, I'd like to understand what the section entitled "readonly" is supposed to contain. | |
| 13:41:11 | mdbooth | mriedem: Previously they were seeing compute hosts flapping, but that was only logged by n-api. We don't have logs for n-api. | |
| 13:41:12 | mriedem | stephenfin: i've re-ordered some of the less complicated changes in the cross-cell resize series to the bottom, could you take a look at this simple one to plumb passing the power_on flag down through driver.spawn()? https://review.opendev.org/#/c/642590/30 | |
| 13:41:20 | stephenfin | I sure can | |
| 13:41:28 | mriedem | which incidentally is something i think mdbooth has asked for in the past - the ability to create a stopped server | |
| 13:41:30 | mdbooth | mriedem: They *are* experience other environmental issues. | |
| 13:41:31 | mriedem | or maybe i'm dreaming that up | |
| 13:41:40 | mdbooth | mriedem: Yep, I have asked for that. | |
| 13:41:41 | kashyap | aspiers: I agree that a generic section is meaningless. Maybe 'loader_type', which tells _where_ the guest memory should be mapped -- whether it s a 'rom' or 'pflash' ? | |
| 13:42:10 | mdbooth | mriedem: So what I wanted to see be sure is: in the presence of their environmental issues, are the compute nodes continuing to flap? | |
| 13:42:15 | mdbooth | I'm hoping the answer's no. | |
| 13:42:24 | mriedem | right | |
| 13:42:39 | aspiers | kashyap: https://libvirt.org/formatdomaincaps.html#elementsOSBIOS doesn't even make sense here | |
| 13:42:41 | mdbooth | However, I'm not convinced it's worth waiting, tbh. | |
| 13:43:07 | aspiers | kashyap: It says of "type": "Whether loader is a typical BIOS (rom) or an UEFI binary (pflash). This refers to type attribute of the <loader/> element." | |
| 13:43:17 | mdbooth | It's a pretty clear bug, and the fix doesn't break anything we test, or a real environment. | |
| 13:43:55 | aspiers | kashyap: so it's portraying it as an either/or choice, but the schema places no restriction on the number of <value> elements, and indeed the example even lists *both* rom and pflash | |
| 13:44:23 | aspiers | kashyap: Also, there is no "type attribute of the <loader/> element" | |
| 13:44:45 | aspiers | kashyap: the docs for 'readonly' are even worse | |
| 13:44:56 | kashyap | aspiers: The docs _suck_ in this area | |
| 13:45:10 | aspiers | kashyap: and the example gives both yes and no as simultaneous values ... I mean, WT actual F | |
| 13:45:31 | kashyap | aspiers: On the either/or choice -- yeah, the docs say that, but the damn 'enum' implies there _could_ be more in future. Grrr | |
| 13:46:02 | aspiers | If it is supposed to be a choice between rom and pflash (and maybe others) then it should be <type>rom</type> or <type value="rom"/> | |
| 13:46:22 | aspiers | Likewise <readonly>yes</readonly> or <readonly value="yes"/> etc. | |
| 13:46:45 | kashyap | aspiers: Yeah, the upstream schema as it stands is not really ideal :-( | |
| 13:46:48 | aspiers | but a mapping from 'type' / 'readonly' to a list of values makes no sense | |
| 13:47:58 | aspiers | If multiple concurrent types are really allowed then I could get on board with <types>...</types>, or maybe even just about tolerate <enum name='types'>...</enum> | |
| 13:48:09 | aspiers | but 'type' (singular) rather than 'types' (plural) ... no | |
| 13:49:10 | kashyap | Right, so the "reality" is: it is a binary choice as it stands, not a multiple types at the _same_ time. | |
| 13:49:12 | aspiers | kashyap: Do you know whether rom and pflash are mutually exclusive values? Or can it actually be both at once? | |
| 13:49:21 | aspiers | What is actually valid here? | |
| 13:49:26 | kashyap | aspiers: Let me check with Michal Privoznik | |
| 13:49:33 | kashyap | (libvirt dev) | |
| 13:49:49 | aspiers | kashyap: Thanks. Maybe it's more that the example XML given is wrong | |
| 13:50:18 | aspiers | kashyap: I can imagine that they might have written that to show all the <value>s which are allowed, without realising that putting them all together at once is deeply confusing. | |
| 13:51:02 | stephenfin | mriedem: We don't need to post to the mailing list when we add a parameter to the virt driver interface like that, do we? | |
| 13:51:12 | stephenfin | (the 'power_on' parameter) | |
| 13:51:52 | kashyap | aspiers: I completely agree on the confusion there, libvirt upstream definitely needs to address this. | |
| 13:52:13 | aspiers | kashyap: thanks for the moral support ;-) | |
| 13:52:34 | aspiers | kashyap: I've just pinged Jim Fehlig about it too | |
| 13:53:04 | mriedem | stephenfin: "need" to? no. but i can. | |
| 13:53:13 | artom | stephenfin, depends on who you ask, but it won't hurt | |
| 13:53:21 | kashyap | aspiers: Yeah, it is much needed frustration. As you can see, if we're duking around this damn schema so much, it's clearly not us :D | |
| 13:53:35 | kashyap | s/much needed/understandable/ | |
| 13:53:48 | stephenfin | mriedem: Yeah, I think it would be helpful at least, if you have time | |
| 13:56:06 | mriedem | stephenfin: done | |
| 13:56:11 | stephenfin | ta | |
| 13:58:20 | aspiers | kashyap: Actually this is not just a libvirt issue. The confusion is significantly compounded by placing 'type' and 'readonly' under os.loader.features. | |
| 13:58:43 | aspiers | kashyap: It should be os.loader.type and os.loader.readonly instead. | |
| 13:58:54 | kashyap | aspiers: In the Nova config class? | |
| 13:58:58 | aspiers | kashyap: yes | |
| 13:59:07 | aspiers | 'type' is not a feature | |
| 14:00:10 | aspiers | kashyap: Imagine future code like: if 'rom' in os.loader.features['type']: ... | |
| 14:00:20 | aspiers | kashyap: vs. if os.loader.type == 'rom': | |
| 14:00:46 | aspiers | or: if 'rom' in os.loader.types: ... | |
| 14:01:07 | aspiers | The latter two are instantly understandable; the former is a #wtf moment | |
| 14:02:45 | kashyap | aspiers: Nod. Okay, first step is to untangle this by moving type/readonly under os.loader, rather than os.loader.features | |
| 14:02:47 | aspiers | Public safety warning: I'm probably using overly antagonistic language right now, due to the mood which the ongoing clusterfuck in British politics has just put me into. Sorry in advance for any offense caused. | |
| 14:03:01 | aspiers | kashyap: Yes that would be a very good step I think | |
| 14:03:38 | aspiers | kashyap: and then if multiple types are allowed, we can ignore <type> in the libvirt schema and use os.loader.types (plural) instead | |
| 14:04:03 | kashyap | aspiers: Okay, Michal confirms - rom|pflash _are_ mutually exclusive. (As we guessed) | |
| 14:04:11 | aspiers | Ahah :-) | |
| 14:04:16 | aspiers | So the example XML is wrong | |
| 14:04:27 | aspiers | and hence our test fixtures too | |
| 14:05:41 | aspiers | kashyap: What about 'readonly' and 'secure'? | |
| 14:06:19 | kashyap | mprivozn: Yeah, you can't have secure "yes" and "no" at the same time -- that'll be "Schrödinger's XML"... | |
| 14:06:26 | kashyap | aspiers: They are also mutually exclusive. | |
| 14:06:47 | kashyap | aspiers: Okay, so on the example, this is what Michal says on _why_ both of them are listed: | |
| 14:06:48 | aspiers | kashyap: mprivozn isn't here ;-) wrong window? | |
| 14:07:20 | kashyap | aspiers: Here: | |
| 14:07:21 | kashyap | 16:04 < kashyap> mprivozn: Then, I'll submit a docs change to show just _one_ value under type|readonly|secure: https://libvirt.org/formatdomaincaps.html#elementsOSBIOS | |
| 14:07:24 | kashyap | 16:05 < mprivozn> kashyap: please don't | |
| 14:07:27 | kashyap | 16:05 < kashyap> (Because accepted values are shown in the below text) | |
| 14:07:29 | kashyap | 16:05 < kashyap> mprivozn: Hmm, why not? You wanted them there to show what are the possible values? | |
| 14:07:32 | kashyap | 16:05 < mprivozn> kashyap: the idea for domaincaps is to enumerate all accepted values; listing just one variant goes against that | |
| 14:07:35 | kashyap | 16:06 < mprivozn> kashyap: forget OSBIOS for a second and take a look at some other examples, diskDevice for instance | |
| 14:08:56 | kashyap | aspiers: So following the above example, diskDevice lists 4 different values (disk, cdrom, floppy, lun). | |