Earlier  
Posted Nick Remark
#openstack-nova - 2019-02-14
16:36:18 bauzas tbc, I want to have a separate reference to be stored on the libvirt object
16:36:54 gibi bauzas: I would be affraid of copying the self.lock = lockutils.internal_lock(_LOCK_NAME) inside the ProviderTree
16:37:25 efried bauzas: upt is already operating on a copy... but I'm actually not positive that ufpt doesn't dork with it. So yeah, copy.deepcopy is probably the best bet.
16:37:58 gibi bauzas: besides the self.lock the rest of the internals seems copiable
16:38:18 bauzas gibi: so, ideally we should provide a copy method
16:38:19 efried gibi: hm, interesting point about the lock. This impl that bauzas is doing should be read-only. But that still wouldn't stop you from bouncing off the lock if the other instance is using it.
16:38:38 bauzas efried: what I want is purely read-only
16:38:45 efried However, I would be surprised if the lockutils lock doesn't override __copy__ to *not* copy itself.
16:38:49 bauzas actually, you know what ? I'm gonna make it explicit
16:38:55 bauzas by using a propertgy
16:39:11 bauzas we're not intended to manipulate this copy
16:39:12 gibi mriedem: I think I will stick to the remove-the-assert-that-blow-the-world issue, that I can reproduce locally at least
16:39:59 efried bauzas: There are methods to get a read-only copy of the data out of ProviderTree, but I'm pretty sure they only get one provider at a time.
16:40:22 efried Yeah, ProviderTree.data()
16:40:27 mriedem gibi: efried: fwiw i'm also holding off on the bw provider series until https://review.openstack.org/#/c/616239/ makes progress
16:40:40 mriedem and we've got 3 weeks from FF today
16:40:52 efried I guess that's my action
16:41:16 mriedem i'm just waiting for efried to +2 so i can nit pick a few things and make it look like i get it
16:41:17 gibi efried: thanks
16:41:21 bauzas efried: anyway, I'm rushing to propose a new rev
16:41:35 bauzas efried: you will be able to comment on the best approach by reviewing
16:41:41 bauzas not a big deal
16:41:50 gibi efried: to your surprise there is no copy overried in https://github.com/openstack/oslo.concurrency/blob/master/oslo_concurrency/lockutils.py ;)
16:41:57 efried gibi: d'oh!
16:44:11 efried uh, wtf. gibi, bauzas, when I try to deepcopy an object with a lockutils.internal_lock in it, I get an error.
16:44:35 efried so how tf would that be working in the rt? /me looks...
16:51:02 efried gibi, bauzas: http://paste.openstack.org/show/745111/ <== so how tf is https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L885 working??
16:51:37 bauzas heh
16:51:47 bauzas sorcery !
16:54:03 bauzas gibi: looking at the func test
16:54:05 bauzas gibi: you lost me
16:54:47 bauzas gibi: in theory, if you request a VGPU but we don't have inventories because the conf option is unset, you should end up with a beautiful NoValidHost
16:54:56 bauzas so I wonder why it works
16:55:08 bauzas oh wait
16:55:22 bauzas I missed the point you're creating the inventory manually
16:55:44 bauzas oki doki
16:55:58 bauzas so you do this to fake the fact you were running an old version
16:56:22 gibi bauzas: yeah, you are correct
16:56:51 bauzas ok, the good news is that I saw this when I looked at the provider_tree for the allocation RP UUID
16:57:02 bauzas and I was wondering why I was getting a root RP
16:57:10 bauzas gotcha
16:57:15 bauzas perfect, it works then
17:00:40 efried jaypipes: maybe you have some insight?
17:00:59 efried jaypipes: on: http://paste.openstack.org/show/745111/ <== so how tf is https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L885 working??
17:01:54 jaypipes looking...
17:04:09 gibi efried: sorry I'm expired for today
17:04:39 efried gibi: I think you've done enough, don't you? :P
17:05:06 efried "Ohh, I'm gibi, I notice things like lock copies, nya nya nya"
17:05:20 gibi :P
17:05:23 openstackgerrit Merged openstack/nova master: Use math.gcd starting with python 3.5 https://review.openstack.org/636669
17:05:35 openstackgerrit Merged openstack/nova master: doc: specify --os-compute-api-version when setting flavor description https://review.openstack.org/635508
17:05:46 openstackgerrit Merged openstack/nova master: Remove PLACEMENT_DB_ENABLED from nova-next job config https://review.openstack.org/634953
17:10:29 jaypipes efried: I have no idea how that works.
17:11:48 efried jaypipes: I figured we must be monkey-patching lockutils somewhere. Couldn't find it at a glance, but haven't finished looking.
17:12:02 jaypipes no idea. :(
17:13:16 openstackgerrit Stephen Finucane proposed openstack/os-vif master: WIP: Add API docs for various VIF types https://review.openstack.org/637009
17:15:01 openstackgerrit melanie witt proposed openstack/nova master: WIP Remove project_only=True from database queries https://review.openstack.org/637010
17:15:45 cdent efried: if you simply want to fix it, you can implement __deepcopy__ on the ProviderTree class
17:15:55 cdent but it's confusing that it hasn't come up before
17:18:04 openstackgerrit Merged openstack/nova master: Don't set bandwidth limits for vhostuser, hostdev interfaces https://review.openstack.org/635170
17:25:10 efried cdent: Exactly so.
17:27:26 efried Aside: I, for one, am looking forward to hearing all the gweilo trying to pronounce the U release name, whatever it winds up being :P
17:27:38 efried self included, of course
17:29:50 artom efried, heh, we'll just cop out and call it U
17:32:10 cfriesen of the "exception.DeviceNotFound" exception.
17:32:10 cfriesen melwitt: mriedem: question about https://github.com/openstack/nova/blob/master/nova/virt/libvirt/guest.py#L416-L422 I'm seeing this (on pike): http://paste.openstack.org/show/745113/ Which I think means that the original exception is lost (see last line in paste), and so the check for "if 'no target device' in errmsg" will never actually succeed. This results in me hitting the "libvirt.libvirtError" exception instead
17:33:43 melwitt looking
17:37:51 efried gibi: I don't want to delay this another day, but I still don't get it. Are you around to hold my hand and explain it?
17:41:29 melwitt cfriesen: the last line looks like it did the right thing, raised DeviceNotFound after catching 'no target device'. but the previous traceback is different, 'internal error' with a 'not found' message, which isn't handled in any way
17:42:14 efried gibi: nm, I think I see it now. And I am suitably shocked that this is the case.
17:42:25 cfriesen melwitt: it looks like the intent in LibvirtDriver.detach_volume() was to swallow the DeviceNotFound exception, but I seem to be hitting the libvirtError exception path which gets re-raised and handled in DriverVolumeBlockDevice.driver_detach
17:44:01 artom Weird question, but does oslo.config support *writing* config files? Sorta like python's configparser, but hopefully without getting rid of comments when writing (which configparser does :( )
17:44:02 melwitt cfriesen: right. it looks like you're hitting an unexpected combination where you have an libvirt 'internal error' with a 'not found' message, whereas we expect a 'operation failed' libvirt error to go with the 'not found' message
17:45:24 cfriesen got it...I hadn't clued in we were dealing with two separate exceptions
17:46:19 openstackgerrit Sylvain Bauza proposed openstack/nova master: WIP: Use the correct mdev allocated from the pGPU https://review.openstack.org/636591
17:46:20 openstackgerrit Sylvain Bauza proposed openstack/nova master: Add functional test for libvirt vgpu reshape https://review.openstack.org/631559
17:46:20 openstackgerrit Sylvain Bauza proposed openstack/nova master: libvirt: implement reshaper for vgpu https://review.openstack.org/599208
17:46:38 bauzas efried: updated based on our discussions ^
17:46:55 bauzas now the pGPU finding is a dependency for the reshape patch
17:47:21 bauzas because if not, it would mean we would need to trigger another reshape for allocations wrongly set
17:47:45 melwitt cfriesen: I expect the one you're getting is VIR_ERR_INTERNAL_ERROR and we're only handling VIR_ERR_OPERATION_FAILED and VIR_ERR_INVALID_ARG
17:48:41 cfriesen melwitt: thanks, that gives me something to dig into at least.
17:50:33 melwitt cfriesen: np. looks like we might need a 'if errcode in (libvirt.VIR_ERR_OPERATION_FAILED, libvirt.VIR_ERR_INTERNAL_ERROR):' on L409
17:50:52 cfriesen melwitt: it looks like we expect the detach from the persistent config to sometimes fail...what would cause that?
17:51:47 melwitt cfriesen: it's only expected if the device is not there (already detached). that is, libvirt will raise an error if it's already detached
17:52:40 cfriesen melwitt: so why would it be already detached? a previous iteration of the loop?
17:52:43 melwitt that's why we specifically look for 'not found' (in the case of persistent) or 'no target device' (in the case of live)
17:53:54 melwitt cfriesen: we've seen things like, someone requests a detach, the detach from the libvirt domain succeeds, but the detach from the compute host fails, so a fail is returned up to the user
17:54:29 melwitt then later, the user tries to detach the volume again, the device was detached from the libvirt domain last time, so we expect 'not found' at that point, but we still need to detach from the compute host
17:55:15 cfriesen got it, thanks
17:55:20 melwitt np
18:08:19 efried mriedem: I'm +2 now on https://review.openstack.org/#/c/616239/
18:08:48 efried bauzas: Yes, I agree. Looking...
18:09:52 mriedem efried: ack cool
18:10:01 mriedem will try to hit that later today or tomorrow morning
18:10:28 efried My confusion is probably because I missed the bulk of the spec discussion and the bottom of the series.
18:10:55 efried Trying to catch up seemed futile, so I'm trying to jump in in the middle, and suffering the consequences.
18:11:03 mriedem i haven't gone through the comments, but looks like you were expecting us to track vcpu/disk/ram in the request group?
18:11:26 mriedem we still pull those values off the request_spec.flavor during scheduling prior to calling GET /a_c
18:11:40 efried mriedem: Yes, I expected that the request spec's list of request groups would contain the request groups.

Earlier   Later