Posted	Nick	Remark
#openstack-nova - 2017-04-03
00:55:51	openstackgerrit	Matt Riedemann proposed openstack/nova master: Preserve pre-existing port when attach interface fails https://review.openstack.org/452577
00:58:28	mriedem	i'll be back in an hour or so
01:19:15	openstackgerrit	Takashi NATSUME proposed openstack/nova master: api-ref: Fix parameters in os-hosts.inc https://review.openstack.org/416444
02:30:41	mordred	mriedem: I was
02:31:50	mordred	mriedem: basically, the idea that I could put metadata on my glance image and it somehow at boot time might cause other volumes to be needed/mounted is both surprising and also might solve one of clarkb's complaints with booting servers with volumes
02:33:01	openstackgerrit	Takashi NATSUME proposed openstack/nova master: Enable cold migration with target host(1/2) https://review.openstack.org/408955
02:34:57	openstackgerrit	Takashi NATSUME proposed openstack/nova master: Enable cold migration with target host(2/2) https://review.openstack.org/408964
02:35:14	openstackgerrit	Takashi NATSUME proposed openstack/nova master: api-ref: Add parameters in cold migrate action https://review.openstack.org/410042
02:35:37	openstackgerrit	Takashi NATSUME proposed openstack/nova master: Add functional tests for cold migration to same host https://review.openstack.org/414926
02:36:26	mriedem	mordred: all sorts of undocumented goodies lurk beneath
02:36:34	mriedem	and apparently is why we can never change anything
03:06:09	mordred	mriedem: fantastic
03:07:08	mordred	mriedem: I may, when it's not 10pm on a Sunday night, ping you more to try to learn more things
03:07:37	mriedem	wfm
03:07:40	mriedem	i'm learning too :)
03:18:36	openstackgerrit	Matt Riedemann proposed openstack/nova master: Preserve pre-existing port when attach interface fails https://review.openstack.org/452577
03:21:34	openstackgerrit	Matt Riedemann proposed openstack/nova master: Preserve pre-existing port when attach interface fails https://review.openstack.org/452577
04:24:42	openstackgerrit	Maciej Szankin proposed openstack/nova-specs master: Add use-local-scheduler spec https://review.openstack.org/438936
04:36:07	openstackgerrit	Maciej Szankin proposed openstack/nova-specs master: Add use-local-scheduler spec https://review.openstack.org/438936
06:19:04	openstackgerrit	Alex Szarka proposed openstack/nova master: Reduce code complexity in vmops._spawn method https://review.openstack.org/438502
06:19:29	openstackgerrit	Alex Szarka proposed openstack/nova master: Reduce code complexity - servers.py https://review.openstack.org/359861
06:19:34	openstackgerrit	Alex Szarka proposed openstack/nova master: Reduce code complexity - manager.py https://review.openstack.org/359868
06:19:39	openstackgerrit	Alex Szarka proposed openstack/nova master: Reduce code complexity - linux_net.py https://review.openstack.org/359871
06:19:50	openstackgerrit	Alex Szarka proposed openstack/nova master: Reduce code complexity - libvirt/config.py https://review.openstack.org/359879
06:32:06	openstackgerrit	Alex Szarka proposed openstack/nova master: Transform instance.resize.error notifications https://review.openstack.org/417882
07:08:10	openstackgerrit	Alvaro Lopez Garcia proposed openstack/nova master: Ensure that periodic reclaim cleans DB deleted instances https://review.openstack.org/323250
07:19:59	openstackgerrit	Dinesh Bhor proposed openstack/nova master: Use HostAddressOpt for opts that accept IP and hostnames https://review.openstack.org/445782
09:02:29	openstackgerrit	Tadas Ustinavičius proposed openstack/nova master: Implements: blueprint custom-emulator-path https://review.openstack.org/452646
09:35:40	openstackgerrit	John Garbutt proposed openstack/nova-specs master: Add policy-remove-scope-checks spec https://review.openstack.org/433037
09:42:40	openstackgerrit	John Garbutt proposed openstack/nova-specs master: Add additional-default-policy-roles spec https://review.openstack.org/427872
09:53:06	openstackgerrit	Stephen Finucane proposed openstack/os-vif master: doc: Rewrap 'vif_types' document https://review.openstack.org/452665
09:53:07	openstackgerrit	Stephen Finucane proposed openstack/os-vif master: doc: Add glossary https://review.openstack.org/452666
09:57:43	openstackgerrit	Stephen Finucane proposed openstack/nova master: hardware: Flatten functions https://review.openstack.org/367470
09:57:43	openstackgerrit	Stephen Finucane proposed openstack/nova master: Standardize '_get_XXX_constraints' functions https://review.openstack.org/385071
09:57:44	openstackgerrit	Stephen Finucane proposed openstack/nova master: Rename '_numa_get_constraints_XXX' functions https://review.openstack.org/385072
09:57:44	openstackgerrit	Stephen Finucane proposed openstack/nova master: De-duplicate _numa_get_flavor_XXX_map_list https://review.openstack.org/385074
10:26:44	openstackgerrit	Stephen Finucane proposed openstack/nova master: Plumbing for tagged nic attachment https://review.openstack.org/415622
10:26:44	openstackgerrit	Stephen Finucane proposed openstack/nova master: Delete nic metadata when detaching interface https://review.openstack.org/435387
10:26:45	openstackgerrit	Stephen Finucane proposed openstack/nova master: Libvirt support for tagged volume attachment https://review.openstack.org/418100
10:26:46	openstackgerrit	Stephen Finucane proposed openstack/nova master: Delete disk metadata when detaching volume https://review.openstack.org/435082
10:26:48	openstackgerrit	Stephen Finucane proposed openstack/nova master: Plumbing for tagged volume attachment https://review.openstack.org/391941
10:26:49	openstackgerrit	Stephen Finucane proposed openstack/nova master: API support for tagged device attachment https://review.openstack.org/376021
10:26:50	openstackgerrit	Stephen Finucane proposed openstack/nova master: Libvirt support for tagged nic attachment https://review.openstack.org/415520
10:30:43	openstackgerrit	Stephen Finucane proposed openstack/nova master: Optimize creating security_group https://review.openstack.org/315311
10:34:24	f10d4	hi
11:02:21	openstackgerrit	Andrey Volkov proposed openstack/nova master: [placement] Add api-ref for GET RP inventories https://review.openstack.org/449765
11:02:22	openstackgerrit	Andrey Volkov proposed openstack/nova master: [placement] Add api-ref for POST RP inventories https://review.openstack.org/451855
11:02:22	openstackgerrit	Andrey Volkov proposed openstack/nova master: [placement] Add api-ref for PUT RP inventories https://review.openstack.org/452707
11:02:23	openstackgerrit	Andrey Volkov proposed openstack/nova master: [placement] Add api-ref for DELETE RP inventories https://review.openstack.org/452708
11:02:37	openstackgerrit	John Garbutt proposed openstack/nova-specs master: Add additional-default-policy-roles spec https://review.openstack.org/427872
11:10:59	openstackgerrit	Dinesh Bhor proposed openstack/nova master: Use HostAddressOpt for opts that accept IP and hostnames https://review.openstack.org/445782
11:13:07	johnthetubaguy	ildikov: had a deeper look at this check_detach patch, actually I think we loose some super important checks, but I could be missing something big: https://review.openstack.org/#/c/446671
11:27:47	ildikov	johnthetubaguy: I added that check as a prep for multi-attach, which Nova still does not support
11:28:19	johnthetubaguy	ildikov: right, but I am saying it breaks all our existing users, how do we stop that
11:29:07	johnthetubaguy	ildikov: it seems like it would work fine with multi-attach to me, it would be good to describe what checks are bad for multi-attach there, I guess its one of the status checkes
11:29:38	ildikov	johnthetubaguy: it's just that it's unnecessary just like the check_attach was
11:29:55	johnthetubaguy	ildikov: I described in my comments, I believe its critical
11:30:09	ildikov	johnthetubaguy: it's Cinder who should do the status checks
11:30:23	johnthetubaguy	ildikov: sure, but we don't have an API in the new flow to do that
11:30:44	ildikov	ildikov: the status checks you mean?
11:30:49	johnthetubaguy	ildikov: and the old flow doesn't seem to check the instance uuid
11:31:09	ildikov	johnthetubaguy: because it didn't need to
11:31:09	johnthetubaguy	ildikov: ... so your patch says stop check_detach, because we can just call begin_detaching?
11:31:34	johnthetubaguy	ildikov: it really, really does
11:32:17	ildikov	johnthetubaguy: I think we went through the detach cases when we removed check_attach and I think we got a good coverage, but I need to look into the code
11:32:31	johnthetubaguy	ildikov: so lets think about the new flow quickly
11:32:38	johnthetubaguy	ildikov: what call does the checks?
11:33:10	ildikov	johnthetubaguy: you need to find the attachment_id in the bdm to detach
11:33:38	johnthetubaguy	ildikov: that doesn't tell us what the state of that attachment is, we could be out of sync
11:34:02	ildikov	johnthetubaguy: the attachment state is Cinder's responsibility
11:34:07	johnthetubaguy	ildikov: we still need to check that the user has asked us to detach a volume that is currently attached to the instance
11:34:21	johnthetubaguy	ildikov: I agree in principal, but we don't have the APIs for it in the new flow
11:34:48	ildikov	johnthetubaguy: Cinder should do that either as part of attachment_delete or we can look into other options
11:35:10	ildikov	johnthetubaguy: for attach we can create an empty attachment to lock the volume and not for the status checks either
11:35:16	johnthetubaguy	ildikov: so attachment_delete is after the API has returned, and after we have attempted to remove the volume
11:36:24	johnthetubaguy	ildikov: so we need a spec in Nova to describe all the multi_attach loose ends
11:37:19	ildikov	johnthetubaguy: we need that for other reasons too
11:37:27	johnthetubaguy	ildikov: this might be one of those, be we need to be clear what checks are replacing our existing guard rails, and describe how you don't break the current API behaviour for Nova
11:37:53	ildikov	johnthetubaguy: I would like to just clean up the code and solve the problems that we really have at the right place
11:38:11	johnthetubaguy	ildikov: we have the same goal, the problem is your patch breaks our API
11:38:33	ildikov	johnthetubaguy: even for the non-dteach cases?
11:38:41	ildikov	*non-detach
11:38:52	johnthetubaguy	ildikov: its breaks the detach flow only, AFAIK
11:39:26	johnthetubaguy	when you call detach with a volume attach on a different instance, we end up doing some very strange things to the volume the user passed into our API
11:39:27	ildikov	johnthetubaguy: bah, sorry I meant non-multi-attach
11:39:37	johnthetubaguy	yes, it breaks all regular detaches
11:40:05	johnthetubaguy	we now report errors in different places, for some cases, other cases I think just break horribly and cause possible data corrupution
11:40:10	ildikov	I wonder whether that check will be the first place to recognize that
11:40:18	johnthetubaguy	I attempted to describe that in the review comments
11:40:39	johnthetubaguy	ildikov: don't know
11:40:47	ildikov	I saw that, but if I don't mess with the code purely because of multi-attach that check wouldn't be there
11:41:20	ildikov	it's not part of the regular detach flow by design
11:41:58	johnthetubaguy	ildikov: so I think it would be much easier for me to review this if we can write up your thinking around the change, and how all these checks should be done
11:42:32	johnthetubaguy	ildikov: a few notes in the git commit message might be the best way to start it, but honestly, it feels like it needs a spec, because its API sensitive
11:43:08	ildikov	and even for multi-attach case as we have the attachment_id stored in the bdm that approach should help
11:43:24	johnthetubaguy	ildikov: I am not sure how that helps the old code flow
11:43:31	johnthetubaguy	that would still be broken
11:43:43	ildikov	johnthetubaguy: I will look into that later, on a conference this week
11:44:19	ildikov	johnthetubaguy: I will test the old flow case and if I hit that error message I will look into how to fix that
11:44:32	ildikov	johnthetubaguy: but the status checks should go regardless

Earlier   Later