Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-06-04
14:24:36 mriedem so :(
14:27:01 naichuans efried: I do the modification when we discuss the case: customers modified nova.conf, changed the vgpu type. If the gpu already has a vgpu with the previous vgpu type, we can't allocate new gpu type instance(becase we only support one vgpu type per gpug), so we set `reserve` = `total` to prevent scheduling.
14:28:08 efried naichuans: So help me understand which part of the code is detecting old vs new vgpu type. That's the part I'm missing.
14:28:52 naichuans efried: Now we check inventory change to determine if there is vgpu configure changes
14:29:08 efried but inventory changes when you allocate a vgpu
14:29:18 naichuans No
14:29:22 efried no???
14:30:17 naichuans inventory only contains the vgpu type, total, such kinds of features, no resource count
14:31:27 naichuans resouce amount related information is include in allocation(about used), and the real time host states from hypervisor
14:31:35 melwitt mriedem: it looks like the -W can be lifted from this patch now https://review.openstack.org/568382 and the rest of the series be reviewed for the ocata release https://review.openstack.org/571522
14:35:44 naichuans efried: check inventory define in the patch, we only have `total` field, no `remain`
14:36:07 naichuans fields.ResourceClass.VGPU: {
14:36:08 naichuans 'step_size': 1,
14:36:08 naichuans 'max_unit': 1,
14:36:08 naichuans 'min_unit': 1,
14:36:08 naichuans 'total': grp_value['total'],
14:36:09 naichuans }
14:36:11 naichuans }
14:36:46 efried naichuans: I understand that 'remaining' comes from the hypervisor.
14:37:32 naichuans vGPU type changes would effect total and display heads, I will add traits detecting code in the following patches
14:38:21 naichuans effect -> affect. sorry
14:38:48 efried (wow, that's one English speakers have trouble with :)
14:39:19 efried naichuans: Okay, so the key here is has_inventory_changed - meaning, is the inventory in the provider tree different from the inventory you expect...
14:39:28 efried let me stare at this for a sec...
14:40:19 naichuans efried: almost
14:40:51 efried naichuans: So you're using has_inventory_changed to detect that the vgpu type is different? That's the part that doesn't make sense to me.
14:41:17 efried Surely there are vgpu types that will have the same total as each other.
14:41:26 efried so that check won't do anything.
14:41:49 efried I would expect if you're trying to detect a changed vgpu type, you would be looking for the grp_id to be different.
14:42:46 efried see, at the moment, your condition says "the resource provider for the expected grp_id already exists, but its inventory total isn't what I expected. Let's see if anything is allocated. Okay, it is - let's disable this GPU."
14:42:54 naichuans The check is for the gpug with same uuid
14:43:14 efried does that correspond to a distinct gpu type?
14:43:21 efried or vgpu type?
14:44:07 naichuans almost means it is the same gpug, but there is configure changes on it. we can't support vgpu type checking, jianghua and jay has reject this field
14:45:05 mriedem melwitt: we don't run the nova-lvm job in ocata, so not sure if we should hold and backport/verify that? if even in a WIP patch. note that we also merged the pike backports w/o having nova-lvm running in pike either.
14:45:28 efried naichuans: Same group, but config has changed. Okay, I think I can buy that. But why does that mean you have to disable the gpu?
14:47:03 efried ...and how did the configuration change if there are existing allocations?
14:47:19 melwitt mriedem: oh, it's -W just waiting for the higher branches to merge
14:47:23 naichuans efried: we can't support multi-type vgpu on one gpug. so if the old type instance still on the gpug, we can't allocate new type on it. So we want to disable it. At first I suggest to set `total` to 0, but Jay suggests use `reserved`
14:47:52 naichuans change nova.conf to modify vgpu type white list
14:48:07 naichuans we only need a service resart after that.
14:48:07 mriedem melwitt: what is?
14:48:18 melwitt mriedem: this https://review.openstack.org/568382
14:48:35 mriedem melwitt: that was waiting for https://review.openstack.org/#/c/571433/
14:48:38 mriedem but i'm saying,
14:48:47 mriedem https://review.openstack.org/#/c/571433/ is to fix a nova-lvm regression,
14:48:52 mriedem and we don't run the nova-lvm job in pike or ocata,
14:49:06 mriedem so should we hold until we verify nova-lvm works with that change, or just...punt
14:49:15 efried naichuans: But I thought "changing vgpu type" and "changing the configuration (which leads to a different 'total')" were two different things.
14:50:08 naichuans You mean change the `total` without change the conf?
14:51:26 melwitt mriedem: oh, I thought it was waiting for the higher branches version of it to merge (and they have). I was thinking to just punt ... we verified it worked on master, so tbh I wasn't thinking of doing it per branch even though maybe we should have. we don't have access to the old experimental jobs on older branches do we? we'd have to do something like upload a change that adds a new nova-lvm job for ocata, for example?
14:52:22 mriedem we would have to backport https://github.com/openstack/nova/commit/0db7b77b4615f25217ad646b2e4082994e8dfb73#diff-7415f5ff7beee2cdf9ffe31e12e4c086 to pike and then ocata
14:52:39 melwitt okay, I see
14:52:54 mriedem https://review.openstack.org/568382 was waiting for https://review.openstack.org/#/c/571433/ because https://review.openstack.org/568382 regressed lvm
14:53:38 melwitt oh, right. I didn't realize that was that change
14:53:38 mriedem given my current lack of energy, i'm ok with punting
14:54:43 naichuans efried: as I know, we don't supoort hot plugin/plugout, so that case should not happen.
14:55:52 efried 2) User somehow changes the configuration of their vgpu so it now has a different 'total' than it did before. <== how does this happen?
14:55:52 efried 1) User reconfigures which vgpu type they want to use. They do this by changing CONF.devices.enabled_vgpu_types.
14:55:52 efried naichuans: I'm still trying to wrap my head around all of this, but what I think I've gathered so far is that there are two scenarios we need to be worried about:
14:57:09 efried naichuans: In the case of 1), if there are existing allocations of the old type, we need to block any subsequent allocations of the new type because we only support one type at a time. Right?
14:57:23 mnaser monday morning fun
14:57:23 efried And I don't understand which part of the code is doing ^ this.
14:57:35 mnaser nova boot foo.123 fails because
14:57:37 mnaser .123 isn't a tld
14:57:50 mnaser and it doesn't even fail in the api layer, it gets rescheduled
14:58:28 mnaser RescheduledException: Build of instance e142e418-7c47-409f-8892-542d5e86e442 was re-scheduled: Invalid input for dns_name. Reason: 'foo.123' not a valid PQDN or FQDN. Reason: TLD '123' must not be all numeric.\nNeutron server returns request_ids: ['req-3ddffd11-e41e-4be2-98a8-d2c22b8c515c
14:58:38 mnaser well i guess you could say it's a neutron bug, i dunno what'd you'd classify this at
14:59:02 dansmith I wasn't aware that we assigned any special significance to names with dots
14:59:08 mnaser if nova team feels like this is a nova bug, ill file and start looking into fixes
14:59:17 naichuans efried: we set `reserve` = `total`, to avoid scheduling for new vgpu
14:59:19 mnaser i think neutron does validation and we might not be doing proper sanitization when creating a port
14:59:21 melwitt I feel like I've seen that before. because we use display_name as the hostname
14:59:28 dansmith mnaser: I wonder if we're just passing the display name to neutron and it's inferring some signficance?
14:59:38 mnaser i think that's exactly whats happening
14:59:39 dansmith melwitt: ah, that'd make sense
14:59:44 melwitt dansmith: I think that's why
14:59:46 mnaser it creates a port with dns_name=display_name and poof
14:59:46 dansmith so I don't think that's an us thing
15:00:09 mnaser well if nova is giving a non-valid dns_name, i'd say it's okay for neutron to complain?
15:00:11 dansmith unless we shouldn't be passing the display name
15:00:14 melwitt we do this kind of sanitizing https://github.com/openstack/nova/blob/master/nova/compute/api.py#L1416
15:00:36 efried naichuans: Yes, I'm not arguing with that part. When we need to "disable" a vgpu, we set reserved = total. What I'm trying to figure out is, under what circumstances do we decide we *need* to disable a vgpu? Am I on the right track with 1 and 2 above?
15:00:59 melwitt https://github.com/openstack/nova/blob/master/nova/utils.py#L485
15:01:25 dansmith right, but DNS validation shouldn't be part of the sanitization
15:02:01 melwitt yeah, it isn't
15:02:35 naichuans efried: I see. In truth each vgput type consume different resource, the `total` = `whole resource` / `resource for one vgpu of certain type`. so change the vgpu type, you will change the `total`
15:02:54 efried not necessarily?
15:02:55 dansmith mnaser: surely there's some configuration of neutron that doesn't require valid TLDs right?
15:03:08 efried naichuans: Surely there are some "types" that will have the same `total`?
15:03:21 naichuans efried: it should not.
15:03:40 naichuans especially on a same pgpu group
15:03:42 efried naichuans: But different types can have the same group ID?
15:04:13 jaypipes bauzas: so, I note that you didn't respond to any of my review comments on your multi-gpu-types spec...
15:04:44 naichuans efried: right, one pgug has one id, but it would support more than one vgpu type, each of them has different resource amount
15:04:50 bauzas jaypipes: wait? while I haven't replied to your comments, I think I fixed given your opinions :)
15:05:25 bauzas jaypipes: https://review.openstack.org/#/c/557065/5..6/specs/rocky/approved/vgpu-rocky.rst ;)
15:06:24 jaypipes bauzas: ok. I was just expecting you to come back at me with words :)
15:07:04 bauzas jaypipes: heh, sometimes I'm just revisiting my patches if the comment is good for me :)
15:08:22 efried naichuans: I'm having a hard time understanding that. But okay, so the code we're looking at is meant to handle #2. What are we doing to handle #1?

Earlier   Later