Posted	Nick	Remark
#openstack-nova - 2020-06-19
15:11:47	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: Update and correct typing information https://review.opendev.org/714694
15:11:48	openstackgerrit	Stephen Finucane proposed openstack/nova master: libvirt: Add typing information https://review.opendev.org/714695
15:11:48	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: Add validation for 'cpu_realtime_mask' https://review.opendev.org/468203
15:11:49	openstackgerrit	Stephen Finucane proposed openstack/nova master: tests: Split instance NUMA object tests https://review.opendev.org/714696
15:11:49	openstackgerrit	Stephen Finucane proposed openstack/nova master: objects: Replace 'cpu_pinning_requested' helper https://review.opendev.org/714697
15:11:50	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: Don't consider overhead CPUs for unpinned instances https://review.opendev.org/714698
15:11:50	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: Remove handling of pre-Train compute nodes https://review.opendev.org/714699
15:11:51	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: Rework 'get_realtime_constraint' https://review.opendev.org/714700
15:11:51	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: Tweak the 'cpu_realtime_mask' handling slightly https://review.opendev.org/461456
15:11:52	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: Invert order of NUMA topology generation https://review.opendev.org/714701
15:11:52	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: Remove '_numa_fit_instance_cell_with_pinning' https://review.opendev.org/714703
15:11:53	openstackgerrit	Stephen Finucane proposed openstack/nova master: objects: Introduce 'pcpuset' field for InstanceNUMACell https://review.opendev.org/714658
15:11:53	openstackgerrit	Stephen Finucane proposed openstack/nova master: notifications: add the pcpuset info to instance numa cell payload https://review.opendev.org/728480
15:11:54	openstackgerrit	Stephen Finucane proposed openstack/nova master: objects: Introduce the 'CPUAllocationPolicy.MIXED' enum https://review.opendev.org/716267
15:11:54	openstackgerrit	Stephen Finucane proposed openstack/nova master: libvirt: apply mixed instance CPU policy https://review.opendev.org/728481
15:11:55	openstackgerrit	Stephen Finucane proposed openstack/nova master: Validate CPU pinning configuration for mixed instance https://review.opendev.org/716268
15:11:55	openstackgerrit	Stephen Finucane proposed openstack/nova master: Calculate the CPU usage for mixed instance https://review.opendev.org/716269
15:11:56	openstackgerrit	Stephen Finucane proposed openstack/nova master: online data migration for cpuset of dedicated instance https://review.opendev.org/718493
15:35:39	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: Rework 'get_realtime_constraint' https://review.opendev.org/714700
15:35:40	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: Allow 'hw:cpu_realtime_mask' to be omitted https://review.opendev.org/737026
15:35:40	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: Tweak the 'cpu_realtime_mask' handling slightly https://review.opendev.org/461456
15:35:41	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: Invert order of NUMA topology generation https://review.opendev.org/714701
15:35:41	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: Remove '_numa_fit_instance_cell_with_pinning' https://review.opendev.org/714703
15:35:42	openstackgerrit	Stephen Finucane proposed openstack/nova master: objects: Introduce 'pcpuset' field for InstanceNUMACell https://review.opendev.org/714658
15:35:42	openstackgerrit	Stephen Finucane proposed openstack/nova master: notifications: add the pcpuset info to instance numa cell payload https://review.opendev.org/728480
15:35:43	openstackgerrit	Stephen Finucane proposed openstack/nova master: objects: Introduce the 'CPUAllocationPolicy.MIXED' enum https://review.opendev.org/716267
15:35:43	openstackgerrit	Stephen Finucane proposed openstack/nova master: libvirt: apply mixed instance CPU policy https://review.opendev.org/728481
15:35:44	openstackgerrit	Stephen Finucane proposed openstack/nova master: Validate CPU pinning configuration for mixed instance https://review.opendev.org/716268
15:35:44	openstackgerrit	Stephen Finucane proposed openstack/nova master: Calculate the CPU usage for mixed instance https://review.opendev.org/716269
15:35:45	openstackgerrit	Stephen Finucane proposed openstack/nova master: online data migration for cpuset of dedicated instance https://review.opendev.org/718493
15:35:45	openstackgerrit	Stephen Finucane proposed openstack/nova master: scheduler: mixed instance CPU resource translating https://review.opendev.org/728482
15:35:46	openstackgerrit	Stephen Finucane proposed openstack/nova master: flavor: Enable 'hw:cpu_dedicated_mask' for creating a mixed instance https://review.opendev.org/714706
15:35:46	openstackgerrit	Stephen Finucane proposed openstack/nova master: compute: bump nova-compute service version for mixed instance https://review.opendev.org/716271
15:35:47	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: create 'mixed' instance for realtime and shared CPUs https://review.opendev.org/728485
15:35:47	openstackgerrit	Stephen Finucane proposed openstack/nova master: libvirt/hardware: refactor numa_get_constraints https://review.opendev.org/718294
15:35:48	openstackgerrit	Stephen Finucane proposed openstack/nova master: Setup 'mixed' instance through 'PCPU' and 'VCPU' resource https://review.opendev.org/713355
15:52:05	artom	stephenfin, wait, are you proposing we *remove* the "must start with a ^" validation on the realtime mask?
15:52:17	artom	So we call it a mask, except when the user doesn't feel like, and then it's not a mask?
15:52:37	artom	I actually had no idea it was validated to start with ^ in 2.86
15:52:53	stephenfin	it's still a mask
15:52:55	artom	My opinion would be to keep it, and make the dedicated mask the same
15:53:02	stephenfin	it's just not an exclusion mask
15:53:21	stephenfin	it behaves sanely with this change
15:53:55	artom	I've always understood mask to mean exclusion mask
15:54:16	artom	In fact, "exclusion mask" would be a pleonasm in my book
15:54:25	dansmith	not me
15:54:31	stephenfin	that's not how taskset would work
15:54:31	dansmith	mask could be inclusion or inclusion to me
15:54:43	dansmith	ahah
15:54:47	dansmith	inclusion or exclusion :)
15:54:59	artom	OK, I guess it's just me then, and I had it wrong?
15:55:07	stephenfin	bad artom
15:55:24	artom	*sigh* yeah
15:55:53	dansmith	select()'s FDSET is an inclusion mask yeah?
15:56:40	stephenfin	oh, err, I think so?
15:57:14	dansmith	it is, the question mark was rhetorical :)
15:57:24	stephenfin	oh
15:57:33	artom	dansmith, I'm trying to understand what you mean by reading `man 2 select`
15:57:55	stephenfin	we'll come fish you out of that rabbit hole later
15:57:58	artom	There's FD_SET(), but that's a macro
15:58:26	dansmith	artom: the readfds, for example is a pointer to a word, with bits set nonzero if the FD represented by that bit should be checked for readability
15:58:31	artom	(Also, `man 2 select` is the perfect title for a geek version of The Bachelorette)
16:02:18	artom	dansmith, there's nothing about masks in the readfds para...
16:02:27	artom	It's a set of file descriptors
16:02:49	dansmith	artom: no, it's not, you're just not meant to know how it works under the covers
16:03:27	dansmith	FD_SET is just readfds|=(1 << fd)
17:05:57	openstackgerrit	Stephen Finucane proposed openstack/nova master: Guard against missing image cache directory https://review.opendev.org/736964
17:07:44	stephenfin	dansmith: ^
17:09:25	dansmith	stephenfin: I think you got that logic wrong
17:09:32	stephenfin	crap
17:09:48	dansmith	if the directory does not exist, then True, else if it does, True if st_dev matches
17:11:12	stephenfin	At first glance it reads correctly to me. Bear with me
17:11:35	dansmith	it's okay it's confusing, so make sure I'm right here
17:12:15	dansmith	if the directory does exist, then we need to check to see if the st_dev matches.. if the directory does not exist, then we return True because when we create it, it will be on the same device
17:12:24	stephenfin	right, yeah, makes sense
17:12:34	stephenfin	yay, Fridays
17:12:39	stephenfin	I'll respin again
17:18:53	openstackgerrit	Stephen Finucane proposed openstack/nova master: Guard against missing image cache directory https://review.opendev.org/736964
17:23:14	dansmith	stephenfin: got it
17:24:07	stephenfin	I've done enough on that so I'll defer to melwitt or someone to approve
17:24:28	melwitt	yeah I'll review it
17:25:57	dansmith	ack, I think that's a good idea
17:47:09	melwitt	dansmith: added some comments, am I missing something? because to me it seems this is doing the opposite of what is wanted in the commit message https://review.opendev.org/736964
17:48:29	melwitt	I see that you've said the first time an image is cached, the dir will be created, but update_provider_tree is a periodic task right? what will ensure the cache dir is created by the time we ask for get_disk_usage()?
17:49:18	dansmith	nothing will ensure that it's created
17:49:32	dansmith	the bug is that if it's not created and this runs, it doesn't handle the case where it's not created
17:49:56	dansmith	however, we don't just want to do nothing if it's not created, we want to take the path that assumes that when it does get created, it will necessarily be on the same st_dev, and thus not report any cache
17:50:35	melwitt	yeah, I got that part but from what I was looking, if the cache dir doesn't exist it will fall through and start checking usage of the cache dir and if it doesn't exist won't that be bad?
17:51:09	dansmith	where will it fall through?
17:51:14	melwitt	cause we'll end up here if the dir doesn't exist https://github.com/openstack/nova/blob/f5f7c2540150c7ee7640c834d5caec31b3f5a7ab/nova/virt/libvirt/imagecache.py#L366
17:52:11	dansmith	er wait, maybe I'm inverting that in my head
17:52:47	melwitt	it will return 0 disk usage if self.cache_dir_is_on_same_dev_as_instances_dir is False, right?
17:52:49	dansmith	okay right right, I forgot how this ended up
17:52:58	dansmith	we initially talked about reporting inventory for the cache disk itself,
17:53:03	melwitt	else it will do the math on the cache dir
17:53:28	dansmith	and this is the opposite where we reserve only if the cache is the _same_ disk, which I think is why we endded up checking the reverse condition instead of what we're looking for
17:54:05	dansmith	so yeah, we need to bail if the directory doesn't exist because we're checking the cache_dir which might not exist yet
17:54:25	dansmith	I still think the property shouldnt' explode if the directory doesn't exist,
17:54:44	dansmith	but you're right, it'll still hit the same problem but a little later on
17:55:19	dansmith	the logic is right, it just doesn't prevent us from hitting the same problem in the sum()

Earlier   Later