| opendevreview | Merged openstack/nova stable/ussuri: Reproduce bug 1953359 https://review.opendev.org/c/openstack/nova/+/822047 | 03:29 |
|---|---|---|
| opendevreview | Merged openstack/nova stable/ussuri: Extend the reproducer for 1953359 and 1952915 https://review.opendev.org/c/openstack/nova/+/822048 | 03:29 |
| opendevreview | Merged openstack/nova stable/ussuri: [rt] Apply migration context for incoming migrations https://review.opendev.org/c/openstack/nova/+/822050 | 04:28 |
| gibi | sean-k-mooney: about hw:bfv=True. I don't think putting everything into flavor is a good idea. We already have the flavor explosion issue | 06:24 |
| gibi | also in analogous for hw:bfv_type somebody could start asking for hw:port_vnic_type=direct | 06:25 |
| sean-k-mooney[m] | they could but they have wanted to do that for years too | 06:26 |
| gibi | and we said no :) | 06:26 |
| sean-k-mooney[m] | yep im not sure the flavor explosion issue is reall still an issue | 06:27 |
| sean-k-mooney[m] | i think operators are just used to there being many | 06:27 |
| gibi | we still heard the term last week | 06:27 |
| sean-k-mooney[m] | right but they just accept it and create lots of flavors | 06:27 |
| gibi | OK, so if we say no for a long time to hw:bfv=True then they will accept that too :) and stop asking for more orchestrations | 06:28 |
| sean-k-mooney[m] | like some of the telco provideders litrally create seperate flavors for every application in a multi app vnf automaticaly via heat | 06:29 |
| sean-k-mooney[m] | well the latter will never happen | 06:29 |
| gibi | :) | 06:29 |
| sean-k-mooney[m] | but the flavor may not be the right place | 06:29 |
| sean-k-mooney[m] | i dont thing its unreasonable to have a way to say always uses cinder volumes | 06:30 |
| sean-k-mooney[m] | or always to it on these set of hosts by default | 06:30 |
| gibi | if it is _always_ then this can be a config option | 06:30 |
| sean-k-mooney[m] | not it cant be | 06:30 |
| sean-k-mooney[m] | that would be config driven api behavior | 06:31 |
| sean-k-mooney[m] | which is one of the reasons we didnt like having a cinder images type | 06:31 |
| gibi | or even the admin can define the nova instance dir to be in a place with close to 0 disk space to force it | 06:31 |
| sean-k-mooney[m] | well they can force it differntly via config options to day but its not somethign you can discover without trying to boot a vm | 06:32 |
| sean-k-mooney[m] | they are stilll raising it as a pain point so obviouls they still want a way to do this | 06:32 |
| sean-k-mooney[m] | i tried to adress this issue differntly in the past by proposing the idea of vm porfiles to have a semi composable flavor | 06:33 |
| sean-k-mooney[m] | the profiles would only have extra specs that did not change resouce usage so therefor did not change billing | 06:35 |
| gibi | extra spec can reqeust pinning that might be billed differently. | 06:35 |
| gibi | but I feel I'm too negative today. sorry | 06:35 |
| sean-k-mooney[m] | as a use you can request that in your image | 06:35 |
| sean-k-mooney[m] | so if the falvor has not explictly disabled it you can always do that | 06:36 |
| gibi | still dedicated cpus are billed differently in public clouds afaik | 06:36 |
| sean-k-mooney[m] | right but that trivial to adress | 06:36 |
| sean-k-mooney[m] | just put hw:cpu_policy=shared in the base flavors | 06:36 |
| sean-k-mooney[m] | today if you do not do that you can just put hw_cpu_policy=deicated in the image | 06:37 |
| sean-k-mooney[m] | i assumed you were going to bring up resouce: | 06:38 |
| gibi | so in my mind this is similar to network connectivity. if you need it pass a pre-created port to nova. so I would move the whole disk question out from the flavor too. If you need disk (:D) then pass either a pre-created local-disk or a pre-created volume to the boot command. | 06:38 |
| sean-k-mooney[m] | which is why i said extra specs that dont affect resouce useage | 06:38 |
| sean-k-mooney[m] | fair so we should not add delete on terminate for port in the future right :P | 06:39 |
| gibi | honestly. It is different what I would like to have and what we will do. I accept that | 06:39 |
| gibi | :P | 06:39 |
| sean-k-mooney[m] | hehe well we are both to busy to work on this in the near term | 06:40 |
| bauzas | hola | 06:40 |
| gibi | I would remove the whole --nic net-id if I could | 06:40 |
| sean-k-mooney[m] | so just brainstorming | 06:40 |
| bauzas | not sure I have the whole context | 06:40 |
| gibi | yepp, it is just brainstorming | 06:40 |
| gibi | bauzas: morning | 06:40 |
| gibi | bauzas: take a seat, grab a coffee | 06:40 |
| bauzas | morning folks | 06:40 |
| sean-k-mooney[m] | bauzas o/ | 06:40 |
| bauzas | gibi: been there, done that :) | 06:40 |
| bauzas | literrally | 06:41 |
| bauzas | we're a Wednesday, I'm happy to work earlier | 06:41 |
| gibi | I did not drink coffee since left Berlin | 06:41 |
| bauzas | OMG | 06:41 |
| gibi | I miss the taste | 06:41 |
| sean-k-mooney[m] | i dont drink it on weekends which people find odd | 06:41 |
| * bauzas can't imagine a day without coffee | 06:41 | |
| bauzas | as I have kids at home, I love those breaks just after lunch with my wife | 06:42 |
| bauzas | I ask my children to go up to their bedrooms and we take 10 mins with a coffee :) | 06:42 |
| sean-k-mooney[m] | thats a nice ritual | 06:42 |
| bauzas | as I wasn't having a coffee, man... | 06:43 |
| gibi | no kids, wife works non-remote so I have a lot of quite moments :D | 06:43 |
| bauzas | how could I break :p | 06:43 |
| bauzas | that said, I also had beers | 06:43 |
| gibi | no beer for me since Berlin too :) | 06:43 |
| sean-k-mooney[m] | i like to play with fryea {emma’s 8 month old puppy) when i grab a cup | 06:43 |
| gibi | yeah I need a cat | 06:44 |
| gibi | so going back to hw:bfv=True | 06:45 |
| sean-k-mooney[m] | bauzas: we were just chatting about the request form operator to have a way to have vms use boot form volume by default without a config option | 06:45 |
| sean-k-mooney[m] | i was wondering how terrible it would be to have an extra spec | 06:45 |
| gibi | I would disaggregate disk from flavor as I dream big | 06:45 |
| sean-k-mooney[m] | gibi you mean like port count | 06:46 |
| gibi | count? | 06:46 |
| sean-k-mooney[m] | well you dont ask for 5 ports in a flavor | 06:46 |
| sean-k-mooney[m] | ports are entirly seperate | 06:46 |
| gibi | yepp like that | 06:47 |
| sean-k-mooney[m] | so you would prefer if disk was also seperate | 06:47 |
| gibi | yepp | 06:47 |
| sean-k-mooney[m] | there sort of in a weird middel ground today | 06:47 |
| sean-k-mooney[m] | cause they can be seperate and in the case of flavor.ephmeral you can subdevide that into multiple disks | 06:48 |
| bauzas | sean-k-mooney: what is the usecase ? | 06:48 |
| bauzas | why they don't want to ask for a volume or other way ? | 06:48 |
| gibi | $ openstack disk create --type [boot|ephemeral|swap] --backend [local-disk|volume] --size | 06:48 |
| gibi | $ openstack server create ... --disk <disk-uuid> | 06:48 |
| sean-k-mooney[m] | i belive they want to have diskless compute or low disk compute and have there tenants always use cinder storage | 06:49 |
| sean-k-mooney[m] | but they didint want there users to have to rememebr to do that, wasnt in the room :) | 06:50 |
| sean-k-mooney[m] | its the last line on the meet and greet doc | 06:50 |
| sean-k-mooney[m] | gibi that is an intersting idea | 06:50 |
| gibi | or even | 06:50 |
| gibi | # admin | 06:50 |
| gibi | $ openstack disk profile create --type [boot|ephemeral|swap] --backend [local-disk|volume] --size | 06:50 |
| gibi | # user | 06:51 |
| gibi | $ openstack disk create --profile <profile-uuid> | 06:51 |
| gibi | to have the way the admin limit the sizes and types | 06:51 |
| sean-k-mooney[m] | right i prefer the latter but that is getting into composable flavors | 06:51 |
| gibi | exactly | 06:51 |
| gibi | so it solves both problems :D | 06:51 |
| gibi | (no it does not ) | 06:51 |
| bauzas | sean-k-mooney: then maybe they should provide flavors with DISK_GB=0 | 06:52 |
| sean-k-mooney[m] | right if we did that i would prefer to do that for all resouces | 06:52 |
| sean-k-mooney[m] | bauzas they can yes | 06:52 |
| sean-k-mooney[m] | with our default policy that would for bfv usage | 06:52 |
| bauzas | if you create a flavor with some disk value, then it would create an ephemeral volue | 06:52 |
| sean-k-mooney[m] | *force | 06:53 |
| sean-k-mooney[m] | no not always | 06:53 |
| bauzas | so if you have diskless computes, don't create those flavors | 06:53 |
| sean-k-mooney[m] | people pushed back when i suggested we should not allow you do do boot form volume with flavor wiht disk!=0 | 06:53 |
| sean-k-mooney[m] | well what if you have a mix some diskless some with local storage | 06:54 |
| sean-k-mooney[m] | psi has fast nvme storage on the ci aggrate and the general aggreate is backed by ceph | 06:54 |
| sean-k-mooney[m] | that is why we litrally have ci.* and general.* flavors | 06:55 |
| sean-k-mooney[m] | as a user you are ment to rember to use qcow with ci* and raw images with general* | 06:55 |
| sean-k-mooney[m] | gibi to your point i do think composable flaors is proably the best approch but everyone hates thoses so meh | 06:56 |
| sean-k-mooney[m] | and by that somehting liek the —disk-profile you were suggesting | 06:57 |
| gibi | yeah. also I don't think we have workforce to make composable happen | 06:57 |
| gibi | so it is just a dream | 06:57 |
| sean-k-mooney[m] | we can hope | 06:58 |
| sean-k-mooney[m] | there were simpler pain points to adress frist | 06:59 |
| gibi | sorry again for being extra negative this morning | 06:59 |
| sean-k-mooney[m] | im interested to see how many of the quota pain points will get addressed by unifed limits | 06:59 |
| sean-k-mooney[m] | gibi: its oke eventully you will underflow and be very very positive again :) | 07:00 |
| bauzas | sorry, I think I still don't get the problem about what we miss | 07:00 |
| bauzas | but I'm of course wrong | 07:00 |
| gibi | sean-k-mooney[m]: I'm waiting for that :D | 07:01 |
| sean-k-mooney[m] | bauzas a simple declaritive way to say that vms should use bfv | 07:01 |
| sean-k-mooney[m] | the command line is imperitive | 07:01 |
| sean-k-mooney[m] | i would have said images_type=cinder | 07:02 |
| sean-k-mooney[m] | but they expictly said not via a host level config option | 07:02 |
| bauzas | sean-k-mooney: wdym by imperative ? | 07:02 |
| sean-k-mooney[m] | bauzas as a user i have to know and remember to ask for bfv | 07:03 |
| jkulik | just to chime in, we also want flavors that default to boot-from-volume instead of ephemeral disk without the user having to explicitly specify bfv | 07:03 |
| sean-k-mooney[m] | as an operator i cant make that default by declaring it in a flaovr or image | 07:03 |
| bauzas | oh, because of --block-device | 07:03 |
| bauzas | as this is a flag, this can be missed by the user, gotcha | 07:03 |
| sean-k-mooney[m] | yep | 07:03 |
| sean-k-mooney[m] | hence hw:bfv=true in flaovr | 07:04 |
| sean-k-mooney[m] | meaing that this flavor would always use bfv | 07:04 |
| bauzas | well, if you ask for disk=0 with no block device, you end up with ERROR, right? | 07:04 |
| sean-k-mooney[m] | yes | 07:04 |
| gibi | did we fixed that? | 07:04 |
| sean-k-mooney[m] | actully no | 07:04 |
| bauzas | so, basically, users are pushed to add a block device | 07:04 |
| sean-k-mooney[m] | ya so it depned on policy | 07:04 |
| gibi | in the past disk=0 meant no enforcement | 07:04 |
| sean-k-mooney[m] | correct | 07:05 |
| gibi | ahh OK | 07:05 |
| bauzas | honestly, I just want to make sure disk=0 is meaningful | 07:05 |
| sean-k-mooney[m] | so default policy requires admin for the unbounded case | 07:05 |
| bauzas | if you don't ask for a local disk, then you're forced to add a volume | 07:05 |
| sean-k-mooney[m] | it kind of is as an end user with just the member role you cannot boot with bfv if disk=0 | 07:06 |
| sean-k-mooney[m] | *without | 07:06 |
| bauzas | unless people want some OS residing fully in memory | 07:06 |
| sean-k-mooney[m] | right but that imperitive | 07:06 |
| sean-k-mooney[m] | it doesnt just work the same as any other flavor but use cinder netapp storage | 07:06 |
| bauzas | sean-k-mooney: again, I could be wrong, but this behaviour seems correct to me | 07:06 |
| bauzas | which is, nova won't provide you disk for your instance | 07:07 |
| bauzas | then you have to provide it | 07:07 |
| sean-k-mooney[m] | jkulik: did you want to expand on this a little | 07:07 |
| jkulik | this was the approach we fiddled with https://github.com/sapcc/nova/commit/e65287727ab5f78fbcf3f6da26a54456eaebf932 it will automatically add a boot_index=0 BDM if the flavor has a property boot_from_volume=true | 07:07 |
| jkulik | as we don't want to let our customers re-learn how to create VMs. it should "just work" like with ephemeral disks | 07:08 |
| bauzas | in the past, we said those magics behind the hood were nearly orchestration | 07:09 |
| sean-k-mooney[m] | ah you remembered to set delete on terminate to true too | 07:09 |
| sean-k-mooney[m] | jkulik: it would be less maintance for you to do that via middleware by the way | 07:10 |
| sean-k-mooney[m] | right but im not sure nova should avoid all orchestration at the expense of ux | 07:11 |
| sean-k-mooney[m] | i generally agree we should not add any complex orchestation | 07:11 |
| jkulik | sean-k-mooney: can you point me to some docs on that? I can see how we could edit a request coming in, but I wouldn't expect a middleware to be able to query Nova's DB to get the flavor | 07:12 |
| sean-k-mooney[m] | and im sure my deffintion and yours of complex willl differ and thats ok too :) | 07:12 |
| sean-k-mooney[m] | jkulik well it runs in the api process so it can access the the nova.conf | 07:12 |
| sean-k-mooney[m] | i dont think we have example that hit the db today | 07:13 |
| jkulik | sounds wrong to me, tbh | 07:13 |
| sean-k-mooney[m] | less wrong then downstream modifications to the source code :) | 07:13 |
| jkulik | haha, yes, well ... I've given up on not having those | 07:14 |
| sean-k-mooney[m] | i was just suggesting if you have rebase issues due to this maybe middleware would help | 07:14 |
| sean-k-mooney[m] | but i did not think about db access for the flavor definition | 07:15 |
| jkulik | and I thank you for the advice. I had never thgouth about that. | 07:15 |
| sean-k-mooney[m] | for what its worth the keystone middelware makes rest calls to validate the auth token so they can be complex but normally they are simple | 07:16 |
| bauzas | sean-k-mooney: sorry was disturbed by some paperwork due to a flight cancelled | 07:20 |
| bauzas | thanks Europe, I'm owed 250€ | 07:20 |
| bauzas | sean-k-mooney: well, surely we can improve the UX | 07:21 |
| sean-k-mooney[m] | the question is how :) | 07:21 |
| bauzas | I just wanted to express the general thought that was 'if it implies kind of roundtrips between multiple projets and a lot of conditionals, then we should maybe discuss on the opportunity to make it a client thing' | 07:22 |
| jkulik | by linking to the docs in the error-message? :D | 07:22 |
| sean-k-mooney[m] | we used to punt on this and say use a heat template | 07:22 |
| bauzas | now we have openstackcli | 07:22 |
| sean-k-mooney[m] | well there is a simple way to do it in the client now i think | 07:22 |
| bauzas | and thanks to hard efforts of stephenfin and a couple of others, we are able to have it done in the CLI | 07:23 |
| sean-k-mooney[m] | that does not really solve the problem that uers have to rememebr to do it | 07:23 |
| jkulik | most of our users don't use openstackcli either | 07:23 |
| bauzas | directly the REST APIs, heh ? | 07:24 |
| sean-k-mooney[m] | jkulik do they use api directly/via a differnt client or horizon/heat | 07:24 |
| jkulik | $some library or yes, the REST APIs :( | 07:24 |
| bauzas | if so, they are powered users that can understand the need for block storage if disk=0 | 07:24 |
| sean-k-mooney[m] | ah hard core curl users :) | 07:24 |
| jkulik | iirc gophercloud is much used, but there was also a Java library that's in much use | 07:24 |
| sean-k-mooney[m] | bauzas not nessisarly | 07:25 |
| jkulik | right. they build their own library and it takes ages to make them update anything :D that's why we would like to be able to do it on our side | 07:25 |
| sean-k-mooney[m] | they could just be developers of applcition that run on openstack clouds | 07:25 |
| bauzas | fair enough | 07:25 |
| bauzas | but again, conceptually, if we fail with a flavor of disk=0, then I think we're consistent | 07:26 |
| sean-k-mooney[m] | jkulik so you have that patch in production i take it | 07:26 |
| sean-k-mooney[m] | has it helpped | 07:26 |
| bauzas | the problem would be to accept a flavor with disk=0 and magically create ephemeral storage | 07:26 |
| jkulik | no, we don't have it in production. we lacked too much other functionality for bfv VMs (we're still on rocky, e.g. rescue) | 07:26 |
| sean-k-mooney[m] | bauzas that is what we did untill around rocky issue i think | 07:26 |
| bauzas | yup | 07:27 |
| bauzas | I remember this | 07:27 |
| bauzas | point is, look at the figures of the meet-and-greet | 07:27 |
| sean-k-mooney[m] | jkulik so i think rescue is now there and rebuild is in flight this cycle | 07:27 |
| jkulik | but it's not just disk=0, it's also that the flavor is explicitly marked for automatic bfv, right? | 07:27 |
| bauzas | probably the one who requested for a bfv flag was hit by the fact he/she was running older than Rocky | 07:27 |
| jkulik | sean-k-mooney: yeah, looking forward to it. we're also trying to jump to xena by this year | 07:27 |
| bauzas | jkulik: that's what I call 'orchestration' | 07:28 |
| bauzas | the 'automatic' side | 07:28 |
| jkulik | ... and no orchestration in Nova itself | 07:29 |
| sean-k-mooney[m] | jkulik for context novas project scope doc declares orchstration as out of scope and we tend to define that as any addtional inter service operation that can be done by a user and the result passed in to nova | 07:29 |
| jkulik | like creating a volume from image, which Nova already does? | 07:30 |
| sean-k-mooney[m] | i.e. create your port/volume ahead of time and tell nova to use it | 07:30 |
| bauzas | https://docs.openstack.org/nova/latest/contributor/project-scope.html#no-more-orchestration | 07:30 |
| sean-k-mooney[m] | jkulik yep so nova only can do that because we need to support that before cinder was split out | 07:30 |
| bauzas | jkulik: you're exactly pointing some orchestration we keep | 07:30 |
| bauzas | because of the API consistency | 07:31 |
| sean-k-mooney[m] | cinder started as nova-volume | 07:31 |
| jkulik | ok, if we keep it, we can still use it, right? I'm mean in the end it's just a flavor using exactly that | 07:31 |
| sean-k-mooney[m] | the same way ironic started as nova-baremetal | 07:31 |
| sean-k-mooney[m] | jkulik we likely will never remove it | 07:31 |
| sean-k-mooney[m] | we would have to raise our min api verion to do so | 07:31 |
| sean-k-mooney[m] | and we wont do that any time soon | 07:32 |
| sean-k-mooney[m] | if ever | 07:32 |
| bauzas | never | 07:32 |
| jkulik | :) | 07:32 |
| bauzas | in particular given the lag we have | 07:32 |
| sean-k-mooney[m] | bauzas well i object to never | 07:32 |
| bauzas | sean-k-mooney: okay, let's be less pragmatic | 07:32 |
| jkulik | so imho, if the user comes in with a BDM of image -> volume, we create the volume. why can't we specify the flavor to default to image -> volume? | 07:32 |
| bauzas | "could be, eventually" | 07:32 |
| sean-k-mooney[m] | as i think its wrong for use to have a min version filed if that is the stance we are taking | 07:32 |
| sean-k-mooney[m] | should be eventurally | 07:33 |
| bauzas | sean-k-mooney: I was there in 2015 when we drafted v2 | 07:33 |
| bauzas | v2.1 actually | 07:33 |
| bauzas | we create min_version because we were considering it | 07:33 |
| bauzas | created* | 07:34 |
| sean-k-mooney[m] | yep and at that time we planned to increase it eventually | 07:34 |
| bauzas | but given interop and other reasons, we ended up being less optimistic | 07:34 |
| sean-k-mooney[m] | jkulik what volume_type should be used | 07:34 |
| sean-k-mooney[m] | to create the volume form the image | 07:34 |
| sean-k-mooney[m] | should we delete it on terminate | 07:34 |
| jkulik | that's already a decision Nova has to make | 07:34 |
| bauzas | nope | 07:34 |
| jkulik | then Nova does nothing and it's the default volume type of Cinder | 07:35 |
| bauzas | and I don't want config-driven APIs | 07:35 |
| sean-k-mooney[m] | well we use the default volume type | 07:35 |
| sean-k-mooney[m] | which i gues is your point | 07:35 |
| bauzas | we have defaults for sure | 07:35 |
| bauzas | but that's for a volume | 07:35 |
| jkulik | I mean the feature is already there, I just want a flavor to use it | 07:35 |
| sean-k-mooney[m] | yep it is. we just dont have a way to enable it except via an api parmater today | 07:36 |
| bauzas | for many reasons | 07:36 |
| jkulik | right. so my request would be to enable it via flavor parameter | 07:36 |
| bauzas | again, what's the usecase if we force users to add a volume if they have a diskless flavor ? | 07:37 |
| sean-k-mooney[m] | to not force user to do things | 07:37 |
| sean-k-mooney[m] | and provide bettter ux | 07:37 |
| bauzas | we have the "give me a port" thing | 07:37 |
| bauzas | and this isn't within a flavor, right? | 07:38 |
| sean-k-mooney[m] | do you mean give me an network | 07:38 |
| sean-k-mooney[m] | or —network on server create | 07:38 |
| sean-k-mooney[m] | the differnece with a port is that is never in a flaovr to begin with | 07:39 |
| bauzas | sean-k-mooney: not if you do bandwidth-aware requests :) | 07:40 |
| sean-k-mooney[m] | there is nothing in the falvor for that | 07:42 |
| gibi | I think give me a network is when you say --nic auto | 07:42 |
| sean-k-mooney[m] | the bandwith request comes form the port | 07:42 |
| gibi | yepp bw is a disaggregated resource | 07:43 |
| jkulik | why does the flavor have to be specified diskless btw.? Couldn't we just let the operator specify whether the disk should be on ephemeral or on volume storage i.e. determine the deestination_type of the BDM? | 07:43 |
| sean-k-mooney[m] | no give me a network is the neutron feature that will create a network, subnet and router automaticaly to replicate nova-networks behavior | 07:43 |
| gibi | sean-k-mooney[m]: /o\ | 07:43 |
| bauzas | should we ask the same to cinder then ? | 07:44 |
| sean-k-mooney[m] | https://specs.openstack.org/openstack/nova-specs/specs/newton/implemented/get-me-a-network.html | 07:44 |
| bauzas | "give me a volume" | 07:44 |
| bauzas | and nova be --disk auto | 07:44 |
| sean-k-mooney[m] | its called volume create | 07:44 |
| sean-k-mooney[m] | its not the same thing | 07:44 |
| sean-k-mooney[m] | in the neutron case people just wanted the vm to have a port with an ip by defualt | 07:44 |
| sean-k-mooney[m] | but with neutron to do that you ahve to create a networ, subnet and router first | 07:45 |
| bauzas | jkulik: you're right, operators could propose both ephemeral storage and block one | 07:45 |
| bauzas | life would be way easier if we weren't having flavors | 07:46 |
| sean-k-mooney[m] | i was thinking the same | 07:47 |
| bauzas | sean-k-mooney: gibi: this reminds me the Public Cloud SIG session | 07:47 |
| bauzas | sean-k-mooney: gibi: do you know that they stuggle having the same flavor names ? | 07:47 |
| bauzas | struggle | 07:47 |
| sean-k-mooney[m] | disk=0 ram=0 vcpu=0 and just allow —resouce rc=ammount on server create | 07:47 |
| sean-k-mooney[m] | and use unified limits for quota/billing | 07:47 |
| bauzas | proposing a common set of flavors between all Passport operators seemed nearly impossible to achieve | 07:47 |
| sean-k-mooney[m] | ya we should likely not do that | 07:48 |
| sean-k-mooney[m] | the only way to do that is for nova to ship with default flavors | 07:48 |
| bauzas | even that | 07:48 |
| sean-k-mooney[m] | that you can opt into | 07:48 |
| sean-k-mooney[m] | and never update just disable | 07:48 |
| bauzas | they get rid of our default flavors | 07:48 |
| bauzas | OVH has strong concerns about touching their flavors, by instance. | 07:48 |
| sean-k-mooney[m] | we dont ship them the devstack one are not defualt in nova | 07:48 |
| gibi | I have no hard opinion about default flavors. I dont think this is a technical issue, it is an issue of how to create standards | 07:49 |
| bauzas | anyway, I'm just saying we're on a territory I don't like | 07:49 |
| bauzas | gibi: agreed, this isn't a problem we should solve technically | 07:49 |
| bauzas | gibi: we should let the community agree on a standard set, if they agree eventually | 07:50 |
| sean-k-mooney[m] | that or provide supprot for creating them in osc | 07:50 |
| sean-k-mooney[m] | or as a plugin | 07:50 |
| bauzas | that's one of the reasons we wanted this unified client | 07:50 |
| sean-k-mooney[m] | i.e. have a repo where we can host some flavor templats for standard flaovr that opertors can contibute too | 07:51 |
| bauzas | nacking the fact that we could bfv auto in the client just because others don't use our client seems an alternative we quickly refused because $whataboutism | 07:51 |
| sean-k-mooney[m] | bauzas we already have the ablity to do bfv in the client today | 07:52 |
| bauzas | then, personnally I feel the problem solved. | 07:52 |
| jkulik | but not auto-bfv | 07:52 |
| bauzas | then patch the client | 07:52 |
| jkulik | yeah, I can do that. again, doesn't help my customers ;) | 07:53 |
| bauzas | and patch any other fancy client | 07:53 |
| jkulik | I'll patch Nova instead thus. downstream | 07:53 |
| sean-k-mooney[m] | we have openstack server create —boot-form-volume <size> today | 07:53 |
| sean-k-mooney[m] | so ^ is our baseline in terms fo client just incase you were not aware bauzas | 07:54 |
| bauzas | sean-k-mooney: thanks, appreciated | 07:55 |
| bauzas | I was looking at the client docs | 07:55 |
| sean-k-mooney[m] | its here https://docs.openstack.org/python-openstackclient/latest/cli/command-objects/server.html#server-create | 07:55 |
| bauzas | sean-k-mooney: we can move on the etherpad and propose the CLI usage | 07:55 |
| bauzas | and see what people think | 07:55 |
| sean-k-mooney[m] | i thikn since walaby when stepehnfin pushed to get parity | 07:55 |
| bauzas | yay | 07:55 |
| bauzas | again, see the metrics | 07:56 |
| bauzas | Queens and beyond | 07:56 |
| sean-k-mooney[m] | honestly i dont think that helps in any way | 07:56 |
| sean-k-mooney[m] | i was assumeing they already had that and it was not sufficent | 07:56 |
| sean-k-mooney[m] | but sure they may be on old clouds | 07:56 |
| sean-k-mooney[m] | so maybe its enough | 07:56 |
| jkulik | I get the approach of keeping the scope small and saying "we support osc, everything else is out of scope". But adding features like this only to osc will keep them away from a lot of customers on our side, so it might as well not be there. No hard feelings. | 07:56 |
| bauzas | trust me, they use very old relases | 07:56 |
| * sean-k-mooney[m] responed to mail about juno usage last night | 07:57 | |
| bauzas | jkulik: we support any client able to access our REST API | 07:57 |
| sean-k-mooney[m] | well technially we supprot the apis not the client | 07:58 |
| bauzas | we even support clients that aren't able to negociate with the API about new microversions | 07:58 |
| sean-k-mooney[m] | we dont supprot any client excpet nova client from a project perspective | 07:58 |
| bauzas | here, we're not brainstorming about a lack in our APIs | 07:58 |
| jkulik | then I don't get why you would propose to add the feature to a single client instead | 07:58 |
| bauzas | and again, I'm afraid bfv-auto would generate kind of a config-driven API | 07:58 |
| bauzas | jkulik: I'd propose other clients to do the same, to be precise | 07:59 |
| sean-k-mooney[m] | well that is why i suggested the falvor | 07:59 |
| jkulik | config-driven API is where the values, the API should get via request comes in from other sources e.g. flavor or config? | 07:59 |
| sean-k-mooney[m] | no it explcitly refer to useing nova.conf values to alter behavior | 07:59 |
| jkulik | ah, thanks | 08:00 |
| bauzas | like, delete_on_termination | 08:00 |
| sean-k-mooney[m] | that is not a config value is it | 08:00 |
| sean-k-mooney[m] | i did not think it was | 08:00 |
| bauzas | ah my bad, bad example | 08:00 |
| sean-k-mooney[m] | the option i normall think of is force_configdrive | 08:00 |
| sean-k-mooney[m] | which is a config value | 08:01 |
| bauzas | honestly, I'm not opposed to add a way to describe automatic bfv, but we seriously need to consider the impacts | 08:01 |
| bauzas | and I personally have concerns with the solution be a flavor extraspec | 08:01 |
| sean-k-mooney[m] | ack | 08:02 |
| sean-k-mooney[m] | ya thats fair | 08:02 |
| sean-k-mooney[m] | i think i would want to see the last partity gaps get closed which i think is just rebuild first | 08:02 |
| bauzas | agreed | 08:02 |
| sean-k-mooney[m] | up until this cycle the delta for bfv and non bfv was too much to not have teh user be aware of which it was going to use ectra. | 08:03 |
| jkulik | hm ... if not flavor extraspecs where else? we also have baremetal flavors, which can't even use volume storage in our cloud | 08:03 |
| bauzas | sean-k-mooney: that's the problem with operator feedback | 08:03 |
| sean-k-mooney[m] | jkulik actully they could depending on yoru release | 08:04 |
| sean-k-mooney[m] | ironic support boot form cinder via iscsi | 08:04 |
| bauzas | sean-k-mooney: ideally I think we should propose a talk or some live thing about what nova can do with bfv noxw | 08:04 |
| jkulik | sean-k-mooney: not in our cloud. all volume-types are in some vmware datastores ... :( | 08:04 |
| bauzas | to let people know about what BFV is today | 08:04 |
| sean-k-mooney[m] | jkulik its pretty new too only like 2-3 relases i think | 08:05 |
| sean-k-mooney[m] | bauzas ya a general bfv talk woudl be good | 08:05 |
| * bauzas wonders whether we may do this between us and ask the Foundation how to promote it | 08:06 | |
| sean-k-mooney[m] | basically a bfv “state of the union” talk of hay with z this is how it can now be used | 08:06 |
| bauzas | Vancouver is too far away | 08:06 |
| sean-k-mooney[m] | i would wait for vancouver honestly | 08:06 |
| sean-k-mooney[m] | well | 08:06 |
| sean-k-mooney[m] | at least for zed to release | 08:06 |
| bauzas | after Zed, this is fair | 08:07 |
| sean-k-mooney[m] | but maybe as a follow up to the project highlights | 08:07 |
| bauzas | I should run a project update thing anyway | 08:07 |
| sean-k-mooney[m] | ok brb | 08:07 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Optimize numa_fit_instance_to_host https://review.opendev.org/c/openstack/nova/+/845896 | 08:17 |
| gibi | sean-k-mooney: ^^ let me know what you think. I might missed some side effect in the algo that makes this optimization actually breaking change | 08:17 |
| sean-k-mooney | gibi: have not read it yet but when i see cache are you assume all instance numa ndoes will have the same requirements | 08:20 |
| sean-k-mooney | oh you are caching the pairs | 08:20 |
| sean-k-mooney | ok that should work | 08:20 |
| sean-k-mooney | ill read the rest | 08:20 |
| gibi | sean-k-mooney: I cache the result of the _numa_fit_instance_cell for host_cell instance_cell pairs | 08:20 |
| sean-k-mooney | yep pairs are fine to cache | 08:21 |
| sean-k-mooney | that break will jsut break the inner loop right | 08:22 |
| * sean-k-mooney never rememebr how break works with nested loops | 08:22 | |
| sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/845896/1/nova/virt/hardware.py#2376 | 08:22 |
| gibi | yepp break jumps out from the inner loop | 08:23 |
| sean-k-mooney | then i dont see why this would not work | 08:23 |
| gibi | if you look at the except: branch it uses there too with the same effect | 08:23 |
| sean-k-mooney | and its just storing pairs of ints so the memory overhead will be trivial | 08:23 |
| gibi | yepp it is len(host_cells) * len(instance_cells) * 2 integeres | 08:24 |
| gibi | that is a lot lest than then n!/(n-k)! number of fit calls the algo made | 08:24 |
| sean-k-mooney | so like with comptueing factorial this shoudl reduce to the cell checks to linear | 08:25 |
| sean-k-mooney | form O(n^2) | 08:25 |
| gibi | it is actuall O(n!/(n-k)!) | 08:25 |
| gibi | I mean it was | 08:25 |
| sean-k-mooney | ya just realised that it much large then n^2 | 08:25 |
| sean-k-mooney | its n choose m | 08:26 |
| gibi | yeah we generate k long permutations from N items | 08:26 |
| sean-k-mooney | so even with that optimiasation do we still want to change the default on mater | 08:26 |
| gibi | sean-k-mooney: yepp, probably | 08:27 |
| sean-k-mooney | with the sperad default it found a candiate on the first iteration | 08:27 |
| sean-k-mooney | did you try thet repoducer with your chage | 08:27 |
| sean-k-mooney | if not i was going to try that now | 08:27 |
| gibi | the cache change helps with the forst case, the default change only helps in some case | 08:27 |
| gibi | the reproduce took 6 mins for me without the cache, with the cache it took 3 seconds | 08:28 |
| gibi | s/forts/worts/ | 08:28 |
| sean-k-mooney | nice | 08:28 |
| sean-k-mooney | so still double spreaing but much much more reasonable | 08:29 |
| sean-k-mooney | gibi: so other then a releasenote im not sure what elese to add to that patch. | 08:31 |
| sean-k-mooney | im not sure we want to unit test using a cache | 08:31 |
| sean-k-mooney | and fucntional test should not be able to tell the differnce | 08:31 |
| gibi | yepp I hope functional will tell me that everything works as is | 08:31 |
| gibi | I can add a unit test that shows that the cache works by mocking and counting the _numa_fit_instance_cell inner calls | 08:32 |
| sean-k-mooney | they should i don tthink you have change the outcome in anyway | 08:32 |
| sean-k-mooney | gibi i woudl only do that if you pull out that code into its own fucntion | 08:33 |
| sean-k-mooney | that code bing the inner loop | 08:33 |
| sean-k-mooney | i think this is too much of an implemation detail to asser for the top level numa_fit_instance_to_host fucntion as it is | 08:33 |
| opendevreview | ribaudr proposed openstack/nova master: Allow unshelve to a specific host (Compute API part) https://review.opendev.org/c/openstack/nova/+/831507 | 08:34 |
| opendevreview | ribaudr proposed openstack/nova master: Allow unshelve to a specific host (REST API part) https://review.opendev.org/c/openstack/nova/+/845897 | 08:34 |
| sean-k-mooney | espically since its a function scoped cache | 08:34 |
| gibi | yeah I dropped the unit test idea. after played around it a bit it seems very artifical | 09:33 |
| gibi | I have to fix up some unit test case and I will add a reno and follow your inline suggestion | 09:33 |
| gibi | the functional tests passed so I think it is correct | 09:33 |
| sean-k-mooney | i would be interested to triger a white box run against it but we dont really have much numa testing with whitebox upstream | 09:39 |
| sean-k-mooney | i think its a pretty safe optimisation | 09:39 |
| sean-k-mooney | if you like i can try and find some time to try it on real hardware or simulated multi numa hardware at least | 09:39 |
| sean-k-mooney | but if our func test pass i do trust them for most numa stff at this point | 09:40 |
| *** tosky_ is now known as tosky | 09:42 | |
| gibi | I also feel pretty safe with this now | 09:43 |
| gibi | I don't think we should spen much time manual testing it. I will ask the bug author to test it for us | 09:44 |
| gibi | they have big hardware appareantly | 09:45 |
| sean-k-mooney | ish | 09:53 |
| sean-k-mooney | its not really that big given its a singel socket system and amd launched that chip about 3 years ago | 09:54 |
| sean-k-mooney | there are more of them out in the wild then you might otherwise expect vexhost has a similar sku them but they do not expose all the numa nodes | 09:54 |
| sean-k-mooney | its configurable in the bios | 09:55 |
| sean-k-mooney | but sure | 09:55 |
| sean-k-mooney | lets get the op to test and provide feedback | 09:55 |
| gibi | ack | 10:07 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Optimize numa_fit_instance_to_host https://review.opendev.org/c/openstack/nova/+/845896 | 10:40 |
| gibi | now with reno ^^ | 10:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova stable/wallaby: Fix eventlet.tpool import https://review.opendev.org/c/openstack/nova/+/845838 | 10:43 |
| gibi | bauzas: fyi, reported a new gate failure https://bugs.launchpad.net/nova/+bug/1978817 I will push a fix soo | 11:02 |
| gibi | it is not a blocker | 11:02 |
| gibi | it only fails on slooow nodes | 11:02 |
| sean-k-mooney | gibi other then formating it does not look like much changed in https://review.opendev.org/c/openstack/nova/+/845896/2/nova/tests/unit/virt/test_hardware.py | 11:13 |
| gibi | sean-k-mooney: I needed to add id field for instance cells | 11:13 |
| sean-k-mooney | oh just saw that | 11:13 |
| gibi | (and yes I reformatted it :D) | 11:13 |
| sean-k-mooney | ok ide make sense i guess | 11:13 |
| sean-k-mooney | we need to modle the guest numa node that the object represents | 11:14 |
| sean-k-mooney | and in real code it woudl always be set | 11:14 |
| sean-k-mooney | so you are just fixing the test data | 11:14 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Make test_wait_for_instance_event_* test time independent https://review.opendev.org/c/openstack/nova/+/845922 | 11:16 |
| gibi | sean-k-mooney: yepp it is just adding a more realistic test data | 11:16 |
| sean-k-mooney | ok im +2 on that patch assuming zuul is happy this time | 11:17 |
| sean-k-mooney | stephenfin: you proably know that code the best out of the remaining cores would you mind looking at that if you have time | 11:17 |
| sean-k-mooney | if not artom your +1 and bauzas review would be nice | 11:18 |
| gibi | yepp. I also asked for feedback from the bug reporter | 11:18 |
| artom | sean-k-mooney, which one? | 11:47 |
| bauzas | gibi: ack for the gate failure | 11:52 |
| gibi | bauzas: since then I pushed the fix https://review.opendev.org/c/openstack/nova/+/845922 | 11:52 |
| bauzas | just saw it | 11:53 |
| gibi | artom: I think sean-k-mooney refered to the numa scheduling perf optimization fix https://review.opendev.org/c/openstack/nova/+/845896 | 11:53 |
| bauzas | gibi: 1.23 secs, heh | 11:53 |
| bauzas | any reason why this value ? | 11:53 |
| gibi | 123 :) | 11:54 |
| gibi | just a sequence of ints | 11:54 |
| *** tosky_ is now known as tosky | 11:55 | |
| bauzas | could be 0.123 :p | 12:00 |
| gibi | lost opportunity | 12:02 |
| gibi | bauzas: finally I understood your comment. see the response in https://review.opendev.org/c/openstack/nova/+/845922/1#message-84ed99281b79342e03258fde3778208610868563 | 12:37 |
| bauzas | gibi: sorry yeah, I understood this was a returned value of a mock | 12:38 |
| gibi | so there is no delay in the test | 12:38 |
| bauzas | yeah I was wrong when commenting | 12:39 |
| gibi | no worries | 12:40 |
| opendevreview | Merged openstack/nova stable/yoga: Simulate bug 1969496 https://review.opendev.org/c/openstack/nova/+/840832 | 13:06 |
| *** dasm|off is now known as dasm | 13:08 | |
| *** tosky__ is now known as tosky | 13:17 | |
| *** tosky is now known as Guest2169 | 13:29 | |
| *** tosky__ is now known as tosky | 13:29 | |
| *** tosky__ is now known as tosky | 13:58 | |
| opendevreview | norman shen proposed openstack/nova master: Clear connection info if vol disconnected https://review.opendev.org/c/openstack/nova/+/845995 | 14:07 |
| sean-k-mooney | artom: yes i was refering to https://review.opendev.org/c/openstack/nova/+/845896 | 14:07 |
| opendevreview | Merged openstack/nova stable/train: Only allow one scheduler service in tests https://review.opendev.org/c/openstack/nova/+/751362 | 14:10 |
| opendevreview | Merged openstack/nova stable/train: func tests: move _run_periodics() into base class https://review.opendev.org/c/openstack/nova/+/751363 | 14:10 |
| opendevreview | Merged openstack/nova stable/train: Helper to start computes with different HostInfos https://review.opendev.org/c/openstack/nova/+/751364 | 14:10 |
| opendevreview | Merged openstack/nova stable/train: tests: Add reproducer for bug #1879878 https://review.opendev.org/c/openstack/nova/+/751365 | 14:10 |
| opendevreview | Merged openstack/nova stable/train: Add generic reproducer for bug #1879878 https://review.opendev.org/c/openstack/nova/+/751366 | 14:10 |
| gibi | sean-k-mooney: one hickup in the pci-tracking work: update_provider_tree is the place to report resources BUT [pci]device_spec and the whole pci tracking lives outside of the virt driver. So either we need to report PCI devs to placement ouside of update_provider_tree along with the pci tracker OR do the reporting in update_provider tree and duplicate [pci]device_spec handling in the libvirt driver | 14:19 |
| gibi | I can start building up things in the libvirt driver and see if the disconnect with the pci manager causes any issues | 14:20 |
| sean-k-mooney | not realy | 14:20 |
| sean-k-mooney | we already hae access tot he pci tracker form in that fuction | 14:20 |
| sean-k-mooney | we just neede to read the info form it | 14:20 |
| gibi | we see the pci tracker in update_provider_tree? | 14:21 |
| sean-k-mooney | its aviable via the resouce tracker | 14:21 |
| gibi | do we have access to the resource tracker from the libvirt driver? | 14:22 |
| gibi | I dont see it | 14:22 |
| sean-k-mooney | we should | 14:22 |
| gibi | so the tracker calls the virt driver to update the resource inventories but the tracker is not passed down to the driver | 14:23 |
| gibi | as far as I see | 14:24 |
| sean-k-mooney | hum | 14:25 |
| sean-k-mooney | can we just pass it in | 14:25 |
| sean-k-mooney | i dont like the idea of duplicating the logic in the driver | 14:25 |
| sean-k-mooney | these are the two main places we care about correct https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L9244= | 14:27 |
| gibi | https://github.com/openstack/nova/blob/93a65f06df67ce39d65827692150c78013c7f6d5/nova/virt/libvirt/driver.py#L8530 | 14:28 |
| gibi | we need to report the inventories in update_provider_tree | 14:28 |
| sean-k-mooney | sorry wrong link https://github.com/openstack/nova/blob/93a65f06df67ce39d65827692150c78013c7f6d5/nova/compute/resource_tracker.py#L1221-L1232= | 14:29 |
| gibi | sean-k-mooney: so you suggest to pass down the pci_tracker to the update_provider_tree? | 14:32 |
| sean-k-mooney | yep | 14:35 |
| sean-k-mooney | can catch the not implemted error and invoke without it if the driver does nto support it for one release | 14:35 |
| sean-k-mooney | with a fixme | 14:35 |
| sean-k-mooney | so that we dont rbeak out of tree drivers | 14:35 |
| sean-k-mooney | for pci in placment we only are adding supprot for libvirt for now | 14:35 |
| opendevreview | Alexey Stupnikov proposed openstack/nova stable/victoria: Test aborting queued live migration https://review.opendev.org/c/openstack/nova/+/845748 | 14:36 |
| opendevreview | Alexey Stupnikov proposed openstack/nova stable/victoria: Add functional tests to reproduce bug #1960412 https://review.opendev.org/c/openstack/nova/+/845753 | 14:36 |
| opendevreview | Alexey Stupnikov proposed openstack/nova stable/victoria: Clean up when queued live migration aborted https://review.opendev.org/c/openstack/nova/+/845754 | 14:36 |
| sean-k-mooney | gibi: do you think that passing it down or a subset of data form it woudl be problematic | 14:38 |
| gibi | it is different than the other resources like CPU of VGPU | 14:39 |
| sean-k-mooney | gibi: the stat pools woudl be all we need | 14:39 |
| sean-k-mooney | well the other way to do it si to import the code form the pci module | 14:39 |
| sean-k-mooney | and filter the list of pci device with it | 14:40 |
| sean-k-mooney | and thel look at all the instnace and there pci usage | 14:40 |
| gibi | we are moving logic around a virt driver boundary so I'm affraid | 14:40 |
| sean-k-mooney | actully for placment | 14:40 |
| sean-k-mooney | we only need to know the set of pci devices and the whitelist | 14:41 |
| sean-k-mooney | but i would be unfortable with using the current set form libvirt as if its passed into a guest i dont know if that will still be in the data form libvirt | 14:41 |
| sean-k-mooney | its also more work to recompute it so i would be much more comfortabel geting the data form the pci tracker | 14:42 |
| frickler | nova is running nova-live-migration-ceph and tempest-integrated-compute-centos-8-stream as non-voting jobs in gate, can someone have a look and clean that up? https://review.opendev.org/840833 | 14:42 |
| frickler | (just noticed because both are failing and I was worried about "my" patch queued behind it) | 14:42 |
| gibi | sean-k-mooney: I will look into passing the pci tracker down to the virt driver but I feel bad about it architecturally | 14:42 |
| sean-k-mooney | well you have anohter option | 14:43 |
| sean-k-mooney | wich is to implemnt this part of the update in the compute manager | 14:43 |
| gibi | that would duplicate the reshape logic :/ | 14:44 |
| gibi | ideally I would like to have all the similar thing is the same place | 14:44 |
| gibi | so if GPU is handled in the virt driver level then PCI should be too | 14:44 |
| gibi | but I'm not sure why we have a pci tracker in the manager level instead of in the virt level | 14:45 |
| gibi | I mean I know a lot of reasons but I probably don't know all of them | 14:45 |
| gibi | so moving the pci tracker down to the virt level is scary | 14:45 |
| gibi | just passing it down will create coupling that is scary too | 14:45 |
| gibi | <can I rewrite nova, please?> | 14:46 |
| sean-k-mooney | well vGPU shoudl not be where it si now | 14:47 |
| sean-k-mooney | and the pci module is ment to be shared across virt driver which is why its where it is | 14:47 |
| gibi | so should there be a mdev_tracker in the resource tracker? | 14:49 |
| gibi | and also a cpu/memory/disk tracker? | 14:50 |
| gibi | then we would not need the update_provider_tree to run in the virt level | 14:50 |
| gibi | it could run on the compute manager level | 14:50 |
| gibi | but for some reasons we introduced update_provider tree down in the virt level | 14:50 |
| sean-k-mooney | i wanted to track mdevs in the pci tracker or in the resouces table | 14:50 |
| sean-k-mooney | that we use for pmem | 14:50 |
| sean-k-mooney | we have multipel resouce tracker in nova already | 14:50 |
| sean-k-mooney | cpus and memory are traced in the hsot numa toplogy blob vai the hardware.py module | 14:51 |
| sean-k-mooney | pmem uses the resouces table | 14:51 |
| sean-k-mooney | pci has its own tracker | 14:51 |
| sean-k-mooney | and medev use the libvirt xml domain files | 14:51 |
| sean-k-mooney | which is proably the worst of them | 14:51 |
| gibi | so when numa (if ever) will be in placement then the cpu inventory reporting in update_provider_tree need to be moved also to the compute manager level? | 14:52 |
| gibi | as most of the numa tracking is in the resource tracker not in the virt level | 14:52 |
| sean-k-mooney | prbably | 14:52 |
| gibi | so this shows that the concept of update_provider_tree is wrong | 14:53 |
| sean-k-mooney | the resouce tracker is were most of the tackign happens | 14:53 |
| gibi | it cannot really update anything | 14:53 |
| sean-k-mooney | well we can compute it | 14:53 |
| sean-k-mooney | the virt driver has the list of cores | 14:53 |
| sean-k-mooney | and the list of instnaces | 14:53 |
| sean-k-mooney | actully its simpelr then that | 14:53 |
| sean-k-mooney | for cpus we jsut need cpu_share_set and cpu_dedicate_set | 14:54 |
| sean-k-mooney | to define the capstiy | 14:54 |
| sean-k-mooney | and all the traits are provided by the virt dirver | 14:54 |
| sean-k-mooney | the same is technially true for pci devices | 14:54 |
| gibi | yepp | 14:54 |
| sean-k-mooney | so you can just import the pci module | 14:54 |
| sean-k-mooney | and pass it the set of hypervior devices | 14:55 |
| sean-k-mooney | and ask it for the set of pools | 14:55 |
| sean-k-mooney | then use the pools to do the update | 14:55 |
| sean-k-mooney | but that is duplciationg the data we already have | 14:55 |
| sean-k-mooney | so it just comes down to efficnecy | 14:55 |
| gibi | in my mind it comes down to coupling today the virt driver does not couple to the pci tracker at all but tomorrow it will | 14:56 |
| gibi | it increase complexity | 14:56 |
| sean-k-mooney | right so really the reosuce tracker and pci tracker are ment to be the source of truth for the resouce that are avlaibel | 14:56 |
| sean-k-mooney | and the virt driver just provide the raw resoucs | 14:56 |
| sean-k-mooney | btu that is not how it works today | 14:57 |
| sean-k-mooney | we have the virt driver direclty updateign the tree | 14:57 |
| gibi | yepp | 14:57 |
| sean-k-mooney | so that is backwards architularly but we did it because how that tree will like is virt dirver depentent because ironic | 14:57 |
| sean-k-mooney | well ironic vmware ectra they look differnt then libvirt or hyperv | 14:58 |
| sean-k-mooney | e.g. clsuterd vs 1:1 drivers | 14:58 |
| gibi | hm it is differnt per driver yes | 14:58 |
| gibi | so that is baad abstraction | 14:59 |
| sean-k-mooney | currently yes | 14:59 |
| sean-k-mooney | if you jsut want to resue the code form the pci module that is fine | 14:59 |
| gibi | we have the generic nova scheduler code that depends on the tree but the tree if virt driver dependent | 14:59 |
| sean-k-mooney | i dont really think we shoudl port it | 14:59 |
| gibi | I will play around more before I decide | 14:59 |
| sean-k-mooney | ack | 14:59 |
| gibi | thanks for talking to me about it | 15:00 |
| sean-k-mooney | gibi: i will be happy to review working code | 15:00 |
| gibi | sorry If I was sooo negative | 15:00 |
| sean-k-mooney | what ever form that takes | 15:00 |
| gibi | shit meeting :/ | 15:00 |
| gibi | I almost made progress today :D | 15:00 |
| opendevreview | Merged openstack/nova stable/yoga: Allow claiming PCI PF if child VF is unavailable https://review.opendev.org/c/openstack/nova/+/840833 | 15:06 |
| *** xek__ is now known as xek | 15:34 | |
| dansmith | gibi: sean-k-mooney bauzas: glance is planning to make a change to how image locations are updated (for the tight integration with ceph) | 15:53 |
| dansmith | right now we use the user's token and don't have a "talk to glance with this service account" sort of setup like we do for neutron | 15:54 |
| dansmith | we'd need to create images with their token, but then update the location with the service account | 15:54 |
| dansmith | which is similar I think to things we do for/with neutron | 15:54 |
| dansmith | but the spec in glance is very close to merging and I don't think anyone from nova has looked at it or grokked it from a nova perspective (other than me) | 15:55 |
| dansmith | anyone have any concerns over that or want to review it first? | 15:55 |
| bauzas | dansmith: sorry, triage meeting atm | 15:56 |
| * bauzas looks above | 15:56 | |
| dansmith | bauzas: yep, np, reply when you can | 15:56 |
| bauzas | dansmith: pass me the spec | 15:57 |
| dansmith | https://review.opendev.org/c/openstack/glance-specs/+/840882/15/specs/zed/approved/glance/new-location-info-apis.rst | 15:57 |
| bauzas | dansmith: from what you described, the usecase seems legit | 15:58 |
| dansmith | bauzas: I think so to and it matches what we do for neutron at least, but it's a bit of a departure from how we talk to glance today (which is always with the user's token) so I just wanted to make sure we're all in agreement | 15:59 |
| bauzas | dansmith: I'm more concerned by who would be in charge of doing the necessary bits in nova | 16:00 |
| sean-k-mooney | so we kidn of have this precedent for nuton because | 16:00 |
| sean-k-mooney | we have to update the host-id field | 16:00 |
| bauzas | dansmith: I don't want us to lag with old behaviour because of lack of resources | 16:00 |
| sean-k-mooney | for glance dont we already need admin for reading the multi sotre backend locations | 16:00 |
| bauzas | but if the spec owner signs off for doing the nova bits, I'm cool | 16:00 |
| sean-k-mooney | this is one of the things that would be good to do with the service role in the futre | 16:00 |
| whoami-rajat | bauzas, i might end up doing it since I'm the author of the glance spec but if not will find someone who will be willing to do it | 16:01 |
| dansmith | sean-k-mooney: no, we use the internal api endpoint with the user's token | 16:01 |
| sean-k-mooney | ah ok | 16:01 |
| bauzas | whoami-rajat: cool, I understand | 16:01 |
| dansmith | bauzas: yeah, I'm also curious about that, since it won't be super trivial given that it's buried pretty deep | 16:01 |
| sean-k-mooney | so we cant depend on having the admin token today correct | 16:01 |
| whoami-rajat | since I'm already in charge of the cinder changes, nova changes should not be too hard or different | 16:01 |
| bauzas | dansmith: you get my point | 16:01 |
| bauzas | dansmith: I'm afraid of this being not trivial and requiring some hard work, including testing | 16:02 |
| dansmith | sean-k-mooney: right, this is to solve that problem, and let nova present actual credentials to do that thing, instead of letting unprivileged people do it, but only through the internal api endpoint | 16:02 |
| dansmith | sean-k-mooney: basically real auth instead of host-based auth | 16:02 |
| dansmith | bauzas: yeah, I don't think it'll be super hard since we only do this in a couple places, but it's deep, so not trivial | 16:02 |
| bauzas | whoami-rajat: before saying "not too hard", it's quite self-contained | 16:03 |
| bauzas | ah, burned by dansmith | 16:03 |
| bauzas | but there be dragons | 16:03 |
| bauzas | also | 16:03 |
| dansmith | I really don't think there will much burning or dragons, but I haven't gone digging myself to see exactly what would be involved, so I'm hedging :) | 16:04 |
| bauzas | I guess you'll stick with the existing auth'd roundrobins for a couple of releases ? | 16:04 |
| bauzas | rolling upgrades is a bit of a thing | 16:04 |
| dansmith | bauzas: I think what we do is: if the credentials are configured, use the new way, if not, assume the old way is still active | 16:04 |
| bauzas | all good then | 16:04 |
| dansmith | because some people could still be running glance from folsom and be perfectly happy | 16:05 |
| dansmith | so we don't want to introduce a hard requirement.. we could force a move later, but I think it has to be a decent window of overlap | 16:05 |
| bauzas | I'll chime into the review, thanks for the heads up | 16:05 |
| whoami-rajat | Folsom??? but as dansmith said, we will have backward compatibility | 16:05 |
| dansmith | whoami-rajat: sarcasm so extreme to make it obvious :P | 16:05 |
| bauzas | whoami-rajat: as someone who played with Folsom, glance was perfectly good at that time | 16:06 |
| dansmith | haha | 16:06 |
| dansmith | honestly, openstack has been downhill since folsom anyway :P | 16:06 |
| whoami-rajat | :D | 16:06 |
| whoami-rajat | wow, glance has been stable forever! | 16:06 |
| bauzas | things were easier honestly | 16:06 |
| bauzas | nova-net, glance and nova-volume were easy to manage | 16:06 |
| bauzas | but I'm diverting | 16:07 |
| dansmith | okay so to circle back, | 16:07 |
| dansmith | are there any concerns with the general approach that should hold up glance merging the spec and working on this new interface in zed so that we can start converting nova and cinder in Aardvark? | 16:08 |
| bauzas | I saw the first +2 | 16:08 |
| bauzas | I'll review the spec now | 16:08 |
| bauzas | but from a 10.000ft level, no I think we're on a safe bet | 16:09 |
| dansmith | bauzas: ack, could you look at it today before you go? if so I can throw my +2 on there | 16:10 |
| bauzas | dansmith: on it | 16:10 |
| dansmith | bauzas: thanks | 16:10 |
| whoami-rajat | thanks dansmith and bauzas for the discussion | 16:10 |
| dansmith | ++ | 16:10 |
| gibi | I read back I have no objection | 16:27 |
| whoami-rajat | thanks gibi | 16:28 |
| bauzas | I reviewed the patch | 16:32 |
| opendevreview | Artom Lifshitz proposed openstack/nova stable/train: fake: Ensure need_legacy_block_device_info returns False https://review.opendev.org/c/openstack/nova/+/843958 | 16:32 |
| opendevreview | Artom Lifshitz proposed openstack/nova stable/train: tests: work around malformed serial XML https://review.opendev.org/c/openstack/nova/+/844743 | 16:32 |
| opendevreview | Artom Lifshitz proposed openstack/nova stable/train: functional: Use tempdir for CONF.instances_path https://review.opendev.org/c/openstack/nova/+/844750 | 16:32 |
| opendevreview | Artom Lifshitz proposed openstack/nova stable/train: Add a regression test for bug 1939545 https://review.opendev.org/c/openstack/nova/+/843959 | 16:32 |
| opendevreview | Artom Lifshitz proposed openstack/nova stable/train: compute: Ensure updates to bdms during pre_live_migration are saved https://review.opendev.org/c/openstack/nova/+/843960 | 16:32 |
| opendevreview | Artom Lifshitz proposed openstack/nova stable/train: fup: Make connection_info returned by CinderFixture unique per attachment https://review.opendev.org/c/openstack/nova/+/844744 | 16:33 |
| opendevreview | Artom Lifshitz proposed openstack/nova stable/train: func: Add live migration rollback volume attachment tests https://review.opendev.org/c/openstack/nova/+/844745 | 16:33 |
| opendevreview | Artom Lifshitz proposed openstack/nova stable/train: fup: Assert state of connection_info during LM rollback in func tests https://review.opendev.org/c/openstack/nova/+/844746 | 16:33 |
| bauzas | my only concern which isn't blocking is about making sure we provide a smooth upgrade path for our operators | 16:33 |
| bauzas | asking them to change their config before upgrading is a pain | 16:33 |
| bauzas | so, please, don't make it mandatory in one cycle | 16:34 |
| bauzas | dansmith: ^ this reminds me the tick-tock cadence | 16:34 |
| dansmith | bauzas: yeah absolutely. like I said, I think we need a pretty decent window | 16:34 |
| bauzas | if all of this becomes Aa | 16:34 |
| dansmith | definitely longer than even a single tick-tock | 16:35 |
| bauzas | then we can't remove the old client usage in nova in Bb | 16:35 |
| dansmith | for sure | 16:35 |
| bauzas | anyway, this is a nova trhing | 16:36 |
| bauzas | no need to paper it out in the glance spec | 16:36 |
| bauzas | but just sayin', you could keep the old API for a bit | 16:36 |
| bauzas | whoami-rajat: ^ | 16:36 |
| whoami-rajat | bauzas, yep, that's mostly the plan to keep a window, also until the service-to-service interaction becomes available, this all will be a placeholder and won't be functionally active i.e. old path will be used | 16:41 |
| bauzas | whoami-rajat: how do you plan to identify all the nova calls that modify the image location ? | 16:42 |
| whoami-rajat | bauzas, we've the glance code in nova that calls the glanceclient, eg: for add location https://github.com/openstack/nova/blob/93a65f06df67ce39d65827692150c78013c7f6d5/nova/image/glance.py#L558 | 16:51 |
| whoami-rajat | s/code/file | 16:51 |
| whoami-rajat | all the places calling glance should be calling this file but not sure if that's what you mean | 16:52 |
| gibi | sean-k-mooney: one more info for the todays pci-tracking work PciDeviceStats is not good for me as the pool count is 0 if the devices are consumed, but I still need to create inventory for consumed devices too. So I need to build the inventory based on the PciDevice objects alone and match them to DeviceSpec one by one to get the metadata (resource_class, and traits from the config) | 17:20 |
| sean-k-mooney | ok so you can either get the rows directly from the tracker or build them i guess | 17:34 |
| sean-k-mooney | i tought the stats also had the capastity but i guess not | 17:34 |
| sean-k-mooney | ah yes | 17:36 |
| sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/objects/pci_device_pool.py#L55= | 17:37 |
| gibi | I can access count via stats.pools directly but that is only meaningful if there is no pci allocation at all on the compute | 17:38 |
| gibi | and yes I saw that the count is not visible via the object interface | 17:38 |
| sean-k-mooney | right i tought we also had a total in the stats | 17:42 |
| sean-k-mooney | that did not change in addtion to the count of free devices | 17:42 |
| sean-k-mooney | we do not | 17:42 |
| gibi | sean-k-mooney: quick question, does nova alway have a PciDevice object for parent devices? E.g. if only the VF is whitelisted nova still create a PciDevice for the parent is it so? | 17:50 |
| gibi | never mind I will track it down tomoroow | 17:52 |
| gibi | o/ | 17:52 |
| sean-k-mooney | no i dont think we do | 17:58 |
| sean-k-mooney | we could build oen but i dont know if we do by default | 17:58 |
| sean-k-mooney | in the pci tracker today | 17:58 |
| sean-k-mooney | if you whitelist a vf we do not create a row for the vf in the db | 17:59 |
| melwitt | elodilles: I wanted to get your opinion on backporting a refactor that skips a release https://review.opendev.org/c/openstack/nova/+/844750 | 18:24 |
| elodilles | melwitt: hmm. is there any reason to skip ussuri? O.o | 18:28 |
| sean-k-mooney | im not aware of one | 18:29 |
| melwitt | elodilles: not that I know of. we also don't know why the patch helps or why it's needed to make CI pass. have you ever heard of something like that? | 18:30 |
| sean-k-mooney | unless it merged in ussuri when it was master | 18:30 |
| melwitt | I looked at it and nothing rang a bell why it would help ¯\_(ツ)_/¯ | 18:30 |
| sean-k-mooney | it merged in victoria | 18:31 |
| sean-k-mooney | https://github.com/openstack/nova/commit/980711f3d3f2db9dfd792a8c6fd15cb45a85f615 | 18:31 |
| elodilles | i guess it was added in train to avoid some kind of conflict maybe? (and i guess it was not needed in ussuri) | 18:31 |
| sean-k-mooney | i would guess so too | 18:32 |
| sean-k-mooney | perhaps it was merged in in ussurit into one of the other commtis | 18:32 |
| sean-k-mooney | or artom just forgot | 18:32 |
| melwitt | the commit message says CI was failing with "No such file or directory: '/home/zuul/src/opendev.org/openstack/nova/instances/tmpif84g_jd'" i.e. looking for a tempdir and it doesn't exist. but it's weird it wasn't needed in ussuri | 18:33 |
| elodilles | anyway, i'd rather see that either removed from the patch series, or backported to ussuri as well (if that is really needed) | 18:33 |
| melwitt | ++ | 18:34 |
| melwitt | thanks | 18:35 |
| sean-k-mooney | is it because of python 2? | 18:36 |
| sean-k-mooney | probably not actully | 18:36 |
| melwitt | hm.. I didn't think of that. I'm looking at it a bit more to see if I can find why ussuri doesn't seem to need it. I don't like mystery patches 😆 | 18:37 |
| sean-k-mooney | i think it does | 18:38 |
| sean-k-mooney | if im reading this right | 18:38 |
| sean-k-mooney | this is fixint an intermitent failure | 18:38 |
| sean-k-mooney | NOTE(artom) This is needed to prevent the live migration test added | 18:38 |
| sean-k-mooney | in the subsequent patch from failing in the gate | 18:38 |
| sean-k-mooney | so either ther is shared state | 18:38 |
| artom | Yeah, I'm confused too | 18:38 |
| sean-k-mooney | or something else that causes the failure | 18:38 |
| artom | I initially thought it was a ussuri patch originally | 18:39 |
| sean-k-mooney | and if stephen fixed that in victoria | 18:39 |
| sean-k-mooney | it proably affect ussuri | 18:39 |
| artom | And only when melwitt noticed that it was in victoria and not ussuri did I realize something was weird | 18:40 |
| sean-k-mooney | i think stephen orginally just did it as part of generic test cleanup when improving the fucntional test | 18:40 |
| sean-k-mooney | to ensure that the instance path was always unique | 18:40 |
| sean-k-mooney | so i think it should be fine too backport | 18:41 |
| sean-k-mooney | artom: i assume you were seeing failure in the ci as the not suggest without this | 18:42 |
| artom | Yeah | 18:42 |
| artom | You'll see it in the Zuul run on PS1 of https://review.opendev.org/c/openstack/nova/+/844745/1 | 18:43 |
| artom | melwitt, I mean I'm with you, I don't like mystery patches | 18:44 |
| artom | The weird thing is that I couldn't reproduce the CI failure locally on stable/train | 18:44 |
| artom | I suppose it's worth a try to run CI on the func test in the gate without the instance_path fixture | 18:45 |
| melwitt | I'm checking in opensearch to see if there's anything interesting | 18:45 |
| melwitt | I mean, I'm ok with backporting the patch simply on the basis that the change it makes is the right thing to do, I was just not so comfortable with skipping ussuri | 18:46 |
| sean-k-mooney | same | 18:48 |
| artom | I mean if proposing it to ussuri is the quickest way forward, I'mma do it | 18:48 |
| sean-k-mooney | i jus t looked at the func test and i dont see anything obvious that is shareing the instance_path config option | 18:48 |
| sean-k-mooney | oh | 18:48 |
| sean-k-mooney | actully | 18:49 |
| sean-k-mooney | could this be due to how you are starting the compute service | 18:49 |
| artom | Maybe? And do actually, the backport of the _start_computes() helper just landed, so I rebased on top of it | 18:50 |
| artom | And that's straight from ussuri, so maybe with that helper in place, I no longer need the fixture? | 18:50 |
| * artom tries? | 18:50 | |
| sean-k-mooney | no stephs orginal patch that had the temp dir fix was based on https://review.opendev.org/c/openstack/nova/+/746943/6 | 18:50 |
| sean-k-mooney | which added the start_compute helper | 18:51 |
| sean-k-mooney | instead of start_computes | 18:51 |
| sean-k-mooney | artom: you readded the start_computes usage when backporting | 18:52 |
| sean-k-mooney | form victoria to ussuri | 18:52 |
| artom | I initially re-implemented start_compute(), then just rebased on top of the start_computes() with an s | 18:52 |
| artom | sean-k-mooney, oh, in U, yeah, I changed it to use start_computeS | 18:52 |
| sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/843948/3/nova/tests/functional/regressions/test_bug_1939545.py#58 | 18:53 |
| sean-k-mooney | vs https://review.opendev.org/c/openstack/nova/+/843951/3/nova/tests/functional/regressions/test_bug_1939545.py#60 | 18:53 |
| sean-k-mooney | so i wonder if that is somehow related | 18:53 |
| artom | Well, the individual start_compute didn't exist in ussuri | 18:53 |
| sean-k-mooney | sure just wondering if there is a delta there | 18:54 |
| sean-k-mooney | its one of the thigs you had to manually fix | 18:54 |
| sean-k-mooney | https://github.com/openstack/nova/blob/stable/victoria/nova/tests/functional/libvirt/base.py#L40= | 18:56 |
| sean-k-mooney | so on victoria you get the temp path set in base.ServersTestBase, | 18:57 |
| artom | sean-k-mooney, the *libvirt* base | 18:58 |
| sean-k-mooney | yes | 18:59 |
| artom | Oh, which the regression test inherits from | 18:59 |
| sean-k-mooney | yes | 18:59 |
| artom | Oh, and it's not there in ussuri | 18:59 |
| sean-k-mooney | correct | 19:00 |
| artom | But... https://github.com/openstack/nova/commit/980711f3d3f2db9dfd792a8c6fd15cb45a85f615 | 19:00 |
| artom | That's the git blame | 19:00 |
| artom | Wait | 19:00 |
| sean-k-mooney | that on victoria | 19:00 |
| sean-k-mooney | its what you packported | 19:00 |
| artom | No, nothing's new here - the instances_path fixture is in V, not in U, and I cherry-picked it to T | 19:01 |
| sean-k-mooney | yep | 19:01 |
| artom | The mystery is - why is the test failure that I cherry-picked it for not happening in U | 19:01 |
| sean-k-mooney | so you just missed it on U | 19:01 |
| artom | I know :) | 19:01 |
| sean-k-mooney | am either the test that it was confilciitng with on V is not in u | 19:01 |
| artom | The mystery is - why did we not apparently not need it in U | 19:01 |
| sean-k-mooney | or it will fail if we recheck enough | 19:01 |
| sean-k-mooney | you said you did not need it locally right | 19:02 |
| artom | Nope | 19:02 |
| artom | At least, not with a couple of runs | 19:02 |
| sean-k-mooney | so its proably timing/test order related | 19:02 |
| artom | elodilles, is your -1 on https://review.opendev.org/c/openstack/nova/+/843678 blocking? | 19:03 |
| artom | As in, do I need to respin the entire stack? | 19:03 |
| sean-k-mooney | well we have to at least wait for them to merge on the other branches | 19:03 |
| sean-k-mooney | but i would personally respin U and T | 19:03 |
| sean-k-mooney | just adding that patch | 19:04 |
| artom | sean-k-mooney, T already has it | 19:04 |
| artom | Will add to U | 19:04 |
| sean-k-mooney | right but the commits will change | 19:04 |
| sean-k-mooney | for the cherry picked form lines | 19:04 |
| melwitt | elodilles: dang you have eagle eyes | 19:05 |
| melwitt | I didn't notice it 😑 | 19:05 |
| elodilles | artom melwitt: i was just curious why the nova-tox-validate-backport job failed :D | 19:06 |
| melwitt | oh argh, the n-v caught me | 19:07 |
| elodilles | and since the hash was matching... i figured out something else should be the problem :) | 19:07 |
| sean-k-mooney | did it | 19:07 |
| sean-k-mooney | it looks gren to me | 19:07 |
| sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/844750/2 | 19:07 |
| melwitt | sean-k-mooney: bc it's n-v on the check queue | 19:07 |
| sean-k-mooney | it would still say fail | 19:07 |
| melwitt | that gets me sooo often | 19:07 |
| sean-k-mooney | if it failed | 19:07 |
| sean-k-mooney | https://zuul.opendev.org/t/openstack/build/393a44fbe4ee444a8fcc29b78b969edb | 19:08 |
| melwitt | it says failed but it's green +1 | 19:08 |
| melwitt | no sorry | 19:08 |
| sean-k-mooney | are we looking at diffenrt patches | 19:08 |
| sean-k-mooney | https://zuul.opendev.org/t/openstack/build/393a44fbe4ee444a8fcc29b78b969edb/log/job-output.txt#1417 | 19:08 |
| melwitt | it is red fail but I didn't notice it bc it is n-v | 19:09 |
| sean-k-mooney | melwitt: do you have a link | 19:09 |
| melwitt | sean-k-mooney: https://review.opendev.org/c/openstack/nova/+/843678/3 | 19:09 |
| elodilles | melwitt: well, we added this n-v as a feature, to spare some rechecks when the same patch on a newer branch merges | 19:10 |
| melwitt | elodilles: yeah I know. I just keep not seeing it for some reason | 19:10 |
| elodilles | melwitt: but yes, it could be misleading in some cases | 19:10 |
| sean-k-mooney | ah yes we were looking at differnt patches | 19:10 |
| elodilles | (for example in this case :)) | 19:11 |
| sean-k-mooney | i was looking at the tempurl patch form train | 19:11 |
| elodilles | artom: so anyway, that series needs some respin for multiple reasons :S | 19:14 |
| elodilles | artom: also I saw somewhere some interesting relation chain with a patch 2x in it (with different Patch Set number) :-o | 19:15 |
| sean-k-mooney | you also get interesting behavior if you use depend-on in the same repo | 19:16 |
| melwitt | elodilles: I think that was a patch containing a squash that hasn't been abandoned yet (I didn't verify that yet tho) | 19:17 |
| sean-k-mooney | crap that remiends me i have a backport somewhere that shoudl have 3 patches squashed but it only has 2 | 19:20 |
| elodilles | melwitt: oh, i see. could be the case :S | 19:21 |
| opendevreview | Rico Lin proposed openstack/nova master: libvirt: Ignore LibvirtConfigObject kwargs https://review.opendev.org/c/openstack/nova/+/830644 | 19:23 |
| opendevreview | Rico Lin proposed openstack/nova master: libvirt: Remove unnecessary TODO https://review.opendev.org/c/openstack/nova/+/830645 | 19:23 |
| opendevreview | Rico Lin proposed openstack/nova master: add locked_memory extra spec and image property https://review.opendev.org/c/openstack/nova/+/778347 | 19:23 |
| opendevreview | Rico Lin proposed openstack/nova master: libvirt: Add vIOMMU device to guest https://review.opendev.org/c/openstack/nova/+/830646 | 19:23 |
| opendevreview | Rico Lin proposed openstack/nova master: Add traits for viommu model https://review.opendev.org/c/openstack/nova/+/844507 | 19:23 |
| opendevreview | Rico Lin proposed openstack/nova master: Add traits for viommu model https://review.opendev.org/c/openstack/nova/+/844507 | 19:31 |
| ricolin | sean-k-mooney: hi need your suggestion on https://review.opendev.org/c/openstack/nova/+/844507/1/nova/tests/fixtures/libvirt_data.py | 19:34 |
| ricolin | as I don't have environment actually get iommu in domain XML, what do you think might be the way to go here? | 19:35 |
| sean-k-mooney | you dont have a vm with qemu installed? | 19:35 |
| ricolin | gibi: yes | 19:35 |
| sean-k-mooney | well libvirt | 19:35 |
| ricolin | yes | 19:35 |
| sean-k-mooney | this sample date can be gotten via the virsh command line | 19:36 |
| ricolin | gibi: just update https://review.opendev.org/c/openstack/nova/+/778347 please kindly take a review again thanks :) | 19:36 |
| ricolin | sean-k-mooney: okay, let me check | 19:38 |
| sean-k-mooney | the logic you have implement is not correct | 19:39 |
| sean-k-mooney | ricolin: ^ | 19:39 |
| sean-k-mooney | you seam to be mixing up values form libvirt and the extra specs | 19:39 |
| artom | elodilles, the duplicate is because I messed up change IDs when rebasing and squashing, I abandonned the wrong one | 19:39 |
| elodilles | artom: ack, thx! | 19:43 |
| ricolin | sean-k-mooney: ah, so I assume this is also a wrong logic here to include `auto`? https://review.opendev.org/c/openstack/nova/+/844507/3/nova/virt/libvirt/driver.py#12103 | 19:52 |
| sean-k-mooney | ricolin looking | 20:05 |
| sean-k-mooney | that logic might work at least partly | 20:05 |
| sean-k-mooney | it depend on what is in fields.VIOMMUModel.ALL | 20:06 |
| sean-k-mooney | ricolin: so https://review.opendev.org/c/openstack/nova/+/830646/11/nova/objects/fields.py#611 is wrong | 20:07 |
| sean-k-mooney | well | 20:07 |
| sean-k-mooney | no that is the allow set for the image porperty | 20:08 |
| sean-k-mooney | so https://review.opendev.org/c/openstack/nova/+/844507/3/nova/virt/libvirt/driver.py#12103 | 20:08 |
| sean-k-mooney | shoudl work but we wont have a auto or non trait | 20:08 |
| sean-k-mooney | *none | 20:08 |
| ricolin | ack | 20:09 |
| sean-k-mooney | so ya its just the test data that is wrong looking at it quickly | 20:10 |
| sean-k-mooney | you added auto and non to the test data | 20:11 |
| sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/844507/3/nova/tests/fixtures/libvirt_data.py#1157 | 20:11 |
| sean-k-mooney | ricolin: if you have say ubuntu with multipel qemu backend installed | 20:11 |
| sean-k-mooney | ubuntu installs all the backends by defualt | 20:12 |
| sean-k-mooney | you can basicaly do virsh domaincapabliities --arch aarch64 --machine virt | 20:12 |
| sean-k-mooney | https://termbin.com/840k8 | 20:14 |
| sean-k-mooney | ricolin: actully im sorry im tired and confusing myslef | 20:14 |
| sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/844507/3#message-1ac3afc15e677d670697b8c2da280224ca15b516= | 20:15 |
| sean-k-mooney | if you look at my review comment the iommu info is not in the domain caps output | 20:16 |
| sean-k-mooney | ricolin: so i think i said or intended to say you need to determin this based on teh qemu/libvirt version | 20:16 |
| sean-k-mooney | since libvirt does not have an api to report it | 20:16 |
| sean-k-mooney | at least at first glance | 20:16 |
| sean-k-mooney | ill try and take a look at this again but you will likely need to determin the traits based on the libvirt/qemu version and perhapse check if the emulator is aviable | 20:17 |
| sean-k-mooney | so you cant add an iommu section to the test data unless you update libvirt to provide that | 20:18 |
| ricolin | when you said libvirt/qemu version you mean the min version we supported right? | 20:22 |
| sean-k-mooney | ricolin: no | 20:26 |
| sean-k-mooney | the actual version installed on the compute node | 20:26 |
| ricolin | sean-k-mooney: thanks for the advise, will take a deeper look tomorrow :) | 20:30 |
| *** dasm is now known as dasm|off | 21:09 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!