*** BjoernT has quit IRC | 00:00 | |
*** BjoernT_ has joined #openstack-nova | 00:00 | |
*** BjoernT_ has quit IRC | 00:01 | |
*** BjoernT has joined #openstack-nova | 00:01 | |
*** BjoernT has quit IRC | 00:02 | |
*** BjoernT_ has joined #openstack-nova | 00:02 | |
*** trident has joined #openstack-nova | 00:03 | |
*** ozzzo has quit IRC | 00:04 | |
*** BjoernT_ has quit IRC | 00:06 | |
*** macz has joined #openstack-nova | 00:10 | |
*** ozzzo has joined #openstack-nova | 00:10 | |
*** macz has quit IRC | 00:37 | |
*** gyee has quit IRC | 00:39 | |
openstackgerrit | Artom Lifshitz proposed openstack/nova master: NUMA live migration support https://review.opendev.org/634606 | 00:40 |
---|---|---|
openstackgerrit | Artom Lifshitz proposed openstack/nova master: Deprecate CONF.workarounds.enable_numa_live_migration https://review.opendev.org/640021 | 00:40 |
openstackgerrit | Artom Lifshitz proposed openstack/nova master: Functional tests for NUMA live migration https://review.opendev.org/672595 | 00:40 |
openstackgerrit | Dustin Cowles proposed openstack/nova master: Provider Config File: YAML file loading and schema validation https://review.opendev.org/673341 | 00:57 |
openstackgerrit | Dustin Cowles proposed openstack/nova master: WIP: Provider Config File: Function to further validate and retrieve configs https://review.opendev.org/676029 | 00:57 |
openstackgerrit | Dustin Cowles proposed openstack/nova master: WIP: Provider Config File: Merge provider configs to provider tree https://review.opendev.org/676522 | 00:57 |
*** tetsuro has joined #openstack-nova | 00:58 | |
*** markvoelker has joined #openstack-nova | 01:00 | |
*** markvoelker has quit IRC | 01:05 | |
*** slaweq has joined #openstack-nova | 01:11 | |
*** slaweq has quit IRC | 01:15 | |
*** kaliya has quit IRC | 01:19 | |
*** rabel has quit IRC | 01:28 | |
*** kaliya has joined #openstack-nova | 01:29 | |
*** rabel has joined #openstack-nova | 01:29 | |
*** markvoelker has joined #openstack-nova | 02:01 | |
*** markvoelker has quit IRC | 02:05 | |
*** tetsuro has quit IRC | 02:18 | |
*** tetsuro has joined #openstack-nova | 02:20 | |
*** kaliya has quit IRC | 02:22 | |
*** rabel has quit IRC | 02:24 | |
*** rabel has joined #openstack-nova | 02:25 | |
*** mkrai has joined #openstack-nova | 02:27 | |
openstackgerrit | Brin Zhang proposed openstack/python-novaclient master: Microversion 2.78: Add delete_on_termination to volume-attach API https://review.opendev.org/673485 | 02:28 |
alex_xu | dansmith: Luyao defined a field type for resource_class https://review.opendev.org/#/c/678448/13/nova/objects/fields.py@94, but I doubt validate the standard rc is right thing. | 02:49 |
*** nicolasbock has quit IRC | 02:49 | |
alex_xu | dansmith: if we are in the middle of upgrade, like Live migraiton, there will be have object transfer between new and old node. so for old node, it may have old os-resource-class lib, so it may not know the new RC, then the deserilzier will blowup. So if we want the validation, I think we only need a copy of generic RC pattern | 02:51 |
alex_xu | https://github.com/openstack/placement/blob/master/placement/schemas/common.py#L19, let me know if it is right | 02:51 |
*** cfriesen has quit IRC | 02:54 | |
*** mdbooth has quit IRC | 02:58 | |
*** bbowen_ has quit IRC | 02:59 | |
*** bbowen_ has joined #openstack-nova | 02:59 | |
openstackgerrit | Merged openstack/nova master: Use microversion in put allocations in test_report_client https://review.opendev.org/679622 | 03:01 |
*** slaweq has joined #openstack-nova | 03:11 | |
*** slaweq has quit IRC | 03:15 | |
*** tetsuro has quit IRC | 03:24 | |
*** mdbooth has joined #openstack-nova | 03:25 | |
openstackgerrit | Brin Zhang proposed openstack/python-novaclient master: Microversion 2.78: Add delete_on_termination to volume-attach API https://review.opendev.org/673485 | 03:30 |
*** markvoelker has joined #openstack-nova | 03:31 | |
*** markvoelker has quit IRC | 03:41 | |
*** tetsuro has joined #openstack-nova | 03:42 | |
*** ricolin has joined #openstack-nova | 03:49 | |
*** mvkr has joined #openstack-nova | 03:51 | |
*** etp has joined #openstack-nova | 04:12 | |
gmann | melwitt: johnthetubaguy alex_xu looking for review/feedback on this first set of policy-defaults-refresh changes. This one and all its base patches - https://review.opendev.org/#/c/648480/ | 04:14 |
alex_xu | gmann: got it | 04:15 |
gmann | alex_xu: thanks. and this nit followup change in spec - https://review.opendev.org/#/c/669196/ | 04:16 |
gmann | alex_xu: this is review guide i sent on ML - http://lists.openstack.org/pipermail/openstack-discuss/2019-August/008504.html | 04:17 |
alex_xu | gmann: got it, I will give it try, let see if i have enough bandwidth | 04:17 |
gmann | alex_xu: thx again , kenichi has +2 on most of them i think | 04:18 |
*** markvoelker has joined #openstack-nova | 04:30 | |
*** markvoelker has quit IRC | 04:35 | |
*** larainema has joined #openstack-nova | 04:48 | |
*** tetsuro has quit IRC | 04:50 | |
*** tetsuro has joined #openstack-nova | 04:52 | |
openstackgerrit | Akira KAMIO proposed openstack/nova master: VMware: disk_io_limits settings are not reflected when resize https://review.opendev.org/680296 | 04:53 |
*** Luzi has joined #openstack-nova | 04:55 | |
*** hoonetorg has quit IRC | 04:56 | |
*** udesale has joined #openstack-nova | 05:02 | |
*** artom has quit IRC | 05:09 | |
*** artom has joined #openstack-nova | 05:09 | |
*** slaweq has joined #openstack-nova | 05:11 | |
*** adriant has quit IRC | 05:11 | |
*** adriant has joined #openstack-nova | 05:12 | |
*** hoonetorg has joined #openstack-nova | 05:13 | |
*** slaweq has quit IRC | 05:15 | |
*** ratailor has joined #openstack-nova | 05:19 | |
*** markvoelker has joined #openstack-nova | 05:30 | |
*** ccamacho has quit IRC | 05:32 | |
*** avolkov has joined #openstack-nova | 05:34 | |
*** etp has quit IRC | 05:34 | |
*** etp has joined #openstack-nova | 05:34 | |
*** markvoelker has quit IRC | 05:35 | |
*** psachin has joined #openstack-nova | 05:37 | |
openstackgerrit | Sundar Nadathur proposed openstack/nova master: ksa auth conf and client for Cyborg access https://review.opendev.org/631242 | 05:40 |
openstackgerrit | Sundar Nadathur proposed openstack/nova master: Add Cyborg device profile groups to request spec. https://review.opendev.org/631243 | 05:41 |
openstackgerrit | Sundar Nadathur proposed openstack/nova master: Create and bind Cyborg ARQs. https://review.opendev.org/631244 | 05:41 |
openstackgerrit | Sundar Nadathur proposed openstack/nova master: Get resolved Cyborg ARQs and add PCI BDFs to VM's domain XML. https://review.opendev.org/631245 | 05:41 |
openstackgerrit | Sundar Nadathur proposed openstack/nova master: Delete ARQs for an instance when the instance is deleted. https://review.opendev.org/673735 | 05:41 |
*** mdbooth has quit IRC | 05:45 | |
openstackgerrit | Brin Zhang proposed openstack/python-novaclient master: Microversion 2.78: Add delete_on_termination to volume-attach API https://review.opendev.org/673485 | 05:53 |
*** mdbooth has joined #openstack-nova | 05:54 | |
*** mdbooth has quit IRC | 06:18 | |
*** mdbooth has joined #openstack-nova | 06:20 | |
*** tetsuro has quit IRC | 06:22 | |
*** slaweq has joined #openstack-nova | 06:23 | |
*** ccamacho has joined #openstack-nova | 06:27 | |
*** slaweq has quit IRC | 06:28 | |
*** psachin has quit IRC | 06:31 | |
*** mmethot_ has joined #openstack-nova | 06:43 | |
*** etp has quit IRC | 06:44 | |
*** etp has joined #openstack-nova | 06:44 | |
*** mmethot has quit IRC | 06:45 | |
openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: Enable driver configuring PMEM namespaces https://review.opendev.org/679640 | 06:46 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: Add functional tests for virtual persistent memory https://review.opendev.org/678470 | 06:46 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: doc: attaching virtual persistent memory to guests https://review.opendev.org/680300 | 06:46 |
*** cfriesen has joined #openstack-nova | 06:50 | |
*** openstackgerrit has quit IRC | 06:51 | |
*** slaweq has joined #openstack-nova | 06:52 | |
*** damien_r has joined #openstack-nova | 06:54 | |
*** tetsuro has joined #openstack-nova | 06:55 | |
*** luksky has joined #openstack-nova | 06:56 | |
*** udesale has quit IRC | 06:56 | |
*** janki has joined #openstack-nova | 06:58 | |
*** tetsuro has quit IRC | 06:59 | |
*** mkrai has quit IRC | 07:00 | |
brinzhang | gmann: takashin: https://review.opendev.org/#/c/673133/15/api-ref/source/os-volume-attachments.inc@46, replace the response of the `GET os-volume_attachments`` or ``SHOW os-volume_attachments`` response body by latest version. | 07:02 |
brinzhang | gmann: takashin: I am not sure is it appropriate. If the microversion less than 2.78, the delete_on_termination field will not contained in the response. | 07:02 |
brinzhang | gmann: takashin:In PS12 I also add the v2.78 response example to that palce, but mriedem let me removed it and just add the os-volume_attachments response to the POST API. https://review.opendev.org/#/c/673133/12/api-ref/source/os-volume-attachments.inc@54 | 07:02 |
*** maciejjozefczyk has joined #openstack-nova | 07:02 | |
*** mkrai has joined #openstack-nova | 07:06 | |
*** markvoelker has joined #openstack-nova | 07:06 | |
*** yan0s has joined #openstack-nova | 07:07 | |
takashin | brinzhang: If the microversion less than 2.78, the delete_on_termination field will not contained in the response. But it is not problem because the parameter description says delete_on_termination has been added since microversion 2.78. | 07:09 |
*** markvoelker has quit IRC | 07:10 | |
*** sapd1_x has quit IRC | 07:10 | |
brinzhang | takashin: thanks, I will replace it by follow-up. | 07:14 |
*** tesseract has joined #openstack-nova | 07:15 | |
*** trident has quit IRC | 07:21 | |
*** cfriesen has quit IRC | 07:22 | |
*** ivve has joined #openstack-nova | 07:27 | |
*** trident has joined #openstack-nova | 07:29 | |
*** threestrands has quit IRC | 07:32 | |
*** rcernin has quit IRC | 07:33 | |
*** trident has quit IRC | 07:34 | |
*** takashin has left #openstack-nova | 07:34 | |
*** takashin has joined #openstack-nova | 07:37 | |
*** trident has joined #openstack-nova | 07:43 | |
*** openstackgerrit has joined #openstack-nova | 07:47 | |
openstackgerrit | Boxiang Zhu proposed openstack/nova master: Preserve UEFI NVRAM variable store https://review.opendev.org/621646 | 07:47 |
*** brault has joined #openstack-nova | 07:50 | |
openstackgerrit | Boxiang Zhu proposed openstack/nova master: Make evacuation respects anti-affinity rule https://review.opendev.org/649963 | 07:58 |
openstackgerrit | Boxiang Zhu proposed openstack/nova master: Fix live migration break group policy simultaneously https://review.opendev.org/651969 | 07:58 |
*** takashin has left #openstack-nova | 08:00 | |
*** tkajinam has quit IRC | 08:05 | |
*** etp has quit IRC | 08:07 | |
*** etp has joined #openstack-nova | 08:07 | |
openstackgerrit | Brin Zhang proposed openstack/python-novaclient master: Microversion 2.78: Add delete_on_termination to volume-attach API https://review.opendev.org/673485 | 08:07 |
*** brault has quit IRC | 08:13 | |
*** ralonsoh has joined #openstack-nova | 08:20 | |
*** cdent has joined #openstack-nova | 08:21 | |
*** etp has quit IRC | 08:30 | |
*** etp has joined #openstack-nova | 08:30 | |
*** derekh has joined #openstack-nova | 08:31 | |
*** cdent has quit IRC | 08:31 | |
*** cdent has joined #openstack-nova | 08:32 | |
openstackgerrit | Yongli He proposed openstack/nova master: Clean up orphan instances virt driver https://review.opendev.org/648912 | 08:34 |
openstackgerrit | Yongli He proposed openstack/nova master: clean up orphan instances https://review.opendev.org/627765 | 08:34 |
*** dtantsur|afk is now known as dtantsur | 08:38 | |
kashyap | aspiers: This is from OpenSUSE, see the maddenning number of OVMF binaries :-( -- http://paste.openstack.org/show/693648/ | 08:41 |
kashyap | aspiers: An impressive 35 | 08:41 |
kashyap | aspiers: Anyway, can you get the same output from SLES? (`rpm -ql qemu-ovmf-x86_64`) | 08:42 |
kashyap | aspiers: But my educated guess is that, these two are the recommended ones for Secure Boot: | 08:44 |
kashyap | /usr/share/qemu/ovmf-x86_64-ms-4m-code.bin | 08:44 |
kashyap | /usr/share/qemu/ovmf-x86_64-ms-4m-vars.bin | 08:44 |
kashyap | (The above are from OpenSUSE. Hopefully they're similarly named on SLES.) | 08:44 |
kashyap | aspiers: In short, for SB, you want the 'ms'-variant '-code.bin' (and its corresponding '-vars.bin') | 08:45 |
*** markvoelker has joined #openstack-nova | 08:45 | |
aspiers | kashyap: it's not 35, you're counting text files and directories in that | 08:46 |
aspiers | kashyap: but still agree that it's way too many | 08:46 |
kashyap | aspiers: Yeah, sorry, exaggerated; do rake me on coals | 08:46 |
aspiers | XD | 08:46 |
aspiers | It's the same on SLES | 08:47 |
*** markvoelker has quit IRC | 08:50 | |
dr_gogeta86 | good morning | 08:52 |
dr_gogeta86 | any hitachi guys here ? | 08:52 |
kashyap | aspiers: Okay, then. Do you have the recorded notes from your previous successful Secure Boot? | 08:58 |
kashyap | aspiers: If so, I'd bet they are the ms-variant binaries. | 08:58 |
aspiers | kashyap: No it was -suse-code | 08:58 |
kashyap | aspiers: Ah, okay. So they are the MS-enrolled ones for SLES | 08:59 |
aspiers | kashyap: But the problem is I'm trying to boot an unreleased test kernel | 09:00 |
aspiers | so it doesn't have the official signature | 09:00 |
kashyap | Nod; for this unrelease kernel, either you just go into the Tianocore BIOS and disable SB | 09:01 |
*** trident has quit IRC | 09:01 | |
aspiers | kashyap: https://en.opensuse.org/openSUSE:UEFI#Booting_a_custom_kernel has been suggested | 09:01 |
kashyap | Yep, those are the steps the Secure Boot / BIOS dev I know recommended too | 09:01 |
kashyap | You have to do a lot of manual muckery, generate certs, run 'mokutil', etc | 09:02 |
aspiers | kashyap: do BIOS definitions get persisted to the VARS file? is that what https://review.opendev.org/#/c/621646/ is about? | 09:03 |
aspiers | kashyap: I'm confused by that commit, because even without it I *already* see the vars files persisting in /var/lib/libvirt/qemu/nvram | 09:04 |
kashyap | aspiers: The VARS file contains anything a user configures. | 09:04 |
aspiers | Oh, that's the bug | 09:04 |
kashyap | aspiers: The context is: just like traditional BIOS, UEFI has menus (just more of them), and needs to store them somewher | 09:04 |
aspiers | "Anything" meaning what though? BIOS settings? anything else? | 09:04 |
kashyap | So, if a user customizes something, it goes into the "VARS" file | 09:05 |
kashyap | aspiers: One of the things it stores is the UEFI keys | 09:06 |
kashyap | aspiers: Yeah, the bug is reinitializing a VARS from the default template, which is is undesirable. | 09:07 |
aspiers | right | 09:08 |
aspiers | so e.g. SB would have to be disabled every boot | 09:09 |
aspiers | there should really also be some way to change the default template | 09:09 |
aspiers | even per image | 09:09 |
*** trident has joined #openstack-nova | 09:09 | |
kashyap | aspiers: In what else way do you want to "change the default (VARS) template"? | 09:10 |
aspiers | e.g. disabling SB | 09:10 |
aspiers | or any other BIOS tweak you can imagine | 09:10 |
kashyap | You don't want to mess with the default template itself, but rather do what libvirt does -- copy it per guest, in the "domain private path", and _then_ do what you want | 09:11 |
kashyap | aspiers: If you _don't_ want SB, then simply use the un-enrolled VARS file. | 09:12 |
aspiers | How? | 09:12 |
aspiers | I guess I didn't explain the use case clearly enough | 09:12 |
aspiers | Let's say I want to boot 100 VMs with SB, and another 100 without SB | 09:12 |
aspiers | How do I do that? | 09:12 |
kashyap | This way: | 09:12 |
kashyap | - For the 100 VMs with SB: When booting the guest, use the OVMF the binary that is built with SB/SMM (i.e. suse-code.bin) | 09:13 |
kashyap | - For the 100 VMs with SB: When booting the guest, use the OVMF the binary that is *not* with SB/SMM (i.e. suse.bin -- or whatever it's called) | 09:14 |
aspiers | But as an unprivileged non-admin how do I choose which OVMF binary gets used? | 09:14 |
kashyap | (Both the above is for UEFI boot, though.) | 09:15 |
kashyap | aspiers: So for a non-admin uesr: | 09:15 |
kashyap | You simply _don't_ set the 'uefi' image property | 09:15 |
aspiers | No, because then SEV breaks | 09:15 |
*** swamireddy has quit IRC | 09:15 | |
kashyap | Ah! We're talking in SEV context | 09:16 |
aspiers | Of course :) | 09:16 |
aspiers | Well, just as an example | 09:16 |
kashyap | Forgot the world spins around SEV for a sec :D | 09:16 |
aspiers | It's one example of a more general issue | 09:16 |
aspiers | What if I want to boot 100 VMs with some UEFI option tweaked and 100 with it not tweaked? It's not always going to be the case that there's a binary offering exactly what I want; there should be an option to create custom VARS templates, and to choose which VARS template to start from | 09:16 |
kashyap | aspiers: Now the question is: why would you want SEV *without* SB? | 09:17 |
aspiers | Well I already gave yesterday's example: testing new unsigned kernels | 09:17 |
aspiers | but please don't get hung up on the SEV part, that's just one example | 09:17 |
kashyap | Sure, sure. | 09:18 |
aspiers | It seems entirely reasonable to assume that at some point hardly anyone will want to boot non-UEFI | 09:18 |
kashyap | aspiers: For enrolling your own VARS files, the process is not "simple" :-( | 09:18 |
aspiers | and even though maybe most people will want SB and other default UEFI options most of the time, I'm sure they will want non-defaults at other times | 09:18 |
kashyap | E.g. to enroll _default_ UEFI (MS) keys (which most distros ship now), we wrote this tool: https://github.com/puiterwijk/qemu-ovmf-secureboot | 09:19 |
aspiers | and at those other times, it will *not* be acceptable to manually go into the console of every single VM at boot-time and press Escape and manually customise things | 09:19 |
aspiers | This is entirely analogous to the need for datacentre operators to tweak legacy BIOS settings over 1000s of machines | 09:19 |
aspiers | At some point around 2002, people got really tired and fed up of having to do that | 09:20 |
aspiers | so the h/w vendors gradually introduced ways of programmatically tweaking the BIOS settings | 09:20 |
aspiers | What we are talking about here is just the 2019 equivalent of that problem | 09:20 |
kashyap | aspiers: Yeah, I completely agree that manually tweaking BIOS settings at boot-time is fugly | 09:20 |
kashyap | (And undesirable) | 09:20 |
aspiers | OK, so do you now see why customisable VARS templates are useful? | 09:20 |
aspiers | or am I missing another way to achieve the same? | 09:21 |
kashyap | aspiers: I see the rationale for customizable VARS templates -- | 09:21 |
kashyap | but for most majority use cases won't need it | 09:22 |
kashyap | aspiers: On any other way -- we can ask Laszlo (OVMF maint) on email | 09:22 |
aspiers | Why would most use cases need it any less than people needed to tweak BIOS settings in 2002? | 09:23 |
*** ociuhandu has joined #openstack-nova | 09:23 | |
aspiers | I would expect more need if anything, since surely UEFI is way more tweakable | 09:23 |
aspiers | although granted I know very little about UEFI | 09:23 |
kashyap | Yeah, I don't claim to be an expert either. | 09:24 |
aspiers | Anyway, that's thinking ahead. So for now it sounds like there is no way to cater for use case #1 - i.e. choosing per SEV image whether to use SB or not | 09:26 |
kashyap | aspiers: Talking to a BIOS dev: | 09:26 |
aspiers | so that makes my life a little harder | 09:26 |
kashyap | 11:24 <kashyap> Do you know of any other way to create customizable VARS files? | 09:26 |
kashyap | 11:24 <puiterwijk> There's no way, just boot and enter the tianocore bios setup | 09:26 |
aspiers | kashyap: Sure, *creation* is the easy part. It's the *reuse* which I'm talking about. | 09:27 |
aspiers | Creation of a VARS template only has to be done once, so manual is fine | 09:27 |
aspiers | Reuse across 500 VMs is different, however | 09:27 |
aspiers | Eventually I could imagine uploading VARS template files to (say) glance, and choosing which to use at boot-time via the nova API | 09:28 |
aspiers | Maybe my use case (testing SEV+SB with stable *and* bleeding edge kernels) is the only use case the world will ever encounter, but that seems unlikely to me. | 09:29 |
*** puiterwijk has joined #openstack-nova | 09:29 | |
aspiers | Well, the "SEV" component can be dropped from that immediately and the use case is still clearly relevant for kernel devs who care about UEFI and SB | 09:30 |
kashyap | aspiers: 1 sec | 09:30 |
kashyap | aspiers: I invited the expert to this channel, Patrick (puiterwijk) -- thanks for joining | 09:31 |
aspiers | Hi puiterwijk :) | 09:31 |
puiterwijk | aspiers: hi | 09:31 |
puiterwijk | the problem is that the VARS file format is internal to TianoCore itself, and there's (to my knowledge) no API guarantee on it, nor an external library to manipulate them. That is also why the enroll script I hacked together just boots an EFI binary that enrolls the keys. And the other one I wrote literally just automates stepping through the BIOS menus | 09:32 |
aspiers | puiterwijk: If you want you can quickly catch up via the bottom of http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-09-05.log.html | 09:32 |
* puiterwijk goes read that | 09:32 | |
aspiers | puiterwijk: Creation of template VARS files is not the hard part here, since that only needs to be done once per template, so doing it manually is fine. | 09:33 |
aspiers | Reuse across 500 VMs is different, however. | 09:33 |
puiterwijk | Ah, so what you want is a way to tell nova to use a different vars file template? | 09:33 |
aspiers | Yes | 09:33 |
aspiers | Maybe I'm way off here (since I don't know much about UEFI in general) but I would expect that to be useful in the future as use of UEFI gets more sophisticated. What do you think? | 09:34 |
puiterwijk | Ahhh, I see. Okay, then I retract my comments on how that's hard to automate :). And I think that, based on the questions I got from kashyap, he might also have misunderstood that that was an option | 09:34 |
puiterwijk | So, the one question I'd have is which BIOS settings you want to tweak regularly? | 09:35 |
aspiers | In the legacy BIOS world, there are loads | 09:35 |
puiterwijk | Since based on the settings available in tianocore, I don't think there's that many that are interesting that you can't tweak from external. | 09:35 |
kashyap | (Hmm, in the scrollback I did mention that using different VARS files _should_ be possible...) | 09:35 |
puiterwijk | kashyap: ah, okay, sorry. I did not read the full backlog yet. | 09:36 |
aspiers | Let me look at tianocore settings quickly | 09:36 |
kashyap | No problem; don't expect you to read everything I ramble :D | 09:36 |
aspiers | I've never looked at them before | 09:36 |
puiterwijk | aspiers: so, the ones I'd think are most often changed would be the secureboot settings, and the boot order. And the latter you can tweak directly from qemu | 09:36 |
puiterwijk | (but sure, there's a bunch of other settings, and there's probably ones I'm just not thinking of right now) | 09:37 |
kashyap | aspiers: puiterwijk: Oh, something interesting here: https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/firmware.json | 09:40 |
puiterwijk | So, I do wnat to make one clarification though that I do see from skimming the backlog: I think we'd always want to use the OVMF *binarY* that has secureboot capabilities compiled in, but that we'd want to change the OVMF *template* (i.e. settings) | 09:40 |
aspiers | makes sense | 09:40 |
kashyap | aspiers: So in the firmware descriptor files, there's also an "amd-sev" feature represented: | 09:40 |
kashyap | 112 # @amd-sev: The firmware supports running under AMD Secure Encrypted | 09:41 |
kashyap | 113 # Virtualization, as specified in the AMD64 Architecture | 09:41 |
kashyap | 114 # Programmer's Manual. QEMU command line options related to | 09:41 |
kashyap | 115 # this feature are documented in | 09:41 |
kashyap | 116 # "docs/amd-memory-encryption.txt". | 09:41 |
kashyap | (And libvirt parses this, IIRC) | 09:41 |
* kashyap --> tea; bbiab | 09:42 | |
*** etp has quit IRC | 09:45 | |
puiterwijk | So, on the multiple vars files: yes, I agree that it would probably be useful to be able to ask nova for a specific vars file to be used. But maybe do require that vars file to be put in place and registered by an admin. | 09:45 |
*** etp has joined #openstack-nova | 09:45 | |
aspiers | right | 09:45 |
puiterwijk | Because even if you only have the secureboot settings, you might want a vars file that has your own custom keys enrolled | 09:45 |
aspiers | exactly | 09:45 |
aspiers | it could be an extra spec on the flavor | 09:45 |
aspiers | hw_uefi_vars or something | 09:46 |
puiterwijk | Yeah, agreed that that sounds decent | 09:46 |
aspiers | kashyap: is it worth adding this to http://specs.openstack.org/openstack/nova-specs/specs/train/approved/allow-secure-boot-for-qemu-kvm-guests.html ? | 09:47 |
puiterwijk | (do note that I'm not too deeply integrated into how Openstack (Nova) do things, other than mostly operational and fixing my own bugs, so it might be that this is not in line iwth their design goals) | 09:47 |
aspiers | puiterwijk: Yeah I'm not sure about this either but I think it's worth suggesting at least | 09:47 |
puiterwijk | Yep, agreed. | 09:47 |
aspiers | OK I just managed to fix my broken devstack so maybe I can finally look inside tiano | 09:49 |
kashyap | puiterwijk: aspiers: Hmm, 'hw_uefi_vars' idea does sound reasonable -- but would need to tackle that _after_ adding initial SB support in the spec | 09:49 |
aspiers | kashyap: sure | 09:49 |
puiterwijk | Sure, that sounds fine | 09:49 |
*** mdbooth has quit IRC | 09:49 | |
kashyap | aspiers: And yeah, it does belong to that spec. Should we "extend" the spec later? Or have an additional BP for this? | 09:50 |
*** mdbooth has joined #openstack-nova | 09:51 | |
aspiers | kashyap: Very good questions... | 09:51 |
kashyap | Probably something additional on top will be simpler process-wise. | 09:52 |
puiterwijk | The problem with adding it on top is: how do the two work together then? | 09:52 |
puiterwijk | i.e. if I request secureboot per the current blueprint, and then in the later one I ask for another firmware that doesn't have secureboot? | 09:52 |
kashyap | Heh | 09:53 |
puiterwijk | (My suggestion would be "if the requested vars isn't secureboot, and secureboot is asked, return error due to invalid config") | 09:53 |
puiterwijk | or invalid request* | 09:53 |
kashyap | puiterwijk: I mean, they're _related_ topics, and will be designed to work together | 09:53 |
aspiers | kashyap: I think it might work as a flavor extra spec | 09:54 |
aspiers | so the admin controls exactly which templates are available | 09:54 |
puiterwijk | kashyap: sure. I'm just thinking of what would need to be thought about if talking about a new BP :) | 09:54 |
kashyap | puiterwijk: Ah, nod. | 09:54 |
aspiers | How do I get into the TianoCore settings? Escape doesn't work, goes straight to asking for the encrypted disk password | 09:54 |
puiterwijk | aspiers: boot without working boot options | 09:55 |
puiterwijk | i.e. detach the disk | 09:55 |
aspiers | Erm, not sure I can do that in nova | 09:55 |
puiterwijk | Oh, inside nova... | 09:55 |
aspiers | It's not a volume | 09:55 |
puiterwijk | Yeah, I'm not sure either. | 09:55 |
aspiers | I guess maybe boot from volume | 09:55 |
aspiers | kashyap: ideas? | 09:55 |
puiterwijk | aspiers: so... I know that in the Fedora grub, we have an option to "go to system setup" | 09:56 |
puiterwijk | (because "go to setup" is available as an UEFI call) | 09:56 |
aspiers | I can get to the grub commandline | 09:56 |
aspiers | and I can do chainloader (hd0,gpt2)/efi/... | 09:56 |
puiterwijk | I don't know the grub cmdline for that option right now, let me see in my config | 09:56 |
aspiers | I managed to get into mokmanager that way | 09:57 |
aspiers | or whatever it's called | 09:57 |
puiterwijk | Oh | 09:57 |
puiterwijk | "fwsetup" | 09:57 |
puiterwijk | that's the grub command | 09:57 |
puiterwijk | menuentry 'System setup' $menuentry_id_option 'uefi-firmware' { | 09:57 |
puiterwijk | fwsetup | 09:57 |
puiterwijk | } | 09:57 |
stephenfin | melwitt: Heh, sorry. That auto-topic thing was killing me downstream and like mriedem I always do 'git checkout -b bug/NNN' (or 'bp/foo') manually first. Also, it was broken with storyboard | 09:57 |
aspiers | IRC is awesome for apparent non-sequiturs | 09:58 |
stephenfin | melwitt: mordred reviewed it though so if you _really_ want to blame someone, I suggest blaming him. Everyone else does it :P | 09:58 |
aspiers | morning stephenfin | 09:58 |
aspiers | ;-p | 09:58 |
stephenfin | yo | 09:58 |
aspiers | puiterwijk: fwsetup is accepted but does nothing | 09:58 |
* stephenfin is in Dublin today, where the amount of cranes and general building sites has reached insane levels | 09:59 | |
kashyap | aspiers: So you got into Grub command-line... | 09:59 |
stephenfin | I can see four new towers going up just from our window | 09:59 |
aspiers | kashyap: yes | 09:59 |
aspiers | stephenfin: try London for a bit | 09:59 |
stephenfin | once that legislation is passed, sure :) | 10:00 |
aspiers | I get "error: you need to load the kernel first." | 10:00 |
stephenfin | til then, I'll stay where food supplies aren't a concern | 10:00 |
aspiers | :'( | 10:00 |
aspiers | entirely understandable | 10:01 |
* aspiers got his Irish passport recently just in case | 10:01 | |
aspiers | well, more on principle | 10:01 |
aspiers | no a-hole is going to take away my EU membership | 10:01 |
*** tetsuro has joined #openstack-nova | 10:04 | |
*** tetsuro has quit IRC | 10:08 | |
*** luksky has quit IRC | 10:10 | |
sean-k-mooney | i think its funny that they keep talking about people in northerin ireland losing ther eu citizenship give that almost everyone born there is entitled to irish citizen ship grantign them full eu citizenship too | 10:11 |
aspiers | Yeah they have other things to worry about but not that | 10:12 |
sean-k-mooney | well not funny in the convnetional sense more tragic that politions in london dont know thats a thing | 10:12 |
aspiers | half our politicians are totally clueless | 10:12 |
aspiers | let's not drag this channel down that rathole though | 10:13 |
sean-k-mooney | quite so | 10:13 |
cdent | half? | 10:13 |
sean-k-mooney | its stressful enough watching the news | 10:13 |
aspiers | cdent: probably more, I was being generous off the back of yesterday | 10:14 |
cdent | somebody else was trying to tell me that yesterday was a spark of hope, but I'm not sure. But I agree, turning an openstack channel into the brexit-therapy-network is probably a misuse | 10:15 |
aspiers | it's definitely a spark of hope | 10:15 |
aspiers | not sure how big but we'll see | 10:15 |
openstackgerrit | sean mooney proposed openstack/nova master: multi numa nfv testing job https://review.opendev.org/679656 | 10:19 |
*** udesale has joined #openstack-nova | 10:20 | |
aspiers | kashyap, sean-k-mooney: turns out that my new q35 patch introduced a circular import loop https://review.opendev.org/#/c/680065/ | 10:24 |
aspiers | so I'm trying to figure out the best way of moving code around | 10:24 |
kashyap | aspiers: Did it? Will look in a bit; buried in untangling something :-( | 10:24 |
*** tetsuro has joined #openstack-nova | 10:24 | |
aspiers | see discussion with efried last night http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-09-04.log.html#t2019-09-04T20:57:14 | 10:25 |
sean-k-mooney | aspiers: between what | 10:25 |
aspiers | between virt.hardware and libvirt.utils | 10:25 |
aspiers | and also libvirt.driver in between which imports libvirt.utils | 10:25 |
sean-k-mooney | right virt.hardware cant call the libvirt code | 10:25 |
aspiers | so there are some options for how to fix this | 10:26 |
aspiers | hmm, need a cloud-based sequence diagram editor now to help discuss this | 10:26 |
sean-k-mooney | i see what your importing | 10:27 |
aspiers | sean-k-mooney: it affects the next apply-config patch too | 10:27 |
aspiers | one option is to move sev_enabled from utils to hardware | 10:27 |
sean-k-mooney | actully libvit.utils does not use teh hardware module | 10:28 |
sean-k-mooney | so you shoudl be able to import that form hardware.py | 10:28 |
*** udesale has quit IRC | 10:28 | |
sean-k-mooney | aspiers: well they should be in hardware.py anyway | 10:28 |
aspiers | sean-k-mooney: https://review.opendev.org/#/c/644565/51/nova/virt/libvirt/utils.py@38 | 10:28 |
*** udesale has joined #openstack-nova | 10:29 | |
sean-k-mooney | oh you adding it in another patch | 10:29 |
aspiers | another option is a new machine_types.py | 10:29 |
aspiers | a third option efried suggested is a new virt.libvirt.hardware | 10:29 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: object: Introduce Resource and ResourceList objs https://review.opendev.org/678448 | 10:30 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: Add resources dict into _Provider https://review.opendev.org/678449 | 10:30 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: Retrieve the allocations early https://review.opendev.org/678450 | 10:30 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: Claim resources in resource tracker https://review.opendev.org/678452 | 10:30 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: Enable driver discovering PMEM namespaces https://review.opendev.org/678453 | 10:30 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: report VPMEM resources by provider tree https://review.opendev.org/678454 | 10:30 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: Support VM creation with vpmems and vpmems cleanup https://review.opendev.org/678455 | 10:30 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: Parse vpmem related flavor extra spec https://review.opendev.org/678456 | 10:30 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: Enable driver configuring PMEM namespaces https://review.opendev.org/679640 | 10:30 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: Add functional tests for virtual persistent memory https://review.opendev.org/678470 | 10:30 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: doc: attaching virtual persistent memory to guests https://review.opendev.org/680300 | 10:30 |
aspiers | but I was struggling to figure out whether SEV is libvirt specific. At least machine types are QEMU-specific | 10:30 |
sean-k-mooney | sev is tecnically not | 10:30 |
sean-k-mooney | but the way you are checking for it is | 10:30 |
kashyap | aspiers: SEV is hardware-specific; machine types indeed are not "libvirt | 10:30 |
kashyap | ... -specific" | 10:31 |
kashyap | As you discovered (read the chat). | 10:31 |
kashyap | But they're typically handled by libvirt for Nova. | 10:31 |
aspiers | Right | 10:31 |
aspiers | The problem is that get_mem_encryption_constraint() and its helpers are tied to virt.hardware | 10:32 |
sean-k-mooney | aspiers: move sev_enabled out of utils into the driver | 10:33 |
aspiers | because they rely on _get_flavor_image_meta() in that file, and so do other functions there | 10:33 |
aspiers | sean-k-mooney: originally sev_enabled had to be outside driver.py | 10:34 |
aspiers | so it could be called from vif.py | 10:34 |
*** mdbooth has quit IRC | 10:34 | |
aspiers | but now I moved the iommu stuff into designer.py | 10:34 |
aspiers | so maybe I could move it back | 10:34 |
*** mdbooth has joined #openstack-nova | 10:34 | |
sean-k-mooney | all calls from vif.py are form the driver | 10:34 |
sean-k-mooney | so you can pass it in a flag if its needed | 10:34 |
aspiers | that wasn't easily doable before | 10:34 |
sean-k-mooney | but we dont pas the libvirt host object to any other funciton in utils | 10:35 |
aspiers | but now it's not needed anyway | 10:35 |
sean-k-mooney | the utils dont actully talk to libvirt as far as i can see | 10:35 |
aspiers | right | 10:35 |
sean-k-mooney | so the sev_enabled check does not fit with the rest of the module | 10:35 |
sean-k-mooney | so ya simple fix is just make it a member funciton of the driver then you can do self.host and just pass in the flavor and image | 10:36 |
aspiers | I don't think this is sufficient | 10:37 |
aspiers | how would https://review.opendev.org/#/c/644565/51/nova/virt/libvirt/utils.py@38 change? | 10:37 |
aspiers | oh sorry | 10:37 |
sean-k-mooney | i will remove the only use of hardware.py form the utils | 10:37 |
aspiers | wrong link | 10:37 |
aspiers | how would https://review.opendev.org/#/c/680065/4/nova/virt/hardware.py change? | 10:38 |
sean-k-mooney | that would not | 10:38 |
aspiers | but that already causes a cycle | 10:38 |
sean-k-mooney | your only calling libvirt_utils.get_machine_type(image_meta) | 10:38 |
*** mdbooth has quit IRC | 10:38 | |
sean-k-mooney | how | 10:38 |
aspiers | look at the CI failures :) | 10:38 |
sean-k-mooney | hardware.py will import utils but utils wont import it | 10:39 |
sean-k-mooney | so if you remove the change in https://review.opendev.org/#/c/644565/51/nova/virt/libvirt/utils.py@38 | 10:39 |
sean-k-mooney | its not an issue right | 10:39 |
aspiers | ohhh OK those CI failures on the first patch are something else | 10:40 |
*** mdbooth has joined #openstack-nova | 10:40 | |
aspiers | I thought I saw circular import errors in both patches but I was wrong | 10:40 |
aspiers | cool | 10:40 |
*** nicolasbock has joined #openstack-nova | 10:41 | |
sean-k-mooney | there is another way to break circual dependcies in python but moveing htat function is the simplest | 10:42 |
aspiers | which one? | 10:42 |
aspiers | sev_enabled? | 10:42 |
sean-k-mooney | yes | 10:42 |
sean-k-mooney | moving it to the driver | 10:42 |
aspiers | you mentioned another function in utils you are gonna move? | 10:42 |
aspiers | which one? | 10:42 |
sean-k-mooney | no you dont need to move anything else | 10:42 |
aspiers | oh that's the only one OK cool | 10:43 |
sean-k-mooney | just the new funtion your intoducing | 10:43 |
aspiers | so move that to libvirt/driver.py | 10:43 |
aspiers | where it was originally :) | 10:43 |
sean-k-mooney | ya | 10:43 |
aspiers | cool | 10:43 |
aspiers | using designer.py made everything a lot cleaner | 10:43 |
aspiers | this is another example of that | 10:43 |
aspiers | thanks a lot! | 10:43 |
sean-k-mooney | we want to kill desigenr.py | 10:43 |
yonglihe | sean-k-mooney, stephenfin: good morning, guys. | 10:43 |
aspiers | my brain was hurting from thinking about this | 10:43 |
sean-k-mooney | or at least we did | 10:43 |
aspiers | sean-k-mooney: there should be a list of tech debt goals somewhere public | 10:44 |
sean-k-mooney | yonglihe: hi | 10:44 |
aspiers | sean-k-mooney: e.g. storyboard | 10:44 |
aspiers | maybe it's on an etherpad somewhere | 10:44 |
sean-k-mooney | aspiers: i was going to say etherpad would be good. | 10:44 |
aspiers | WFM too | 10:44 |
aspiers | OK, gonna take a break | 10:44 |
sean-k-mooney | aspiers: the designer.py was intoduced by danpb to breakout some fo the code out of the driver but it was started and never finish and became techdebt | 10:45 |
sean-k-mooney | really we shoudl merge it into the config.py | 10:45 |
aspiers | I disagree | 10:45 |
*** bbowen_ has quit IRC | 10:45 | |
sean-k-mooney | its basically doing the same thing | 10:45 |
aspiers | I think the scoping text at the top of config.py is good | 10:46 |
*** sapd1_x has joined #openstack-nova | 10:46 | |
sean-k-mooney | well i gues | 10:46 |
aspiers | it's good to keep logic out of config,py | 10:46 |
sean-k-mooney | config.py is giving you the datamodels | 10:46 |
aspiers | just keep it parsing/generating XML only | 10:46 |
sean-k-mooney | and serialistaion code | 10:46 |
sean-k-mooney | and the designer is wiring up the elements | 10:46 |
*** markvoelker has joined #openstack-nova | 10:46 | |
sean-k-mooney | but the problem is so is the driver.py | 10:46 |
sean-k-mooney | so without moveing more of the xml logic out of the dirver and into the desinger among other things its kind of messey to have it in two places | 10:47 |
sean-k-mooney | yonglihe: o/ | 10:47 |
*** tbachman has quit IRC | 10:49 | |
sean-k-mooney | yonglihe: i see stephenfin has +2'd the toplogy api seires. it should merge soon | 10:50 |
sean-k-mooney | yonglihe: that just leaves your orpahn vm series right | 10:50 |
openstackgerrit | Adam Spiers proposed openstack/nova master: Ensure q35 machine type is used when booting with SEV https://review.opendev.org/680065 | 10:51 |
openstackgerrit | Adam Spiers proposed openstack/nova master: Ensure q35 machine type is used when booting with SEV https://review.opendev.org/680065 | 10:51 |
*** markvoelker has quit IRC | 10:52 | |
yonglihe | sure, i notice that. thank you guys. That orphan is real orphan now. -:) | 10:52 |
sean-k-mooney | ill try and check it out again this week. | 10:52 |
yonglihe | That's great ! | 10:53 |
yonglihe | Pile of patches recently. | 10:53 |
sean-k-mooney | well code freeze is in a week so it always gets hechtic at this time | 10:54 |
yonglihe | deja vu | 10:55 |
*** luksky has joined #openstack-nova | 10:58 | |
*** sapd1_x has quit IRC | 11:07 | |
*** tetsuro has quit IRC | 11:07 | |
stephenfin | yaawang, alex_xu: I'm going to address my nits in the docs for https://review.opendev.org/#/c/670298/ and then fast approve, if that's okay. I'd like to see a follow-up for the non-doc fixes though | 11:12 |
aspiers | sean-k-mooney: https://review.opendev.org/#/c/680065/6/nova/virt/hardware.py@30 still doesn't work because that imports libvirt.driver which imports virt.hardware | 11:15 |
aspiers | kashyap: ^^^ | 11:15 |
kashyap | Hmm | 11:16 |
openstackgerrit | wangfengsheng proposed openstack/nova master: test https://review.opendev.org/680372 | 11:16 |
kashyap | aspiers: Is it simpler if you just put it in utils? | 11:16 |
aspiers | kashyap: http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-09-05.log.html#t2019-09-05T10:32:41 | 11:20 |
*** Sundar has joined #openstack-nova | 11:20 | |
kashyap | Ah, "The problem is that get_mem_encryption_constraint() and its helpers are tied to virt.hardware" | 11:21 |
aspiers | yes, keep reading | 11:21 |
*** mdbooth has quit IRC | 11:21 | |
*** mdbooth has joined #openstack-nova | 11:22 | |
* kashyap does | 11:22 | |
alex_xu | stephenfin: cool | 11:23 |
*** ociuhandu has quit IRC | 11:23 | |
cdent | aspiers, sean-k-mooney : https://twitter.com/richard_parris/status/1169414190093627392 | 11:24 |
aspiers | cdent: heh that's good | 11:25 |
* kashyap 's head is spinning with all these circular deps | 11:26 | |
*** lpetrut has joined #openstack-nova | 11:27 | |
aspiers | mine too | 11:28 |
aspiers | argh | 11:28 |
Sundar | Hi gibi, I resubmitted the Cyborg interaction patch series [1] with the updated notification structure. Please review it when you can. [1] https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/nova-cyborg-interaction | 11:28 |
* sean-k-mooney im glad i was not drinking coffee when i clicked that | 11:29 | |
aspiers | sean-k-mooney: :) | 11:30 |
kashyap | aspiers: Sorry for no bright ideas yet; I'm mired in mutliple tasks (multitasking--); now for lunch | 11:33 |
artom | aspiers, so ideally you'd want to fail early at the compute API level, around the same place _validate_flavor_image_mem_encryption() is called | 11:33 |
artom | The problem is that we don't want to have driver-specific calls in there (libvirt_utils.get_machine_type) | 11:34 |
Sundar | sean-k-mooney: How are you? | 11:34 |
aspiers | artom: That's already happening; the challenge is getting import dependencies right | 11:34 |
aspiers | artom: Yeah that's a good point, maybe the crux of the problem | 11:34 |
artom | aspiers, right, sorry, you're calling your thing from get_mem_encryption_constraint | 11:34 |
aspiers | Well you're right that we're calling driver-specific stuff from the API level | 11:35 |
artom | But if we move to fail within libvirt, which is where the machine type stuff belongs, it's super late (around https://review.opendev.org/#/c/644565/51/nova/virt/libvirt/driver.py@4931 presumably) | 11:35 |
sean-k-mooney | Sundar: hi. im ok i did not sleef last night but other then that im good | 11:35 |
sean-k-mooney | Sundar: how is the cyborg integration comming | 11:36 |
aspiers | artom: exactly | 11:36 |
aspiers | artom: so how to achieve https://review.opendev.org/#/c/680065/6//COMMIT_MSG ? | 11:36 |
artom | aspiers, I don't want to over design, but maybe drivers need a validate_image_props() or something | 11:36 |
aspiers | urgh | 11:36 |
artom | I know :( | 11:37 |
aspiers | a cloud suddenly passed over the chances of landing SEV before feature freeze :-( | 11:37 |
Sundar | sean-k-mooney: The integ is progressing. When you tried to deploy Cyborg, perhaps you may have grappled with which patch series to deploy. Many of them have merged now. If you have the time, I can explain further. | 11:37 |
artom | That's what we get for exposing driver-specific stuff in what's essentially our API | 11:37 |
aspiers | artom: you referring to stuff like hw_machine_type extra spec? | 11:38 |
aspiers | I don't see how that could be avoided | 11:38 |
Sundar | sean-k-mooney: Hope Cyborg is not what's keeping you up ;) | 11:38 |
sean-k-mooney | Sundar: i ended up spending the last few days teting artoms numa live migration patch. | 11:38 |
Sundar | Ah, I see | 11:38 |
sean-k-mooney | Sundar: no i set up a clean vm to test cyborg in with centos 7 as you suggeted | 11:38 |
sean-k-mooney | but i did not get time to actully execute devstack and deploy it | 11:39 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: object: Introduce Resource and ResourceList objs https://review.opendev.org/678448 | 11:39 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: Add resources dict into _Provider https://review.opendev.org/678449 | 11:39 |
artom | aspiers, yes to both points :P | 11:39 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: Retrieve the allocations early https://review.opendev.org/678450 | 11:39 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: Claim resources in resource tracker https://review.opendev.org/678452 | 11:39 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: Enable driver discovering PMEM namespaces https://review.opendev.org/678453 | 11:39 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: report VPMEM resources by provider tree https://review.opendev.org/678454 | 11:39 |
*** ociuhandu has joined #openstack-nova | 11:39 | |
openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: Support VM creation with vpmems and vpmems cleanup https://review.opendev.org/678455 | 11:39 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: Parse vpmem related flavor extra spec https://review.opendev.org/678456 | 11:39 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: libvirt: Enable driver configuring PMEM namespaces https://review.opendev.org/679640 | 11:39 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: Add functional tests for virtual persistent memory https://review.opendev.org/678470 | 11:39 |
openstackgerrit | Luyao Zhong proposed openstack/nova master: doc: attaching virtual persistent memory to guests https://review.opendev.org/680300 | 11:39 |
aspiers | artom: well it wouldn't be too hard to add validate_flavor_image() or whatever to the driver interface I guess? | 11:40 |
Sundar | sean-k-mooney: There was an issue that Cyborg's notificaiton event would come too soon before the n-cpu has begun to wait, so the event was getting dropped. I believe I have fixed it now. The crux of it is to do it in _build_resources, and to launch the wait before calling Cyborg for the bind, so that there is no window in between. | 11:40 |
aspiers | artom: it could just be a nop in the base class | 11:40 |
*** ociuhandu has quit IRC | 11:40 | |
aspiers | artom: or even individual noops in each driver except libvirt | 11:40 |
Sundar | sean-k-mooney: In the patch series https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/nova-cyborg-interaction, the notification is in 'Create/bind ARQs' patch https://review.opendev.org/631244 | 11:41 |
aspiers | efried: seems there is a substantial flaw in our idea of checking for hw_machine_type=q35 at API-level, since it's driver-specific ... | 11:41 |
*** ociuhandu has joined #openstack-nova | 11:41 | |
*** ociuhandu has quit IRC | 11:41 | |
aspiers | efried: see above discussion with artom ^^^ | 11:41 |
Sundar | The lvirt driver merely queries Cyborg to get the ARQs, relyung upon _build_resources to have done the wait. | 11:41 |
aspiers | artom: can you think of any other way around this? | 11:42 |
stephenfin | yaawang, alex_xu: Actually, there's an issue with the second patch in the series so I'll leave the release note rework to yaawang :) | 11:42 |
*** ociuhandu has joined #openstack-nova | 11:42 | |
aspiers | artom: the alternative would be to not check at API-level and then it would try and fail on every compute host, right? | 11:42 |
sean-k-mooney | Sundar: ok | 11:42 |
alex_xu | stephenfin: hah | 11:42 |
sean-k-mooney | Sundar: we treat missign the event as fatal uncondtionally correct | 11:43 |
artom | aspiers, or just move get_machine_type out of libvirt utils | 11:43 |
artom | Well | 11:43 |
*** ociuhandu has quit IRC | 11:43 | |
artom | No, because it depends on the default machine type if nothing is set | 11:43 |
artom | I'll drive my kids to school/daycare and think about it | 11:44 |
aspiers | artom: previous version did not check for q35 when SEV is enabled but instead overrode the default to q35 | 11:44 |
sean-k-mooney | Sundar: i dont see any depends on links in teh nova patches to point to cyborg | 11:44 |
aspiers | artom: it also overrode machine type even if it was set on the image | 11:44 |
*** ociuhandu has joined #openstack-nova | 11:44 | |
sean-k-mooney | Sundar: has all the cyborg code merged? | 11:44 |
sean-k-mooney | Sundar: if not we should add it to the first patch that calls cyborg directly | 11:45 |
sean-k-mooney | Sundar: which i think would be https://review.opendev.org/#/c/631244/38 | 11:45 |
sean-k-mooney | Sundar: by the way can we abandon https://review.opendev.org/#/q/topic:bp/nova-cyborg-interaction+(status:open+OR+status:merged)+project:opendev/sandbox | 11:47 |
Sundar | sean-k-mooney: Good point, I'll add the dependencies. Some Cyborg patches are close to merging, but the patches together are testable. The tempest scenarios are possible with one exception of device profile deletion in some circumstances. But that doesn't affect VM launch. | 11:48 |
artom | aspiers, overriding isn't cool either | 11:48 |
sean-k-mooney | Sundar: ok it would be good to ad a testing patch that runs the tempst job on nova | 11:48 |
artom | aspiers, validate_flavor_image() can't be in the driver itself because we don't want to call to the individual driver instance | 11:48 |
artom | Which is why get_machine_type is in libvirt_utils | 11:49 |
sean-k-mooney | Sundar: do you mind if i abandon the two patches against the sandbox repo to clean up the gerrit topic | 11:49 |
artom | Because it's libvirt'y, but doesn't depend on a specific host | 11:49 |
artom | aspiers, if you can find a way to abstract that ^^ for all drivers, I think it'd be the best solution | 11:49 |
*** ociuhandu has quit IRC | 11:50 | |
sean-k-mooney | artom: the validate flavor stuff belongs in hardware.py | 11:50 |
Sundar | sean-k-mooney: I wasn;t even aware of these patches! They seem very preliminary. I don't know the author. But, IMHO, they can be abandoned. | 11:50 |
sean-k-mooney | Sundar: the sandbox repo is the one for teaching people how to use gerrit | 11:50 |
sean-k-mooney | so they are not but they have the topic which is confusing | 11:51 |
sean-k-mooney | ill close them so | 11:51 |
Sundar | sean-k-mooney: Re. "testing patch that runs the tempst job on nova", I believe that would be https://review.opendev.org/#/c/670999/ | 11:51 |
sean-k-mooney | given they are 6 mohts old | 11:51 |
artom | sean-k-mooney, yeah but aspiers has a problem with circular dependencies | 11:51 |
sean-k-mooney | artom: not any more | 11:51 |
aspiers | artom, sean-k-mooney: so the suggestion is a common validation interface across all drivers? | 11:51 |
aspiers | sean-k-mooney: no I do | 11:51 |
aspiers | sean-k-mooney: I was wrong (again) | 11:51 |
aspiers | sean-k-mooney: https://review.opendev.org/#/c/680065/ has circular dependencies even before sev_enabled is added | 11:52 |
sean-k-mooney | hardware specifc tratit valdiation lives in hardare.py | 11:52 |
luyao | stephenfin: comments addressed , and thanks for your review. https://review.opendev.org/#/q/topic:bp/virtual-persistent-memory+(status:open) | 11:52 |
aspiers | sean-k-mooney, artom: http://paste.openstack.org/show/771288/ | 11:53 |
aspiers | sean-k-mooney: so unfortunately your suggestion wasn't enough to solve it | 11:53 |
sean-k-mooney | s/triatis/flavor and image/ its where all the numa, pinning, hugepages and pci checks live | 11:53 |
aspiers | sean-k-mooney: right but until now it never cared about machine types | 11:54 |
aspiers | maybe moving get_machine_type to a separate class is the only way | 11:54 |
aspiers | s/class/file/ | 11:54 |
sean-k-mooney | no that wont work | 11:55 |
aspiers | right it won't | 11:55 |
aspiers | well, not unless it is moved outside nova.virt.libvirt | 11:55 |
sean-k-mooney | it because importing nova/virt/libvirt/__init__.py has sideffects | 11:55 |
aspiers | yes | 11:55 |
sean-k-mooney | we should remove this https://github.com/openstack/nova/blob/master/nova/virt/libvirt/__init__.py#L17 | 11:55 |
aspiers | that sounds good | 11:55 |
aspiers | why is it even there? | 11:56 |
sean-k-mooney | we proably moved things and that was a quick hack | 11:56 |
aspiers | OK so first I have to get rid of that | 11:56 |
aspiers | sigh | 11:56 |
sean-k-mooney | remind me why you need to call the fucntion to get the machine type | 11:58 |
sean-k-mooney | i know its for suspend and live migate | 11:58 |
aspiers | no | 11:58 |
aspiers | it's to check for q35 | 11:58 |
sean-k-mooney | yes | 11:58 |
aspiers | https://review.opendev.org/#/c/680065/6//COMMIT_MSG | 11:58 |
aspiers | SEV doesn't work without q35 | 11:59 |
sean-k-mooney | aspiers: where is that check going to be called form | 11:59 |
sean-k-mooney | it wont be used in the api | 11:59 |
aspiers | yes in the API was the idea | 11:59 |
sean-k-mooney | and it can only be known on the compute node | 11:59 |
sean-k-mooney | but what api | 11:59 |
sean-k-mooney | live migrate and suspend? | 11:59 |
aspiers | no, launch | 11:59 |
sean-k-mooney | it cant be known at lauch | 12:00 |
sean-k-mooney | we have not selected the host | 12:00 |
sean-k-mooney | and the machine type can be set in the hsot config | 12:00 |
aspiers | right, this is the crux of the problem | 12:00 |
sean-k-mooney | right so you cant check for q35 in the api | 12:00 |
aspiers | but we can at least check the image | 12:00 |
sean-k-mooney | at lest not the create server api | 12:00 |
aspiers | no we can check for hw_machine_type in the image | 12:01 |
aspiers | we already require hw_firmware_type=uefi | 12:01 |
sean-k-mooney | checkign the image does not require calling get default machine tyhpe | 12:01 |
sean-k-mooney | or whathever | 12:01 |
sean-k-mooney | if we are jsut checking the image then just check the metadata directly | 12:01 |
aspiers | that's what I was originally doing https://review.opendev.org/#/c/644565/49/nova/virt/libvirt/utils.py@538 | 12:02 |
*** HagunKim has quit IRC | 12:02 | |
sean-k-mooney | that check should be in hardware.py direclty | 12:02 |
*** markvoelker has joined #openstack-nova | 12:02 | |
aspiers | https://review.opendev.org/#/c/644565/49/nova/virt/libvirt/driver.py@5097 | 12:02 |
aspiers | I was originally only checking the image | 12:02 |
sean-k-mooney | you can have a second check for the machine type on the host | 12:02 |
sean-k-mooney | aspiers: for the fast fail on create server i think that is all you can do | 12:03 |
aspiers | hang on | 12:03 |
sean-k-mooney | then you can do the late check in the dirver | 12:03 |
aspiers | either we *require* hw_machine_type=q35 image property or we make it optional | 12:03 |
aspiers | which are you suggesting? | 12:03 |
aspiers | I mean in the SEV case | 12:03 |
sean-k-mooney | we will not require it | 12:04 |
sean-k-mooney | or should not | 12:04 |
aspiers | OK so then what's the point of an API check? | 12:04 |
sean-k-mooney | we can check if hw_machine_type is set | 12:04 |
sean-k-mooney | and if its not q35 reject | 12:04 |
aspiers | OK yeah | 12:04 |
aspiers | then if not set, it will fail later | 12:04 |
sean-k-mooney | but the api check was not for create it was for live migrate and suspend originaly | 12:04 |
aspiers | perhaps horribly on multiple compute hosts | 12:04 |
sean-k-mooney | yep | 12:04 |
aspiers | no live migrate/suspend is a different topic | 12:05 |
aspiers | that's not related to machine type | 12:05 |
aspiers | that relies on checking for hw_mem_encryption | 12:05 |
sean-k-mooney | we could maybe report machine types as traits in the future | 12:05 |
sean-k-mooney | but that is not ideal | 12:05 |
aspiers | that's this one https://review.opendev.org/#/c/680158/3 | 12:05 |
aspiers | kashyap: didn't we talk about that? | 12:05 |
*** tbachman has joined #openstack-nova | 12:05 | |
aspiers | kashyap: about machine types as traits? | 12:05 |
aspiers | thought I vaguely remembered some discussion on that | 12:06 |
aspiers | maybe not | 12:06 |
sean-k-mooney | i think that is leaking too much info personally | 12:06 |
*** brault has joined #openstack-nova | 12:06 | |
aspiers | I don't see a security risk but different rathole | 12:06 |
aspiers | maybe you're right but let's not go there now | 12:06 |
sean-k-mooney | no that is not what i ment | 12:06 |
sean-k-mooney | we could advertise that the host has q35 support | 12:07 |
aspiers | OK so to summarise, the plan is this: | 12:07 |
aspiers | hw_machine_type=q35 image prop is optional for SEV | 12:07 |
aspiers | we have an API check enforcing that if it's there, it has to be q35 family | 12:07 |
aspiers | but if it's not there then we just hope the scheduler picks a machine with nova.conf defaulting x86_64 to q35 | 12:08 |
aspiers | if it doesn't it will fail and try other compute hosts | 12:08 |
sean-k-mooney | and check it in the driver | 12:08 |
sean-k-mooney | but yes | 12:08 |
sean-k-mooney | it will fail and rescdule | 12:08 |
sean-k-mooney | and you can use host aggreates ectra if you want too to reduce the chance of that happening | 12:09 |
aspiers | so the operator should configure CONF.libvirt.machine_type to x86_64=q35 on all SEV hosts | 12:09 |
sean-k-mooney | if you dont want to set q35 in the image or the config | 12:09 |
sean-k-mooney | yes i think that is what we shoudl recommend | 12:09 |
aspiers | OK | 12:10 |
aspiers | BUT | 12:10 |
aspiers | does that fix the import cycle? | 12:10 |
aspiers | it means there are two types of q35 check | 12:10 |
aspiers | one image-only, and one including nova.conf | 12:10 |
sean-k-mooney | yes in hardware.py you dont need to import the utils funciton anymore | 12:10 |
sean-k-mooney | hardware.py just does the image check | 12:10 |
*** bbowen has joined #openstack-nova | 12:11 | |
sean-k-mooney | and the driver just does the final machine type check using the image+config | 12:11 |
aspiers | I don't follow | 12:11 |
sean-k-mooney | so no import of libvirt.utils is need in hardware.py | 12:11 |
sean-k-mooney | ill comment on the patch | 12:11 |
aspiers | wait | 12:11 |
aspiers | which one of the two types of q35 check are you suggesting happens in hardware.py? | 12:12 |
aspiers | the API one or the driver one? | 12:12 |
aspiers | the driver one *does* need libvirt.utils | 12:12 |
aspiers | to read the config | 12:12 |
aspiers | sean-k-mooney: ^^ (in case you are already commenting on the patch) | 12:12 |
sean-k-mooney | the api one should happen in hardware.py and it should only check the image | 12:13 |
aspiers | OK so where does the driver one happen? | 12:13 |
aspiers | in driver.py? | 12:13 |
sean-k-mooney | yes in driver.py | 12:13 |
aspiers | hmm | 12:13 |
sean-k-mooney | it can call the hardware.py if it wans ill type up som sudo code. | 12:14 |
*** bbowen_ has joined #openstack-nova | 12:14 | |
aspiers | yeah OK I think that might work | 12:14 |
aspiers | artom: I think sean-k-mooney's proposal should work ^^^ | 12:16 |
aspiers | kashyap too, in case you are interested | 12:16 |
aspiers | efried: you might just want to wait for the new patch sets rather than wade through this conversation ... | 12:16 |
aspiers | OK gotta go | 12:16 |
*** bbowen has quit IRC | 12:16 | |
aspiers | lunch | 12:17 |
brinzhang | gmann: Would you prefer https://review.opendev.org/#/c/673133/15/nova/tests/unit/api/openstack/compute/test_volumes.py@1036 ? | 12:18 |
Sundar | sean-k-mooney: Re. "testing patch that runs the tempst job on nova", I believe that would be https://review.opendev.org/#/c/670999/ | 12:18 |
brinzhang | gmann: I think this test already test pre 2.78 microversion, and the 'delete_on_termination' field not in the response body. | 12:18 |
gmann | brinzhang: yeah that is good test and same we need for list and show API operation | 12:19 |
gmann | brinzhang: that is for POST and same we should test for other modified API also. | 12:19 |
brinzhang | gmann: got it, I will update it, beacause it will be conflict, https://review.opendev.org/#/c/621476/ this patch will be merged first | 12:20 |
brinzhang | gmann: no need a follow-up patch. | 12:20 |
gmann | brinzhang: ok but we cna check how matt prefer. it will be easy for him to re+2 if you only rebase on microverison and do other changes in followup. | 12:21 |
gmann | i am ok either way | 12:22 |
brinzhang | gmann: thanks. | 12:22 |
sean-k-mooney | aspiers: when you get back see if that makes sense. https://review.opendev.org/#/c/680065/6/nova/virt/hardware.py | 12:24 |
*** ociuhandu has joined #openstack-nova | 12:27 | |
sean-k-mooney | Sundar: yes alther when i ran it 3 weeks ago it failed | 12:27 |
sean-k-mooney | ill recheck it | 12:27 |
sean-k-mooney | Sundar: actully that patch does not pull in the nova integration code unless the cyborge cod is | 12:28 |
sean-k-mooney | Sundar: so no that is propsoed agains nova but it does not run any of the nova cyborge integration code | 12:29 |
Sundar | sean-k-mooney: The dependencies need to be fixed there too. Will follow up. Thanks. | 12:29 |
sean-k-mooney | that should jsut be rebased on top of the final cyborg patch | 12:30 |
Sundar | Ok, I will ask thr author or do it myself | 12:31 |
Sundar | *the | 12:31 |
sean-k-mooney | i think we can do that in the gerrit ui | 12:32 |
sean-k-mooney | want me too try | 12:32 |
*** ociuhandu has quit IRC | 12:32 | |
sean-k-mooney | this is the final patch in teh series right https://review.opendev.org/#/c/673735/ | 12:32 |
openstackgerrit | sean mooney proposed openstack/nova master: [WIP] add cyborg tempest job https://review.opendev.org/670999 | 12:33 |
Sundar | sean-k-mooney: Yes, 673735 | 12:33 |
sean-k-mooney | ok its rebased and i updated the topic | 12:33 |
Sundar | Thank you | 12:34 |
sean-k-mooney | and i kicked off an experimental build so we should get results in an hour or so | 12:34 |
Sundar | It will probably fail because the Cyborg code dependencies are not set yet? | 12:35 |
sean-k-mooney | they are | 12:35 |
sean-k-mooney | well they coudl be wrong | 12:35 |
sean-k-mooney | it has | 12:36 |
sean-k-mooney | Depends-On: https://review.opendev.org/#/c/667231/ Depends-On: https://review.opendev.org/#/c/665318/ | 12:36 |
sean-k-mooney | so that is the fake driver and the tempest plugin | 12:36 |
*** jmlowe has quit IRC | 12:36 | |
Sundar | Yea, those are offbase. Should be 670470 and 674520 | 12:37 |
sean-k-mooney | the cyborg one might need to be updated if there is other stuff pending | 12:37 |
sean-k-mooney | ah ok ill let it to them or you to update | 12:37 |
kashyap | aspiers: Was AFK; reading now | 12:40 |
*** etp has quit IRC | 12:40 | |
*** eharney has joined #openstack-nova | 12:41 | |
kashyap | aspiers: We might've discussed (on machine types as traits) ... although I'm fuzzy on what we talked, though | 12:41 |
*** ratailor has quit IRC | 12:42 | |
*** nweinber has joined #openstack-nova | 12:43 | |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: update allocation in binding profile during migrate https://review.opendev.org/656422 | 12:44 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: prepare func test env for moving servers with bandwidth https://review.opendev.org/655109 | 12:44 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Func test for migrate server with ports having resource request https://review.opendev.org/655113 | 12:44 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add min service level check for migrate with bandwidth https://review.opendev.org/680394 | 12:44 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add bandwidth min service level check of source compute https://review.opendev.org/680395 | 12:44 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add bandwidth min service level check of source compute https://review.opendev.org/680396 | 12:44 |
*** nweinber_ has joined #openstack-nova | 12:46 | |
*** nweinber has quit IRC | 12:48 | |
kashyap | aspiers: Glad if you worked out a way to resolve the imports impasse. /me back to duking around with SB code | 12:50 |
openstackgerrit | Merged openstack/nova master: Add server sub-resource topology API https://review.opendev.org/621476 | 12:51 |
sean-k-mooney | kashyap: i think we have. and we discovered a 7 year old bit of tech debt to go clean up eventually. | 12:51 |
kashyap | sean-k-mooney: :D Which is? Also I saw in passing, that you wanted to "kill off designer.py" | 12:52 |
kashyap | IIRC, it was introduced with a purpose back in the day. | 12:52 |
sean-k-mooney | it was but we never finished it | 12:52 |
kashyap | (For making "policy" decisions.) | 12:53 |
sean-k-mooney | but that is not what i was refering too | 12:53 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Make _rever_allocation nested allocation aware https://review.opendev.org/676138 | 12:53 |
sean-k-mooney | kashyap: i was refering to removing this | 12:54 |
sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/__init__.py#L15-L17 | 12:54 |
sean-k-mooney | because of that when you import anythin in nova.virt.libvit.* | 12:54 |
sean-k-mooney | it pulls in the dirver which pulls in half of nova | 12:54 |
sean-k-mooney | well not really but importin nova.virt.libvirt.utils | 12:55 |
sean-k-mooney | shoudl not have that silent side effect | 12:55 |
kashyap | sean-k-mooney: Ah, okay; I see what you mean | 12:55 |
kashyap | Thanks for the context | 12:55 |
sean-k-mooney | so we shoudl remove that at somepoint | 12:55 |
sean-k-mooney | but its been that way for 7 years+ | 12:56 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support reverting migration / resize with bandwidth https://review.opendev.org/676140 | 12:56 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Func test for migrate re-schedule with bandwidth https://review.opendev.org/676972 | 12:56 |
artom | aspiers, so the API-level check boils down to essentially "if 'q35' in machine_type"? | 12:56 |
artom | And actually yeah, we can't do anything else | 12:57 |
artom | It just dawned on me that calling libvirt_utils on anything other than the compute host doesn't make sense | 12:57 |
artom | Since it checks the CONF | 12:57 |
artom | For the machine type, which may not even be set on a controller | 12:58 |
openstackgerrit | Brin Zhang proposed openstack/nova master: Add delete_on_termination to volume-attach API https://review.opendev.org/673133 | 12:59 |
sean-k-mooney | artom: yep | 12:59 |
*** jmlowe has joined #openstack-nova | 12:59 | |
sean-k-mooney | artom: even if it was set on the contoler i t would be wrong | 12:59 |
artom | sean-k-mooney, yep | 13:00 |
sean-k-mooney | the main api check we care about is for live migrate and suspend | 13:01 |
sean-k-mooney | the create server one is fine too but its not the main one | 13:01 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support migrating SRIOV port with bandwidth https://review.opendev.org/676980 | 13:02 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Allow migrating server with port resource request https://review.opendev.org/671497 | 13:02 |
aspiers | artom: exactly | 13:04 |
* sean-k-mooney goes for foods. | 13:05 | |
*** jchhatbar has joined #openstack-nova | 13:05 | |
* aspiers goes out for an hour or two | 13:06 | |
*** takashin has joined #openstack-nova | 13:06 | |
*** janki has quit IRC | 13:06 | |
*** mriedem has joined #openstack-nova | 13:07 | |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Do not query allocations twice in finish_revert_resize https://review.opendev.org/678827 | 13:07 |
*** jchhatba_ has joined #openstack-nova | 13:08 | |
brinzhang | mriedem: Beacause of https://review.opendev.org/#/c/621476/ hit the microversion 2.78, I was rebased it. Can you review https://review.opendev.org/#/c/673133/16 again? | 13:09 |
mriedem | brinzhang: yes | 13:09 |
mriedem | yonglihe: will you be working on adding support for microversion 2.78 to openstackclient as well? | 13:10 |
brinzhang | mriedem: About your comment in PS14, I update https://review.opendev.org/#/c/674243/20/nova/tests/functional/api_sample_tests/test_migrations.py@424, could you please review this again, and I will update it tomorrow. | 13:10 |
*** pcaruana has quit IRC | 13:10 | |
*** jchhatbar has quit IRC | 13:11 | |
brinzhang | mriedem: Alex already leaving comments on https://review.opendev.org/#/c/674243 | 13:12 |
gmann | mriedem: i asked few more test to add in 673133 and in followup as it was +2 by you. as he need to rebase you want to fix those in followup or same commit? what ever is easy to get your +2 :) | 13:12 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Allow resizing server with port resource request https://review.opendev.org/679019 | 13:13 |
*** mkrai has quit IRC | 13:14 | |
*** tbachman has quit IRC | 13:14 | |
mriedem | gmann: you asked for a test that already exists | 13:15 |
mriedem | test_attach_volume_pre_v278 | 13:15 |
gmann | mriedem: that is for POST API only i asked to test the same for GET (list and show) | 13:15 |
mriedem | ok | 13:16 |
brinzhang | mriedem: gmann: thanks | 13:16 |
luyao | dansmith: Hi, Dan, comments addressed, https://review.opendev.org/#/c/678447/ and https://review.opendev.org/#/c/678448/. Could you help confirm if it is right? | 13:17 |
*** brault has quit IRC | 13:18 | |
*** tbachman has joined #openstack-nova | 13:18 | |
*** ociuhandu has joined #openstack-nova | 13:18 | |
mriedem | brinzhang: https://review.opendev.org/#/c/674243/20 is in merge conflict | 13:19 |
brinzhang | mriedem: yeah, beacause of https://review.opendev.org/#/c/621476/ hit the microversion 2.78 | 13:21 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: update allocation in binding profile during migrate https://review.opendev.org/656422 | 13:22 |
*** spatel has joined #openstack-nova | 13:22 | |
brinzhang | mriedem: I will update it by tomorrow, and need your suggestion in https://review.opendev.org/#/c/674243/20/nova/tests/functional/api_sample_tests/test_migrations.py | 13:23 |
*** mkrai has joined #openstack-nova | 13:23 | |
yonglihe | mriedem: the client code is under reviewing now. | 13:24 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add min service level check for migrate with bandwidth https://review.opendev.org/680394 | 13:24 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add bandwidth min service level check of source compute https://review.opendev.org/680395 | 13:24 |
mriedem | brinzhang: i'm likely not going to get to the migrations API change today | 13:26 |
mriedem | lots of other stuff to review | 13:26 |
mriedem | yonglihe: novaclient is, but what about openstackclient? | 13:26 |
mriedem | yonglihe: see: https://etherpad.openstack.org/p/compute-api-microversion-gap-in-osc - we don't want to add more gaps to osc | 13:27 |
bauzas | gibi: can I start looking at your updated change ? ^ | 13:27 |
yonglihe | o, i like to do that also. | 13:27 |
gibi | bauzas: the first 3 patches are ready | 13:27 |
bauzas | gibi: cool | 13:28 |
mriedem | yonglihe: the osc change likely won't land until ussuri but you can start on it, | 13:28 |
mriedem | maybe after we get closer on the novaclient change | 13:28 |
brinzhang | mriedem: Ok, tomorrow I will update it, and then wait for your time :) | 13:28 |
yonglihe | mriedem: Sure, i definitely need few days to do that. | 13:28 |
gibi | mriedem: I replied in https://review.opendev.org/#/c/656422/18/nova/compute/manager.py@2122 I diverged from your original suggestion a bit | 13:29 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Add bandwidth min service level check of source compute https://review.opendev.org/680396 | 13:30 |
sean-k-mooney | donnyd: i seam to be getting node failures when i use multi-numa-ubuntu-bionic-expanded and multi-numa-ubuntu-bionic | 13:31 |
yonglihe | mriedem: I take it as my task, for sure. thanks your remind. | 13:31 |
sean-k-mooney | donnyd: based on https://github.com/openstack/project-config/blob/master/nodepool/nl02.openstack.org.yaml#L348-L357 they shoudl be the correct lables | 13:31 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: prepare func test env for moving servers with bandwidth https://review.opendev.org/655109 | 13:33 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Func test for migrate server with ports having resource request https://review.opendev.org/655113 | 13:36 |
stephenfin | mriedem: Mind taking a look at that patch to validate CPU config options again today? https://review.opendev.org/#/c/680107/ | 13:37 |
*** tbachman has quit IRC | 13:37 | |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Make _rever_allocation nested allocation aware https://review.opendev.org/676138 | 13:39 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support reverting migration / resize with bandwidth https://review.opendev.org/676140 | 13:39 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Func test for migrate re-schedule with bandwidth https://review.opendev.org/676972 | 13:39 |
*** pcaruana has joined #openstack-nova | 13:39 | |
stephenfin | bauzas: I split that "Differentiate between shared and dedicated CPUs" patch into two patches, btw :) https://review.opendev.org/#/c/680108/ and https://review.opendev.org/#/c/671800/ | 13:40 |
* bauzas needs to sharpen his pen then | 13:41 | |
*** spatel has quit IRC | 13:41 | |
stephenfin | what kind of monster sharpens a _pen_? | 13:41 |
* stephenfin notes bauzas has probably spent time in the slammer | 13:41 | |
stephenfin | must be where you got the tattoos | 13:41 |
mriedem | gibi: sure that works | 13:42 |
*** tbachman has joined #openstack-nova | 13:42 | |
gibi | mriedem: phee, I was a it worried | 13:42 |
sean-k-mooney | donnyd: actully it seams to be running now | 13:42 |
gibi | bauzas, mriedem: I think comments are fixed up until the first functional test. I'm working on that now | 13:43 |
bauzas | stephenfin: the OpenStack CLA doesn't require you to provide your filings | 13:43 |
bauzas | so I let you imagine whatever you want | 13:43 |
efried | sean-k-mooney: Can I please ask you to vet the libvirt xml stuff for vpmem here? https://review.opendev.org/#/c/678455/ | 13:43 |
bauzas | gibi: cool, later today if time allows me | 13:43 |
gibi | bauzas: thanks | 13:44 |
*** jchhatba_ has quit IRC | 13:44 | |
bauzas | mriedem: gibi: just one thing, hardly working on the placement audit command, are we hit by feature freeze or not ? | 13:46 |
bauzas | efried: ^ | 13:46 |
*** Luzi has quit IRC | 13:46 | |
gibi | bauzas: ff is next week | 13:46 |
bauzas | I know | 13:46 |
bauzas | talking of https://bugs.launchpad.net/nova/+bug/1793569 | 13:47 |
openstack | Launchpad bug 1793569 in OpenStack Compute (nova) "Add placement audit commands" [Wishlist,In progress] - Assigned to sean mooney (sean-k-mooney) | 13:47 |
bauzas | it's wishlist | 13:47 |
mriedem | gibi: a bit confusing that the commit message title is the same for these https://review.opendev.org/#/c/680395/2 https://review.opendev.org/#/c/680396/2 | 13:47 |
gibi | mriedem: noted, I will add some creativity to those | 13:48 |
openstackgerrit | Chris Dent proposed openstack/nova master: single pass instance info fetch in host manager https://review.opendev.org/623558 | 13:48 |
*** bnemec has quit IRC | 13:48 | |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support migrating SRIOV port with bandwidth https://review.opendev.org/676980 | 13:48 |
*** bnemec has joined #openstack-nova | 13:48 | |
gibi | bauzas: do you mean ff affects the work on that bug? | 13:48 |
efried | bauzas: If work isn't already well underway for that, it seems unlikely to make Train. | 13:48 |
donnyd | sean-k-mooney: those labels got put in the pool with the rest of everything else.. so you will likely have to wait a little longer | 13:49 |
bauzas | efried: I'm targeting to upload a last rev by tomorrow | 13:49 |
efried | bauzas: who's on the hook to review it? | 13:49 |
bauzas | efried: good question | 13:49 |
efried | What do you consider the importance/priority versus other things we're behind on? | 13:49 |
bauzas | efried: https://review.opendev.org/#/c/670112/ | 13:49 |
bauzas | efried: I don't disagree with your statement | 13:50 |
*** brault has joined #openstack-nova | 13:50 | |
efried | I haven't made statements, just questions :) | 13:50 |
efried | oh, the "unlikely" one perhaps | 13:50 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Allow migrating server with port resource request https://review.opendev.org/671497 | 13:51 |
mriedem | bauzas: it's not going to make train | 13:51 |
bauzas | efried: well, nevermind my question then, you know what, we'll see what's left | 13:51 |
cdent | efried: i put some unit tests on that host manager change of jays, above. it was as painful as I feared it would be, but I think it got it. at least cover says so | 13:51 |
dansmith | mriedem: so you think you can take a trip through the numa lm set this morning? | 13:51 |
mriedem | dansmith: it's on the todo list | 13:52 |
dansmith | mriedem: thanks | 13:52 |
efried | dansmith: can you run through the vpmem obj patches again please? | 13:52 |
mriedem | so, | 13:53 |
mriedem | efried: dansmith: besides numa lm, | 13:53 |
dansmith | efried: I just dropped a -1.9 on one | 13:53 |
mriedem | it seems the two other series that dansmith was reviewing was the forbidden aggregates stuff and sort of the vpmem stuff, right? | 13:53 |
mriedem | wouldn't it be a better use of dan's time to focus on forbidden aggregates if he's going to review one of those? | 13:53 |
dansmith | um.. | 13:54 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Do not query allocations twice in finish_revert_resize https://review.opendev.org/678827 | 13:54 |
mriedem | not that i'm picking what dan should work on in the next week, | 13:54 |
dansmith | apparently you are :) | 13:54 |
mriedem | but forbidden aggregates isn't merged yet right? | 13:54 |
mriedem | it was all +2ed at some point | 13:54 |
*** BjoernT has joined #openstack-nova | 13:54 | |
dansmith | I haven't looked at that set in a while, but I can circle back | 13:55 |
efried | ugh, forbidden aggregates slid down my list and out of sight at some point. | 13:55 |
mriedem | my point is if you're going to ask dan to review something he's reviewed before, maybe make it the forbidden aggs one | 13:55 |
efried | yeah, same | 13:55 |
dansmith | mriedem: well, I had gone back and forth on the vpmems bottom patches already recently | 13:56 |
mriedem | the vpmems series seeems way too risky to me at this point | 13:56 |
efried | mriedem: for me, vpmem is higher priority, and also more reliant on dansmith's expertise. If we needed to finish up forbidden aggs without him, it would be possible. vpmems (clearly) not so much. | 13:56 |
mriedem | sure, but do we expect to land that whole series in the next week? | 13:56 |
mriedem | w/o regression? | 13:56 |
dansmith | efried: the vpmem series has a -1 on the fourth patch and is really large above that, is that really going to land? | 13:57 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Allow resizing server with port resource request https://review.opendev.org/679019 | 13:57 |
dansmith | efried: because if not, we shouldn't merge the db and object stuff, IMHO | 13:57 |
efried | dansmith: it has been pretty close for a while | 13:57 |
dansmith | mriedem: yeah, I dunno about that, I was just looking to see what was above the two they were asking me to look at and ... it looks like a lot | 13:57 |
efried | mriedem: there should be no regression on that, it's code that isn't hit unless you activate it. I don't think it's risky from that perspective. | 13:57 |
mriedem | if dan hasn't been in the 2nd half of the series i wouldn't call it close | 13:57 |
efried | mriedem: we don't need Dan for the second half. We have three cores on it already | 13:57 |
efried | specialized review for ovo stuff | 13:58 |
efried | is where we need dan. | 13:58 |
*** brault has quit IRC | 13:58 | |
dansmith | just skimming the top, | 13:58 |
dansmith | it seems like there's plenty of room for regression in this stuff | 13:58 |
dansmith | despite being turned on only if you use it, it touches a lot of places | 13:58 |
efried | well, it's not like this is brand new, it's been through many rounds of review | 13:59 |
dansmith | like, changes the order and timing of talking to placement from the compute manager | 13:59 |
dansmith | that's not gated on this feature, that's fun for everyone | 13:59 |
mriedem | anything that deals with the RT flow is non-trivial | 14:01 |
mriedem | and prone to regression | 14:01 |
mriedem | i'm still fixing RT regressions since ocata | 14:01 |
efried | meeting now | 14:01 |
*** Sundar has quit IRC | 14:02 | |
efried | If you like, we can discuss there why we're trying to freeze a feature a week before feature freeze | 14:02 |
dansmith | narrowing focus to get some things through the door instead of spreading focus among everything and getting nothing through the door? | 14:03 |
*** ociuhandu has quit IRC | 14:04 | |
*** lpetrut has quit IRC | 14:05 | |
*** takashin has quit IRC | 14:05 | |
*** takashin has joined #openstack-nova | 14:06 | |
mriedem | gibi: comments in https://review.opendev.org/#/c/656422/21 | 14:06 |
mriedem | let me know if we should just defer to a follow up | 14:06 |
*** brault has joined #openstack-nova | 14:08 | |
*** BjoernT_ has joined #openstack-nova | 14:08 | |
*** BjoernT has quit IRC | 14:09 | |
gibi | mriedem: ack. I think I will defer to follow up there to ease the rebase pain | 14:09 |
mriedem | gibi: ok +2 | 14:11 |
gibi | mriedem: thx | 14:11 |
*** pcaruana has quit IRC | 14:12 | |
*** luksky has quit IRC | 14:19 | |
*** ociuhandu has joined #openstack-nova | 14:20 | |
openstackgerrit | Brin Zhang proposed openstack/nova master: Add user_id and project_id colume to Migration https://review.opendev.org/673990 | 14:22 |
openstackgerrit | Brin Zhang proposed openstack/nova master: Add operator user_id/project_id to the migrations https://review.opendev.org/679413 | 14:22 |
openstackgerrit | Brin Zhang proposed openstack/nova master: Filter migrations by user_id/project_id https://review.opendev.org/674243 | 14:22 |
*** ociuhandu has quit IRC | 14:25 | |
brinzhang | mriedem: https://review.opendev.org/#/c/674243/21 has bean resolved the merge conflict. | 14:28 |
*** lpetrut has joined #openstack-nova | 14:29 | |
*** mlavalle has joined #openstack-nova | 14:38 | |
*** udesale has quit IRC | 14:46 | |
*** lpetrut has quit IRC | 14:47 | |
*** udesale has joined #openstack-nova | 14:47 | |
*** gyee has joined #openstack-nova | 14:50 | |
sean-k-mooney | donnyd: it looks like its not completing and going to node_failure fairly consitently. | 14:51 |
donnyd | hrm | 14:52 |
donnyd | I will try to give that flavor a spin and make sure it actually works | 14:52 |
*** cfriesen has joined #openstack-nova | 14:54 | |
*** shilpasd has joined #openstack-nova | 14:55 | |
donnyd | Well I didn't have any issues getting it to spin up, so I guess we dig a little deeper | 14:57 |
donnyd | sean-k-mooney: do you have a log file handy? | 14:57 |
sean-k-mooney | donnyd: node_failure means that nodepool was not able to provision a node for the zuul job | 14:58 |
sean-k-mooney | so it failed before starting the job | 14:59 |
sean-k-mooney | there would be a log entry in nodepool but not in what is uploaded | 14:59 |
donnyd | http://grafana.openstack.org/d/3Bwpi5SZk/nodepool-fortnebula?orgId=1 | 14:59 |
donnyd | I only see 3 failures from the last 6 hours | 14:59 |
sean-k-mooney | if you search for 679656 in https://zuul.opendev.org/t/openstack/status you wil see the latest one | 15:00 |
sean-k-mooney | this can happen if zuul is jsut at capastity for the provider | 15:01 |
sean-k-mooney | and it timed out waiting | 15:01 |
sean-k-mooney | i was seeing some node faiures before you update the lables too | 15:01 |
*** cdent has quit IRC | 15:01 | |
sean-k-mooney | so maybe its just worse due to the shared pool | 15:02 |
*** takashin has left #openstack-nova | 15:02 | |
sean-k-mooney | it was about 1 in 2 or 1 in 3 that failed previously | 15:02 |
donnyd | sean-k-mooney: I am not sure where I am supposed to be looking | 15:03 |
donnyd | that is a pretty high failure rate no matter what | 15:03 |
donnyd | shouldn't be failing that often | 15:03 |
donnyd | maybe someone in infra can give us some insights. I just provisioned 10 without a single failure | 15:04 |
sean-k-mooney | well as i said it was a node failutre meaning noodpool could not provide a node to zuul to run the job on. that can happen due to hitting quota | 15:04 |
efried | kashyap: can we talk about https://blueprints.launchpad.net/nova/+spec/allow-secure-boot-for-qemu-kvm-guests ? | 15:04 |
kashyap | efried: Saddled with calls; but do ask away | 15:04 |
kashyap | efried: Actively working on it... | 15:04 |
sean-k-mooney | donnyd: FN is proably fine this could be on infras side. | 15:04 |
donnyd | sean-k-mooney: let me up the quota and see if that is helpful | 15:04 |
efried | kashyap: do you hope to land it in the next week or should I defer to ussuri? | 15:04 |
sean-k-mooney | i need to jump on a call but for now ill wait and check the jobs later when the load drops a bit | 15:05 |
donnyd | ok | 15:05 |
openstackgerrit | Kobi Samoray proposed openstack/nova master: Avoid fetching metadata when no subnets found https://review.opendev.org/679247 | 15:05 |
sean-k-mooney | we intend to run this job nightly at 6am with ocational manual triggering so it should not generally be a proablem | 15:06 |
*** yan0s has quit IRC | 15:09 | |
donnyd | sean-k-mooney: infra said they were hitting quota issues, so i just oversubscribed it for instances by 30% and pulled the rest of the quota entirely | 15:10 |
donnyd | openstack.exceptions.HttpException: HttpException: 403: Client Error for url: https://openstack.fortnebula.com:13774/v2.1/e8fd161dc34c421a979a9e6421f823e9/servers, Quota exceeded for instances: Requested 1, but already used 70 of 70 instances | 15:10 |
donnyd | sean-k-mooney: that issue should be solved | 15:11 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: migrate: Add bw min service level check of source compute https://review.opendev.org/680395 | 15:11 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: resize: Add bw min service level check of source compute https://review.opendev.org/680396 | 15:11 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: Validate CPU config options against running instances https://review.opendev.org/680107 | 15:13 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Use sane indent https://review.opendev.org/680229 | 15:13 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: objects: Add 'NUMACell.pcpuset' field https://review.opendev.org/680108 | 15:13 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: hardware: Differentiate between shared and dedicated CPUs https://review.opendev.org/671800 | 15:13 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Start reporting 'HW_CPU_HYPERTHREADING' trait https://review.opendev.org/675571 | 15:13 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: Add support for translating CPU policy extra specs, image meta https://review.opendev.org/671801 | 15:13 |
openstackgerrit | Stephen Finucane proposed openstack/nova master: Add reshaper for PCPU https://review.opendev.org/674895 | 15:13 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: prepare func test env for moving servers with bandwidth https://review.opendev.org/655109 | 15:14 |
artom | dansmith, just want to make sure you saw my updates to the NUMA LM patch last night | 15:14 |
dansmith | artom: I did, but Imma wait until mriedem has a go through it | 15:15 |
artom | dansmith, ack | 15:15 |
efried | dustinc: I'd like to defer provider-config-file to ussuri given it has only had one reviewer (me) looking at it so far. Please let me know if you have reason to believe it can land in the next week. | 15:16 |
*** mkrai has quit IRC | 15:17 | |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Func test for migrate server with ports having resource request https://review.opendev.org/655113 | 15:20 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Make _rever_allocation nested allocation aware https://review.opendev.org/676138 | 15:20 |
kashyap | efried: Don't defer it yet, please. But I'll keep you posted. | 15:20 |
efried | kashyap: ack | 15:21 |
kashyap | efried: I'll also be using that exception from Adam's InvalidMachineType patch; so there's a small dep there | 15:21 |
efried | kashyap: unclear whether SEV will make it too fyi | 15:21 |
kashyap | efried: Hmm, understandable; review bandwidth, topic complexity, etc... | 15:22 |
kashyap | But to state the obvious, I don't want to rush anything, though. | 15:23 |
efried | stephenfin: how are you feeling about cpu-resources? | 15:23 |
stephenfin | efried: I'm happy with it, but I need someone to take the hit on https://review.opendev.org/#/c/671800/ I've simplified it as much as I can at this point, I think | 15:24 |
stephenfin | I've been bugging bauzas for that :) | 15:24 |
efried | stephenfin: what I mean is: do you reasonably expect it can land by next Thursday? | 15:25 |
stephenfin | Yeah, I think so | 15:25 |
efried | okay | 15:25 |
bauzas | efried: I feel it's a reasonable target if someone else but me does reviews as well | 15:25 |
bauzas | hint : could be you | 15:25 |
efried | bauzas: I've been doing what I can, but much of it is beyond my ken. | 15:25 |
bauzas | cool | 15:25 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support reverting migration / resize with bandwidth https://review.opendev.org/676140 | 15:26 |
*** ccamacho has quit IRC | 15:26 | |
stephenfin | efried: tbh, it's probably beyond most people that aren't called me and sean-k-mooney (and maybe cfriesen) | 15:27 |
stephenfin | so I think we're going to have to rely on the fact that there are lot of functional tests there, and ask me all the questions if necessary | 15:27 |
stephenfin | (and I mean, a _lot_ of functional tests) | 15:27 |
efried | Okay, sean-k-mooney if you've got room, it would be helpful if you could ack that stuff. | 15:28 |
efried | I think between bauzas, alex_xu, sean-k-mooney, and me, we should be able to scrape together the expertise. It's more a question of bandwidth. | 15:28 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Func test for migrate re-schedule with bandwidth https://review.opendev.org/676972 | 15:29 |
*** ociuhandu has joined #openstack-nova | 15:31 | |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support migrating SRIOV port with bandwidth https://review.opendev.org/676980 | 15:32 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Allow migrating server with port resource request https://review.opendev.org/671497 | 15:32 |
sean-k-mooney | efried: sorry was on a call | 15:35 |
sean-k-mooney | what was the context | 15:35 |
efried | sean-k-mooney: The cpu-resources series | 15:35 |
bauzas | sean-k-mooney: the point was whether we have reasonable amount of eyes for cpu-resources | 15:36 |
efried | sean-k-mooney: it would increase confidence if it had your ack | 15:36 |
efried | dansmith: regarding isolated vs forbidden, this is a Nova UX vs Placement API thing. | 15:36 |
sean-k-mooney | i can dedicate the next few days to testing it like i was toing with artoms stuff | 15:36 |
dansmith | efried: yeah I called that out | 15:36 |
*** ociuhandu has quit IRC | 15:36 | |
efried | From the Nova perspective, we're isolating aggregates. Internally we're using placement's forbidden aggregates feature. | 15:36 |
sean-k-mooney | i think the numa migration is more or less in a good place so i can review the cpu serieis and start testing | 15:37 |
dansmith | efried: did you see my comment on the later patch? | 15:37 |
sean-k-mooney | efried: while i hate to do this to my own seires we could punt https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/image-metadata-prefiltering if required to help to U. its ready to go but the sev and cpu series conflict with it | 15:38 |
sean-k-mooney | so untill they are landed its going to keep gong into merge conflict | 15:38 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Do not query allocations twice in finish_revert_resize https://review.opendev.org/678827 | 15:38 |
efried | sean-k-mooney: thanks, that's helpful | 15:39 |
sean-k-mooney | efried: stephen created https://etherpad.openstack.org/p/nova-cpu-resources as a set of test he wanted me to try. | 15:40 |
*** brault has quit IRC | 15:40 | |
sean-k-mooney | i can start on them today/tomorrow but if anything else woudl help i could try that. | 15:40 |
sean-k-mooney | i can if need also try and create a temp ci job | 15:41 |
efried | dansmith: seeing it now. Sorry, I'm spread in a number of directions at the moment. | 15:41 |
openstackgerrit | Balazs Gibizer proposed openstack/nova master: Allow resizing server with port resource request https://review.opendev.org/679019 | 15:41 |
dansmith | efried: let me circle back and explain more about why I think the seam needs to move up a step | 15:41 |
efried | dansmith: From my perspective, everything on the nova side should be called isolated, and we should only call placement-isms forbidden | 15:41 |
dansmith | efried: cool, I shall tailor my response to indicate why that's a problem | 15:42 |
efried | dansmith: It's a little bit like where we (should) draw the line between 'member_of' and 'aggregate'. | 15:42 |
dansmith | efried: yep | 15:43 |
efried | 'member_of' should really only start appearing in the thing we're using to construct the placement querystring. | 15:43 |
efried | RequestGroup, I think it's called. | 15:43 |
*** mdbooth_ has joined #openstack-nova | 15:45 | |
dustinc | efried: I’d really like to get it landed in train and am working towards that. I hope to get it reviewable again by eod today. | 15:45 |
efried | dustinc: I would like that too, but unfortunately I don't feel like it's close enough given how much other stuff we're trying to cram in in the next week. | 15:46 |
*** jmlowe has quit IRC | 15:47 | |
efried | dustinc: not because you did anything wrong, just because there has been more to review than reviewers have been able to keep up with, and this is one that slid off the bottom. | 15:48 |
*** mdbooth has quit IRC | 15:48 | |
dansmith | efried: https://review.opendev.org/#/c/671072/11/nova/scheduler/utils.py | 15:48 |
efried | dansmith: ack. You da boss. shilpasd ^ | 15:49 |
dansmith | efried: you just saw a wall of text and gave in without reading didn't you? | 15:49 |
efried | no, I read it. I don't agree, but it's more important to you than to me and I want to see it move forward. So let's do that. | 15:50 |
dansmith | well, that's constructive | 15:50 |
efried | dansmith: tbc, what you want to see is the field in Destination named forbidden_aggregates rather than isolated_aggregates. Everything else is okay? | 15:50 |
dustinc | efried: I understand. I will keep working on it anyway but if the review bandwidth isn’t there then go ahead and punt it to U. Thanks for your time spent reviewing it so far. | 15:52 |
dansmith | efried: I think the set is kinda upside down, which I assume you also saw from the later ones, but what I'm gathering is that by asking me to review, you wanted me to either just approve or only look shallow for things that physically won't work instead of things that might bite us in the future | 15:52 |
dansmith | efried: so maybe I should just drop my -1 and you find someone looking to do _that_ kind of review? | 15:53 |
efried | dansmith: Again, multitasking heavily right now, so I haven't read all of whatever reviews have been done today. I don't want you to rage quit. If there are other nontrivial issues you've called out, then those should be addressed. | 15:55 |
*** jmlowe has joined #openstack-nova | 16:00 | |
*** altlogbot_1 has quit IRC | 16:01 | |
*** altlogbot_3 has joined #openstack-nova | 16:02 | |
*** irclogbot_3 has quit IRC | 16:03 | |
*** irclogbot_2 has joined #openstack-nova | 16:05 | |
*** artom has quit IRC | 16:06 | |
*** damien_r has quit IRC | 16:06 | |
*** irclogbot_2 has quit IRC | 16:07 | |
*** irclogbot_2 has joined #openstack-nova | 16:08 | |
*** artom has joined #openstack-nova | 16:14 | |
*** cdent has joined #openstack-nova | 16:15 | |
*** ivve has quit IRC | 16:19 | |
mriedem | gibi: so close https://review.opendev.org/#/c/680394/2 but there are missing tests for the new _reschedule condition | 16:24 |
*** tbachman has quit IRC | 16:28 | |
*** sapd1_x has joined #openstack-nova | 16:34 | |
*** maciejjozefczyk has quit IRC | 16:38 | |
*** zigo has quit IRC | 16:43 | |
*** spsurya has quit IRC | 16:48 | |
*** sapd1_x has quit IRC | 16:48 | |
*** igordc has joined #openstack-nova | 16:49 | |
*** jaosorior has quit IRC | 16:51 | |
mriedem | gibi: a few easy things to address on the 2 after that as well | 16:52 |
*** ganso has left #openstack-nova | 16:53 | |
openstackgerrit | raphael.glon proposed openstack/nova master: Ironic driver: fix when entering rebuild while already in error https://review.opendev.org/680468 | 16:56 |
*** ociuhandu has joined #openstack-nova | 17:01 | |
*** tbachman has joined #openstack-nova | 17:03 | |
*** markvoelker has quit IRC | 17:04 | |
*** igordc has quit IRC | 17:04 | |
openstackgerrit | Dustin Cowles proposed openstack/nova-specs master: Spec: Provider config YAML file https://review.opendev.org/680471 | 17:04 |
*** tesseract has quit IRC | 17:06 | |
mriedem | Ironic driver: fix when entering rebuild while already in error https://review.opendev.org/680468 is another duplicate of https://review.opendev.org/#/c/523559/ which is pretty old so maybe we should get that in? | 17:06 |
mriedem | melwitt: ^? | 17:07 |
melwitt | mriedem: thanks for the reminder, I was thinking about that the other day. I do want to review that. been dreading re-loading of context on that one | 17:08 |
mriedem | i think given the amount of time, the review it's had, the duplicates, and the fact it was proposed by an operator of an ironic deployment and this fixes their issue, and it's so narrow in focus, it's pretty low risk to just pull the trigger | 17:10 |
mriedem | like a mfing renegad | 17:10 |
mriedem | *renegade | 17:10 |
*** zigo has joined #openstack-nova | 17:11 | |
mriedem | artom: dansmith: just got done going through that bw provider migration series again, now lunch and then i swear on my sweet lucy cat's whiskers that i'll review that numa live migration series | 17:11 |
artom | mriedem, appreciated :) | 17:12 |
*** markvoelker has joined #openstack-nova | 17:13 | |
*** igordc has joined #openstack-nova | 17:15 | |
efried | I think I've deferred all the obvious deferrals at this point. https://blueprints.launchpad.net/nova/train | 17:16 |
efried | A couple on there still that don't seem likely, but aren't obvious so I'm letting them slide off and will punt them next week. | 17:16 |
*** udesale has quit IRC | 17:18 | |
*** maciejjozefczyk has joined #openstack-nova | 17:19 | |
dansmith | mriedem: thanks | 17:21 |
*** jaosorior has joined #openstack-nova | 17:22 | |
dansmith | mriedem: I'm sure you'll find some stuff, so just want to make sure there's time to address it | 17:22 |
*** larainema has quit IRC | 17:22 | |
*** maciejjozefczyk has quit IRC | 17:24 | |
*** igordc has quit IRC | 17:26 | |
*** shilpasd has quit IRC | 17:27 | |
*** ociuhandu_ has joined #openstack-nova | 17:32 | |
donnyd | melwitt: dansmith for the record today I have only thrown 4 errors from image related tasks... usually it is about 5x that | 17:33 |
donnyd | thanks for sticking with me on it | 17:33 |
melwitt | mriedem: noted | 17:33 |
dansmith | donnyd: cool | 17:34 |
melwitt | donnyd: \o/ | 17:34 |
*** ricolin has quit IRC | 17:35 | |
*** ociuhandu has quit IRC | 17:36 | |
*** ociuhandu_ has quit IRC | 17:36 | |
melwitt | mriedem: +2 | 17:41 |
*** gbarros has quit IRC | 17:48 | |
*** cdent has quit IRC | 17:50 | |
mriedem | artom: you could have probably asked in irc or commented without a vote on https://review.opendev.org/#/c/656594/ | 17:51 |
mriedem | but i've replied | 17:51 |
mriedem | i see you've gone hunting for my changes... | 17:51 |
dansmith | ah yes, "I plan to backport this" is always a good reason :D | 17:57 |
mriedem | don't get me wrong, i appreciate the reviews | 17:57 |
mriedem | and lord knows artom is heavily in debt in this regard :P | 17:57 |
dansmith | artom: protip: tell him you'll +1 if he queues up a patch behind to do the right thing that needn't be backported :P | 17:58 |
mriedem | i still won't do what he's suggesting, but he can do that | 17:58 |
dansmith | haha | 17:58 |
dansmith | well, probably still a good thought anyway.. at least he's thinking of front-loading api-related stuff in the api, so I'll give him credit | 17:59 |
mriedem | not to incur debt myself, but if there are any dan's around looking for reviews that is the bug fix for the multi-cell floating IP disassociate issue i found in denver... | 17:59 |
mriedem | i agree it's not crazy | 17:59 |
dansmith | assuming you're moving onto the numa lm patches I could probably look for a reason to -1 your fix | 18:00 |
mriedem | when i don't see lee around much i need to move onto someone else to jab | 18:00 |
mriedem | yeah yeah i'm on it | 18:00 |
mriedem | btw, assuming anyone else is watching zuul, looks like it's taking around at least 5 hours to get a node | 18:01 |
*** gbarros has joined #openstack-nova | 18:01 | |
mriedem | (679473,1) Handle VirtDriverNotReady in _cleanup_running_deleted_instances (17h46m/ | 18:02 |
*** luksky has joined #openstack-nova | 18:02 | |
mriedem | that seems...less than good | 18:02 |
*** brault has joined #openstack-nova | 18:05 | |
*** brault has quit IRC | 18:09 | |
dansmith | crikey, yeah it's backed wayy up | 18:21 |
openstackgerrit | Adam Spiers proposed openstack/nova master: Ensure non-q35 machine type is not used when booting with SEV https://review.opendev.org/680065 | 18:22 |
aspiers | sean-k-mooney, kashyap, efried: I think this one should be good now ^^^ Reworking the other few on top now ... | 18:23 |
efried | dansmith: Can you help me understand under what circumstances a Destination gets persisted and then also reused later to influence a scheduling decision? | 18:23 |
dansmith | efried: it's serialized inside the reqspec yeah? | 18:24 |
efried | dansmith: because if we're not reinvoking the scheduler with filters to recalculate isolated aggregates on a move operation, then I agree that's a problem. | 18:24 |
efried | so we calculate a reqspec once for an instance and then reuse it for move operations from that point on without reworking the stuff inside it?? | 18:25 |
efried | Surely that's only the case for a non-resize-y move | 18:25 |
dansmith | we recalculate some of it, and what we recalculate has drifted a LOT over time | 18:25 |
dansmith | which is my point, | 18:25 |
efried | mm, "some of it". | 18:26 |
dansmith | even if we blow it away currently, | 18:26 |
dansmith | we're storing the values as "isolated aggregates" and not "isolated (at the time of boot which was six months ago) aggregates" | 18:26 |
*** mvkr has quit IRC | 18:26 | |
dansmith | "forbidden" doesn't imply a reason | 18:26 |
dansmith | they were forbidden, they may no longer be forbidden for reasons related to this request or config, etc | 18:26 |
efried | I understand the issue now, thanks. Though I still fail to see how the word choice 'forbidden' vs 'isolated' has any power to clarify. Seems like a distinction without a difference, certainly not one that gives any hint about this persistence-vs-not issue. | 18:27 |
efried | not recalculating aggregate isolation would be a bug for sure, regardless of what we called it. | 18:28 |
dansmith | forbidden implies what we're going to do with it... exclude those aggregates | 18:28 |
efried | I would possibly even go so far as to say that not recalculating *any* request filter would be a bug. | 18:28 |
dansmith | isolated implies *why they are forbidden, which is based on a point-in-time configuration | 18:28 |
*** gbarros has quit IRC | 18:29 | |
efried | I'm normally the pedantic, specific-word-choice guy, but I don't see it, sorry. So I'm sticking with my earlier story: if it makes the difference to you, it's fine with me. | 18:29 |
dansmith | in other news, while looking for a reference for you, I realize that we now exclude destination from the reqspec when serializing, so it won't get persisted | 18:30 |
dansmith | good thing we had a discussion instead of you just pushing until I agree to something eh? | 18:30 |
efried | oh, that's nice :) | 18:30 |
efried | dude, that was never happening, you're making that part up. | 18:30 |
dansmith | shall I quote you? | 18:30 |
efried | please do | 18:30 |
dansmith | I mean seriously | 18:30 |
efried | I am serious | 18:30 |
dansmith | *eyeroll* | 18:31 |
efried | I at no time asked or even implied that you should roll over and approve something you disagreed with. | 18:31 |
dansmith | no, that's not what I said | 18:31 |
*** mriedem has quit IRC | 18:31 | |
dansmith | I said *you* were just going to ask for the change so I'd agree to it without discussion | 18:31 |
dansmith | which you said a few minutes ago, and earlier this morning | 18:31 |
efried | oh, yeah, true story, I was avoiding what I saw as a bikeshed over a name choice | 18:32 |
dansmith | which is not the point of review and of course not the reason I'd ask for a change, just to be appeased | 18:32 |
dansmith | so, here's my other reason for still thinking this is the wrong name for it | 18:33 |
efried | because I didn't (and still don't) see how 'isolated' and 'forbidden' mean different things in this context *other* than nova ux vs placement api terminology. | 18:33 |
*** mriedem has joined #openstack-nova | 18:33 | |
dansmith | presumably we're going to need to forbid aggregates for other reasons in the future, for other nova features unrelated to aggregate isolation right? | 18:33 |
mriedem | efried: dansmith: i dropped and maybe you already talked about this, or maybe it doesn't matter, but request spec's requested_destination isn't persisted | 18:33 |
dansmith | heh | 18:34 |
dansmith | yes, we've covered that already :) | 18:34 |
mriedem | it used to be in the long ago and caused all sorts of problems | 18:34 |
dansmith | yup | 18:34 |
* mriedem slinks back to hole | 18:34 | |
dansmith | the destination object is all about what requests we're making to the scheduler and placement downstream of it, and since it's rpc, putting something in there means that's the interface and changing it later requires a version bump | 18:35 |
efried | dansmith: that's an interesting point, yes. I'm not fully swapped in on the code still, but I would think that would be a good reason to have the Destination field have 'isolated_aggregates', so that we can add 'frobnicated_aggregates' later and merge all of those into 'forbidden_aggregates' for the placement call. | 18:35 |
dansmith | so if we ever think conductor may ask to forbid an aggregate for any other reason, we'd have to shove it into "isolated_aggregates" to make that happen, or add another field with the same purpose | 18:35 |
efried | the latter, yes, IMO | 18:36 |
efried | no? | 18:36 |
dansmith | efried: why? the scheduler doesn't need to know why to exclude aggregates, it just needs to know that it should | 18:36 |
*** dtantsur is now known as dtantsur|afk | 18:36 | |
efried | well | 18:36 |
*** igordc has joined #openstack-nova | 18:36 | |
dansmith | AZs are aggregates, what if I were to ask next cycle for a "disabled" flag on AZs? all we would need to do is tell scheduler (and thus placement) that AZ aggregate $uuid should be forbidden | 18:37 |
efried | gimme a sec to make sure I'm thinking of the right object hierarchy here... | 18:37 |
dansmith | it's not isolated in the notion that nova's new isolated aggregate feature means, it's something else | 18:37 |
efried | ...so IMO what we want to be working toward is to have the placement-isms represented all and only in the RequestSpec.requested_resources field. | 18:38 |
efried | and in that case, since those are RequestGroup, absolutely we would have already funneled everything forbidden_aggregates into forbidden_aggregates. | 18:39 |
artom | mriedem, yep, shamelessly reviewing your bugfixes to quary favour | 18:39 |
dansmith | yup, agree there | 18:39 |
efried | The fact that anything that still needs to be translated to placement-ese lives in Destination (or elsewhere) is, to my way of thinking, tech debt. | 18:39 |
mriedem | favoUr?! | 18:39 |
artom | mriedem, *shrug* I changed it to a +1, didn't I? Given what dansmith said afterwards, looks like my concern was valid | 18:39 |
artom | mriedem, yes, the Queen's spelling | 18:40 |
dansmith | efried: because the words "forbidden" and "aggregates" in that order are forever trademarked by placement? | 18:40 |
dansmith | efried: call them excluded_aggregates in the Destination object and that'd also be fine | 18:40 |
dansmith | artom: here and I was all ready to defend you, but I can't get behind the queen and her broke-ass spelling | 18:41 |
artom | dansmith, thank you, you know you're my favourite | 18:44 |
dansmith | grr. | 18:45 |
efried | dansmith: but for the sake of shilpasd taking action, which will happen while we sleep, Destination.forbidden_aggregates will work for you, yes? | 18:45 |
dansmith | efried: I think I've already said it would | 18:46 |
efried | It's worth clarifying | 18:46 |
efried | measure twice, cut once | 18:46 |
dansmith | if this wasn't codified in our object schema until version 2.0, you could totally claim that arguing over naming is not worth the trouble | 18:47 |
dansmith | but this stuff will stick with us for a long time | 18:47 |
dansmith | so while you try to make it sound like I'm being unreasonably pedantic about the naming, I think I have a lot of version bumping battle scars (which nobody else has, btw) to back up my reasoning | 18:47 |
dansmith | (*object version bumping scars.) | 18:48 |
efried | dooood | 18:48 |
efried | I'm not trying to make it sound like you're being unreasonable | 18:48 |
efried | all I've said is I don't see the difference | 18:49 |
efried | and I'm happy to defer to your judgment. | 18:49 |
efried | which is why I ask you for reviews on object/RPC/etc stuff | 18:49 |
aspiers | efried: quick question, I want to reuse most of the fixture in test_config_kvm() https://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/test_config.py#L2511 in new test I'm adding to test_designer.py - should I move it to fake_libvirt_data.py in a separate commit before reusing it, or move it as part of the commit adding the new test? | 18:50 |
aspiers | artom: BTW this is how our discussion with sean-k-mooney turned out if you're curious https://review.opendev.org/#/c/680065/ | 18:51 |
efried | aspiers: is the patch you're adding just a test? | 18:51 |
aspiers | efried: no, it's the whole "apply SEV config" patch | 18:51 |
efried | yar. Then do it separately. | 18:52 |
aspiers | OK thanks! | 18:52 |
* efried ==> next chauffeur errand | 18:52 | |
*** efried is now known as efried_afk | 18:52 | |
artom | aspiers, yep, logic looks good | 18:53 |
aspiers | artom: cool, thanks | 18:53 |
aspiers | artom: fairly close to having the follow-on commit ready which adds the machine type check to the driver | 18:53 |
aspiers | I'm using sean-k-mooney's nice idea of having the machine_type parameter in hardware.py optional, so the checking code can be reusedin both scenarios | 18:54 |
aspiers | took several redesigns but I think we're finally there | 18:54 |
*** bbowen_ has quit IRC | 18:56 | |
*** gbarros has joined #openstack-nova | 19:00 | |
*** trident has quit IRC | 19:09 | |
*** efried_afk is now known as efried | 19:13 | |
efried | dansmith: moving up, the current ordering is my fault: I thought it was worse to first expose a conf opt that doesn't do anything, followed by the code that does the thing; than to introduce and unit test all the code, followed by a "master switch" that exposes it to the user in one go. | 19:15 |
efried | and thus the latter is the approach I've been advocating to *all* the series I've been reviewing this cycle. | 19:15 |
dansmith | efried: it is, I'm not suggesting to put the conf toggle first, of course | 19:16 |
dansmith | I'm suggesting you get all the plumbing that is in the last patch in front, and then land the filter and its knob together last | 19:16 |
* efried looks... | 19:16 | |
efried | dansmith: that would effectively be combining the last two patches afaict. Which last-patch plumbing do you mean? Add the filter to the list but put an `if True: return` at the top of the filter method, and then s/True/CONF..../ in the last patch? | 19:18 |
dansmith | efried: nova-status, scheduler/report.py, utils.py and associated tests can all be done ahead of time | 19:19 |
dansmith | from the last patch | 19:19 |
dansmith | then you can add the filter and the knob, | 19:19 |
dansmith | and docs could be in there or in a following patch | 19:19 |
dansmith | the last patch is a jumble of things | 19:19 |
dansmith | some of which are like rebase noise or something | 19:19 |
efried | Yes, actually, utils.py looks like it should have been earlier regardless. client/report could just be flattened. I think the only reason these are split is to make the review smaller. | 19:20 |
efried | reviews | 19:20 |
dansmith | it's likyes | 19:20 |
*** trident has joined #openstack-nova | 19:20 | |
efried | which is probably also my fault. | 19:20 |
dansmith | it's like the "uh, add the conf knob and all the other shit I forgot about" patch | 19:21 |
efried | if it was just the former, it would be okay IMO | 19:21 |
dansmith | there's no reason to land the filter and the knob separately | 19:22 |
dansmith | there is reason to land the plumbing separate from the filter/knob | 19:22 |
dansmith | and it's fine to add the docs in a separate patch, which could slip past FF even | 19:22 |
efried | yeah, I just don't actually see any "plumbing" to speak of in that last patch | 19:22 |
dansmith | well, like you said, it's stuff that should have been earlier | 19:22 |
dansmith | the utils change, the placement version requirement bump, etc | 19:23 |
*** ralonsoh has quit IRC | 19:23 | |
efried | okay, so again for the sake of being able to give shilpa a clear message, would it be acceptable to move the .py bits (including conf knob) of the last patch into the second-last patch, and leave the last patch for docs/reno? | 19:23 |
efried | because rn I don't think she understands that there's an action to be taken, based on her response. | 19:24 |
dansmith | I think that the sorted change in utils is probably its own patch, with tests so that it's easy to validate which test is confirming that, then yes the filter, knob, | 19:25 |
dansmith | isn't the report client bit actually already late? | 19:26 |
dansmith | meaning we already have code landed to put the not-member-of stuff in the qparams, we just won't ever run that code nor ask for the right version? | 19:26 |
efried | dansmith: it's not necessary until there's code that can hit the placement request with forbidden agg syntax | 19:26 |
dansmith | if so, that would also be a "whoopsie" pre-patch, IMHO | 19:26 |
dansmith | efried: mixing that up is not great to me.. if there's code that, if called, could generate a query that needs a newer version but it won't be provided, | 19:27 |
dansmith | that was a mistake | 19:27 |
dansmith | but whatever | 19:27 |
efried | yeah, could be perceived as "we can't hit this code so it doesn't matter" regardless of the reason we can't hit it, but I see your point. | 19:27 |
dansmith | anyone else could land a patch right now that calls that code in such a way that it won't generate a sensible placement request | 19:28 |
dansmith | and since it's kinda library/client code, it should be landed in a way that makes it work if called, IMHO | 19:28 |
efried | ack | 19:28 |
dansmith | so, to summarize, | 19:28 |
dansmith | if it were me, I would ask for: | 19:28 |
dansmith | 1. A patch to fix up all the "oops I forgot" stuff in the placement client section, which is a chunk of the last patch currently | 19:29 |
dansmith | 2. A patch to add the filter and its knob | 19:29 |
*** gbarros has quit IRC | 19:29 | |
dansmith | 3. Docs (and reno) at the end, which is the remaining bit of the last patchI think (if you subtract some of the random changes that are in there like renaming a test) | 19:29 |
*** gbarros has joined #openstack-nova | 19:30 | |
mriedem | artom: some comments/questions on patches 2 and 3, onto the big one https://review.opendev.org/#/c/634606/ | 19:30 |
artom | mriedem, cheers! I have to pick up kids from school in 15, so my responses will have to wait later tonight, and probably into tomorrow morning | 19:31 |
*** jmlowe has quit IRC | 19:32 | |
efried | dansmith: okay. I think I could probably do that shuffle and still +2 without losing sleep. | 19:32 |
dansmith | efried: agree | 19:32 |
mriedem | artom: yeah nothing major so far, just +1 with "i'm still going, but answer before i +2" | 19:35 |
dansmith | mriedem: presume you've seen the CI job logs that shows this working too.. it's handy to trawl that looking for things to line up | 19:37 |
dansmith | and also, sean-k-mooney did some manual testing yesterday that was meaningful to me, with edge cases and such | 19:37 |
artom | "+1 I'm still going" is all we can hope from life | 19:38 |
mriedem | dansmith: yes i was https://zuul.opendev.org/t/openstack/build/d51f91efa937411a9179b930eff3ab08/log/controller/logs/screen-n-cpu.txt.gz#2199 | 19:43 |
* dansmith nods | 19:44 | |
efried | rebase only... | 19:44 |
openstackgerrit | Eric Fried proposed openstack/nova master: Nova object changes for forbidden aggregates request filter https://review.opendev.org/671072 | 19:44 |
openstackgerrit | Eric Fried proposed openstack/nova master: DB API changes to get non-matching aggregates from metadata https://review.opendev.org/671074 | 19:44 |
openstackgerrit | Eric Fried proposed openstack/nova master: Add a new request filter to isolate aggregates https://review.opendev.org/671075 | 19:44 |
openstackgerrit | Eric Fried proposed openstack/nova master: Enable request filter isolate_aggregates https://review.opendev.org/667952 | 19:44 |
* artom -> away | 19:45 | |
mriedem | dansmith: there is no up-call in this https://review.opendev.org/#/c/656594/ | 19:47 |
mriedem | that code is only ever called from the api | 19:47 |
dansmith | mriedem: is it? | 19:47 |
dansmith | I thought we called down to the compute which did this | 19:48 |
mriedem | no | 19:48 |
mriedem | maybe nova-net, idk about that, nor really care about nova-net | 19:48 |
mriedem | this is just a proxy call from nova-api to neutron | 19:49 |
dansmith | sure, I just thought we still had to do FIP associate on the compute, like for LB or something | 19:49 |
mriedem | http://codesearch.openstack.org/?q=network_api%5C.associate_floating_ip&i=nope&files=&repos= | 19:49 |
mriedem | i guess mogan cares | 19:49 |
dansmith | okay, well, I did say I hadn't chased all the plumbing | 19:50 |
*** jmlowe has joined #openstack-nova | 19:50 | |
dansmith | I think the thing that led me there, aside from artom's comment, | 19:51 |
dansmith | was that you're saying "if we don't have access to the API DB" which can't happen without nothing else working, if it's just in the api | 19:51 |
dansmith | and the log message to that effect | 19:52 |
dansmith | the other change you reference was to handle calling code that could be either api-level or inside the cell | 19:52 |
dansmith | but if this is only ever at the API, then I don't think there's any need to catch that situation | 19:52 |
dansmith | and if you do, | 19:53 |
dansmith | it's not info-level, it's error-level, | 19:53 |
dansmith | along with all the other tracebacks the api would be throwing if you even got that far :) | 19:53 |
*** nweinber_ has quit IRC | 19:54 | |
mriedem | ok, like i said, the CantStartEngineError thing came up in denver i think, but it's not written down so i can't remember the details for sure - i don't need it, and agree it shouldn't happen - and if it does, we're more f'ed than just this, so i'm happy to remove that in a follow up when i fix that comment in the test | 19:56 |
mriedem | or, if i'm backporting, just nack it and i can remove that now | 19:57 |
dansmith | ack, commenting | 19:57 |
dansmith | mriedem: done. left my +2, you can decide to fix or fup | 19:58 |
dansmith | for backport, I'd think it's worth fixing before it lands, since it's like 24 hours away :) | 19:59 |
*** eharney has quit IRC | 20:01 | |
mriedem | to be clear, | 20:04 |
mriedem | "So, if you're going to catch it and log, I'd say do so at error and then re-raise the exception like all the other API code would." | 20:04 |
mriedem | you're OK with just removing that catch right? | 20:04 |
dansmith | yes, better to just remove it, imho | 20:05 |
openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Find instance in another cell during floating IP re-association https://review.opendev.org/656594 | 20:05 |
dansmith | mriedem: you doing it now? I'll stand by to fast approve if so | 20:06 |
mriedem | no, still going through this big numa lm patch | 20:07 |
mriedem | just got through the commit message... | 20:07 |
openstackgerrit | Eric Fried proposed openstack/nova master: Nova object changes for forbidden aggregates request filter https://review.opendev.org/671072 | 20:08 |
openstackgerrit | Eric Fried proposed openstack/nova master: DB API changes to get non-matching aggregates from metadata https://review.opendev.org/671074 | 20:08 |
openstackgerrit | Eric Fried proposed openstack/nova master: Add a new request filter to isolate aggregates https://review.opendev.org/671075 | 20:08 |
openstackgerrit | Eric Fried proposed openstack/nova master: Docs for isolated aggregates request filter https://review.opendev.org/667952 | 20:08 |
efried | dansmith: gotta go on next chauffeur run, but I think that's what we were shooting for ^ | 20:09 |
efried | basically half of the .py changes needed to be in the first patch, the other half in the third. | 20:10 |
efried | guess I should have done that field rename while I was in there, sec... | 20:11 |
openstackgerrit | Eric Fried proposed openstack/nova master: Nova object changes for forbidden aggregates request filter https://review.opendev.org/671072 | 20:15 |
openstackgerrit | Eric Fried proposed openstack/nova master: DB API changes to get non-matching aggregates from metadata https://review.opendev.org/671074 | 20:15 |
openstackgerrit | Eric Fried proposed openstack/nova master: Add a new request filter to isolate aggregates https://review.opendev.org/671075 | 20:15 |
openstackgerrit | Eric Fried proposed openstack/nova master: Docs for isolated aggregates request filter https://review.opendev.org/667952 | 20:15 |
mriedem | totally unrelated to the numa live migration stuff, but can this fail the 'claim' for sriov pci devices on the dest host? https://github.com/openstack/nova/blob/f7f5e1846c7b19aa05817df7f3c4345819db413f/nova/compute/manager.py#L6491 | 20:16 |
mriedem | and if so, how does it fail? | 20:16 |
*** efried is now known as efried_afk | 20:16 | |
mriedem | because there is a very specific set of exceptions that will trigger a reschedule to an alternate host https://github.com/openstack/nova/blob/f7f5e1846c7b19aa05817df7f3c4345819db413f/nova/conductor/tasks/live_migrate.py#L482 | 20:17 |
mriedem | so if that stuff doesn't result in MigrationPreCheckError we'll fail the live migratoin b/c the first host claim failed | 20:17 |
mriedem | sean-k-mooney: ^ | 20:17 |
openstackgerrit | Dan Smith proposed openstack/nova master: Find instance in another cell during floating IP re-association https://review.opendev.org/656594 | 20:18 |
mriedem | trying to follow that pci claim code is going down a rabbit hole | 20:18 |
dansmith | mriedem: ^ | 20:18 |
mriedem | want to hit that test comment while you're updating? | 20:20 |
*** bbowen_ has joined #openstack-nova | 20:21 | |
*** lpetrut has joined #openstack-nova | 20:34 | |
*** lpetrut has quit IRC | 20:38 | |
*** ociuhandu has joined #openstack-nova | 20:39 | |
openstackgerrit | Merged openstack/nova master: neutron: refactor nw info cache refresh out of associate_floating_ip https://review.opendev.org/678300 | 20:43 |
*** ociuhandu has quit IRC | 20:45 | |
*** ociuhandu has joined #openstack-nova | 20:46 | |
*** gbarros has quit IRC | 20:50 | |
*** ociuhandu has quit IRC | 20:50 | |
mriedem | oooo https://github.com/openstack/nova/blob/f7f5e1846c7b19aa05817df7f3c4345819db413f/nova/scheduler/filter_scheduler.py#L173-L187 - if it weren't for USES_ALLOCATION_CANDIDATES we could burn all of that queens compat code out of the filter scheduler | 20:56 |
mriedem | artom: dansmith: ok comments in the big one https://review.opendev.org/#/c/634606/75 | 20:57 |
*** gbarros has joined #openstack-nova | 21:00 | |
*** markvoelker has quit IRC | 21:05 | |
openstackgerrit | Matt Riedemann proposed openstack/nova master: Find instance in another cell during floating IP re-association https://review.opendev.org/656594 | 21:05 |
dansmith | mriedem: cool thanks | 21:05 |
mriedem | ok i think that multi-cell floating ip thing is ready | 21:05 |
artom | mriedem, thanks! It's supper time now, will work a bit after, then gym, so tomorrow morning | 21:06 |
artom | Haven't been to gym in, like, a week, not cool | 21:06 |
openstackgerrit | Merged openstack/nova master: Trap and log errors from _update_inst_info_cache_for_disassociated_fip https://review.opendev.org/678301 | 21:08 |
*** markvoelker has joined #openstack-nova | 21:11 | |
mriedem | gerrit is down anyway | 21:13 |
-openstackstatus- NOTICE: Gerrit is being restarted to pick up configuration changes. Should be quick. Sorry for the interruption. | 21:14 | |
*** markvoelker has quit IRC | 21:15 | |
*** gbarros has quit IRC | 21:18 | |
openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove old comments about caching scheduler compat https://review.opendev.org/680521 | 21:21 |
openstackgerrit | Eric Fried proposed openstack/nova master: Add a new request filter to isolate aggregates https://review.opendev.org/671075 | 21:36 |
openstackgerrit | Eric Fried proposed openstack/nova master: Docs for isolated aggregates request filter https://review.opendev.org/667952 | 21:36 |
mriedem | alex_xu: can you take a look at the api change to attach a volume (and list/show vol attachments) with delete_on_termination? https://review.opendev.org/#/c/673133/ - it's probably going to be 2.79; i'm +2 on it | 21:37 |
*** mdbooth_ has quit IRC | 21:41 | |
openstackgerrit | Adam Spiers proposed openstack/nova master: Ensure non-q35 machine type is not used when booting with SEV https://review.opendev.org/680065 | 21:42 |
openstackgerrit | Adam Spiers proposed openstack/nova master: Extract fake KVM guest fixture for reuse https://review.opendev.org/680526 | 21:43 |
openstackgerrit | Adam Spiers proposed openstack/nova master: Move get_machine_type() test to test_utils.py https://review.opendev.org/680527 | 21:44 |
efried_afk | dansmith: vpmem json blob => hyp-specific ovo (https://review.opendev.org/#/c/678448/), you said to follow the example of LiveMigrateData. | 21:57 |
efried_afk | So I think that means they should make a base class (e.g. ResourceMetadata) and make LibvirtPMEMDevice (https://review.opendev.org/#/c/678453/19/nova/objects/resource.py) a subclass thereof. | 21:57 |
efried_afk | I can't see where LiveMigrateData is a field in another ovo, though; is it kosher to make Resource.metadata of type ResourceMetadata even though IRL it will always be a *subclass*? | 21:57 |
openstackgerrit | Matt Riedemann proposed openstack/nova master: Rename Claims resources to compute_node https://review.opendev.org/679470 | 21:57 |
dansmith | yeah, although you might have to register the base to make that work, which I don't think was required for the MD case because we always sent the child and not the parent, IIRC, but it's been a whiel | 21:58 |
efried_afk | what does "register the base" mean? | 21:58 |
*** efried_afk is now known as efried | 21:58 | |
mriedem | @obj_base.NovaObjectRegistry.register | 21:59 |
mriedem | note that the base LiveMigrateData is not registered | 22:00 |
mriedem | https://github.com/openstack/nova/blob/master/nova/objects/migrate_data.py#L112 | 22:00 |
mriedem | b/c everything uses the subclasses | 22:00 |
dansmith | right that ^ | 22:00 |
dansmith | efried: ovo requires a shared registry of the schemas on either side to ensure proper versioning | 22:00 |
efried | everything will use the subclass in this case too... unless you mean that the Resource object will "use" the base class? | 22:00 |
mriedem | so i can't do something like this in my driver: migrate_data = objects.LiveMigrateData(all_my_cool_shenanigans={...}) | 22:00 |
dansmith | efried: the difference is the inclusion in a field, which has to specify an object in the registry, IIRC | 22:01 |
dansmith | which is why it's by name and not by reference | 22:01 |
efried | okay, cool. | 22:01 |
*** slaweq has quit IRC | 22:03 | |
efried | dansmith, mriedem: Did I say it right? https://review.opendev.org/#/c/678448/15/nova/objects/resource.py@36 | 22:03 |
mriedem | i don't know that the chinese are going to understand the south park underpants gnomes reference | 22:05 |
efried | I've never actually even seen that thing | 22:05 |
efried | indirect meme reference culture | 22:05 |
* mriedem resists the urge to youtube | 22:05 | |
dansmith | efried: commented | 22:05 |
efried | I could find it if I wanted | 22:05 |
*** luksky has quit IRC | 22:06 | |
efried | dansmith: thanks. I think nullable because not all ResourceZ are going to have metadata. | 22:06 |
mriedem | then don't set the field | 22:06 |
dansmith | efried: yeah, that's the wrong answer | 22:06 |
efried | ah | 22:06 |
efried | unset vs value of None vs object with no fields initialized vs object with fields set to None vs ..... | 22:07 |
dansmith | I thought I already commented about this in this patch, but maybe I was overwhelmed by the larger sin of using a string here and it didn't come through | 22:07 |
mriedem | i need to make dan a shirt that has nullable=True on some object field with the circle bar banned icon on it | 22:07 |
dansmith | I'd wear it | 22:07 |
*** claudiub has joined #openstack-nova | 22:08 | |
dansmith | efried: I suspect the right thing there is to always have a metadata object in that field (hence never None), with either fields set appropriately empty or whatever else, | 22:08 |
dansmith | but it's hard to tell what the right thing is because it's not defined there | 22:08 |
dansmith | that's probably why I didn't comment, because I don't know WHAT that is, other than a dumping ground for blobs | 22:08 |
mriedem | it's this thing i think https://review.opendev.org/#/c/678453/19/nova/objects/resource.py | 22:10 |
efried | it's for hypervisor-specific, resource-specific metadata for a Resource. And not all ResourceZ are going to have any. It would seem like a lot of work to a) make sure to set the value b) to an empty object, meaning c) you have to make sure the object type can be "empty", whatever that means, etc. | 22:10 |
mriedem | used here https://review.opendev.org/#/c/678454/19/nova/virt/libvirt/driver.py@6934 | 22:10 |
mriedem | efried: not empty, just don't set the field | 22:10 |
efried | so does "nullable=False" just mean you're not allowed to set it to None? | 22:10 |
mriedem | correct | 22:10 |
efried | and that's the default behavior. | 22:10 |
mriedem | yeah | 22:11 |
dansmith | correct | 22:11 |
efried | and you check with | 22:11 |
efried | if 'metadata' in res_obj: | 22:11 |
efried | ? | 22:11 |
*** slaweq has joined #openstack-nova | 22:11 | |
dansmith | if there's really a reason to have it =None then it _should_ be nullable, | 22:11 |
dansmith | but for something like this, the presence of the metadata object isn't the thing you want, it's what is inside | 22:11 |
mriedem | efried: yup | 22:11 |
dansmith | so if you always make these things nullable=True, then you always have to check first | 22:12 |
dansmith | like, if that was what it was intended to be in the first place, you'd have to do: | 22:12 |
dansmith | if foo.metadata: if parse(foo.metadata): do_something(foo.metadata) | 22:12 |
dansmith | when really you want to just be able to do something like | 22:12 |
dansmith | if foo in resources.metadata | 22:13 |
dansmith | or something like that | 22:13 |
dansmith | take a degree of freedom out of the thing by always making it present | 22:13 |
mriedem | if you like mega conditionals https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L760 | 22:13 |
efried | egads | 22:13 |
mriedem | RequestSpec is i think the king of "if a and b and c and not d" | 22:13 |
dansmith | because later when you have two versions out there, ambiguity is the enemy | 22:13 |
efried | cool, thanks for the primer. This was easier than having 12h between each call & response. | 22:15 |
dansmith | not for me | 22:15 |
mriedem | time to start a versioned objects FAQs doc | 22:15 |
dansmith | easier to ignore with 12h latency | 22:16 |
dansmith | on my blog, which is apparently the canonical source for nova RPC information | 22:16 |
openstackgerrit | Adam Spiers proposed openstack/nova master: Apply SEV-specific guest config when SEV is required https://review.opendev.org/644565 | 22:16 |
mriedem | heh, you know it | 22:16 |
mriedem | pulling stuff from slides 5-8 here (which is a lot based on the blog) into nova docs might be good to start https://docs.google.com/presentation/d/1YeXlL1EPaZTRkbJDywJKBrugXz-Amri3fIQk2ctOugs/edit?usp=sharing | 22:17 |
efried | dustinc: you around? | 22:18 |
mriedem | note slide 11 :) | 22:18 |
dustinc | efried: yeah what’s up? | 22:18 |
efried | dustinc: I think node.list patch has a regression -- did you see https://review.opendev.org/#/c/656027/29/nova/virt/ironic/driver.py@690 | 22:18 |
dansmith | mriedem: lol | 22:19 |
openstackgerrit | Adam Spiers proposed openstack/nova master: Reject live migration and suspend on SEV guests https://review.opendev.org/680158 | 22:19 |
efried | dustinc: if that's the case, it means we're missing test coverage in the CI job and we should add it. | 22:19 |
dustinc | efried: haven’t really been paying attention to those last two with the providers.yaml work. I will check it out. | 22:20 |
efried | dustinc: this is one already merged, so it takes priority over feature work. | 22:20 |
efried | dustinc: on inspection, it makes sense I think, because we're asking the API for the instance_id field instead of the instance_uuid field. | 22:21 |
*** slaweq has quit IRC | 22:21 | |
efried | this is a pain point with the sdk translation of uuid->id everywhere. | 22:21 |
efried | mordred: fyi | 22:21 |
dustinc | efried oh I see, on it now | 22:22 |
efried | so basically, queries *into* the API need to say uuid, but processing the results on the way out need to say id. | 22:22 |
efried | I think. | 22:22 |
efried | dustinc: You may want to ask for help in -ironic to add coverage to that CI job. Unless you already have it wired. I wouldn't know where to begin. | 22:22 |
openstackgerrit | Adam Spiers proposed openstack/nova master: Enable booting of libvirt guests with AMD SEV memory encryption https://review.opendev.org/666616 | 22:22 |
aspiers | efried: OK, I'm done for the last time ... until next time ^^^ | 22:23 |
aspiers | couple of very simple new patches in there | 22:23 |
aspiers | and more thorough tests than ever | 22:24 |
efried | dustinc: looks like it may be just tempest with configs to set up the ironic driver; which would mean that tempest in general doesn't exercise list_instances... | 22:27 |
*** bbowen_ has quit IRC | 22:33 | |
*** BjoernT_ has quit IRC | 22:35 | |
*** kaisers has quit IRC | 22:37 | |
*** kaisers has joined #openstack-nova | 22:38 | |
openstackgerrit | Matt Riedemann proposed openstack/nova master: doc: cleanup references to conductor doc https://review.opendev.org/680535 | 22:39 |
*** igordc has quit IRC | 22:48 | |
*** mriedem has quit IRC | 22:50 | |
*** tkajinam has joined #openstack-nova | 23:02 | |
*** slaweq has joined #openstack-nova | 23:11 | |
*** slaweq has quit IRC | 23:16 | |
openstackgerrit | Eric Fried proposed openstack/nova master: Bump min for oslo.service & .privsep to fix SIGHUP https://review.opendev.org/679974 | 23:17 |
*** rcernin has joined #openstack-nova | 23:23 | |
*** macz has joined #openstack-nova | 23:24 | |
*** macz has quit IRC | 23:26 | |
*** macz has joined #openstack-nova | 23:27 | |
aspiers | Is it possible to configure zuul to fail early if certain jobs fail? | 23:28 |
aspiers | For example 680296,1 has already failed openstack-tox-pep8 but all the other jobs are still running, taking up valuable CI resources | 23:28 |
*** macz has quit IRC | 23:29 | |
aspiers | In fact there's a large CI backlog which would probably vanish quickly if this were possible | 23:29 |
*** threestrands has joined #openstack-nova | 23:30 | |
aspiers | Hmm, seems to be possible via pipeline.failure | 23:31 |
openstackgerrit | Nathan Kinder proposed openstack/nova master: Allow TLS ciphers/protocols to be configurable for console proxies https://review.opendev.org/679502 | 23:36 |
dustinc | efried: You still online? | 23:45 |
openstackgerrit | Dustin Cowles proposed openstack/nova master: Use fields="instance_uuid" when calling Ironic API https://review.opendev.org/680542 | 23:51 |
openstackgerrit | Dustin Cowles proposed openstack/nova master: Use fields="instance_uuid" when calling Ironic API https://review.opendev.org/680542 | 23:52 |
*** rcernin is now known as rcernin|brb | 23:52 | |
*** bbowen has joined #openstack-nova | 23:58 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!