Posted	Nick	Remark
#openstack-nova - 2023-05-30
16:23:12	bauzas	dansmith: well, some folks are continuing to backport a few things to train
16:23:23	bauzas	and we have some stable cores that do reviews on that branch
16:23:26	bauzas	butn,
16:23:29	dansmith	but not critical CVEs
16:23:38	bauzas	as I said in the email, the two critical CVEs don't have a path forward
16:23:56	bauzas	dansmith: this is unfortunately the problem
16:24:03	dansmith	okay, I see there's more traffic than I expected
16:24:09	dansmith	then I guess I don't care
16:24:10	bauzas	that's not exactly that I'm against backporting the VMDK fix
16:24:25	dansmith	but I do think it's confusing for people to see a community-maintained repo that is missing such large fixes
16:24:34	bauzas	but we'll break olso.utils semver if we do so
16:25:15	dansmith	yeah, idk, I lean towards EOLing
16:25:21	bauzas	for the other CVE (the brick one), I'd say I don't know how much it would be difficult to propose a backport
16:25:41	dansmith	the brick fix is not going to be backported AFAIK
16:25:57	dansmith	and I don't think the cinder one (which is the most important) is being backported past xena
16:26:28	dansmith	our part of the fix for that CVE is pretty minor and we could backport it, but without the FC part of the brick fix it's not complete
16:26:28	bauzas	that's why I personnally too lean forward to EOLing stable/train
16:26:47	dansmith	only debian spoke up about/agains EOLing right/
16:26:51	bauzas	correct
16:27:01	bauzas	I can reply and explain the problem again
16:27:31	dansmith	shrug
16:27:49	dansmith	cinder does still have a train branch
16:27:56	dansmith	and the last thing there was the VMDK fix
16:28:15	dansmith	so does brick, but the last thing was last summer
16:28:30	elodilles	i also stated that we could keep open train as long as the gate is working, though indeed it is unfortunate that CVE fixes doesn't get backported :/
16:28:36	dansmith	idk, almost seems like there's not enough people to care to even push us one way or the other :)
16:28:49	bauzas	I would indeed be more inclined if some other project like cinder would make the same step than us quite at the same time
16:29:20	bauzas	dansmith: elodilles: honestly I guess I'll propose a patch for -eol and people can -1 if they care
16:29:33	bauzas	that's probably where we'll capture most of the concerns
16:29:42	dansmith	ack
16:29:46	bauzas	or we could round forever
16:29:48	elodilles	++
16:30:16	bauzas	#action bauzas to propose a gerrit change for tagging stein-eol so people could vote on it
16:30:30	elodilles	you mean train-eol :)
16:30:35	bauzas	damn
16:30:38	bauzas	#undo
16:30:38	opendevmeet	Removing item from minutes: #action bauzas to propose a gerrit change for tagging stein-eol so people could vote on it
16:30:45	elodilles	stein-eol is long gone for nova ;)
16:30:46	bauzas	#action bauzas to propose a gerrit change for tagging train-eol so people could vote on it
16:30:54	bauzas	elodilles: my brain fscked
16:31:13	bauzas	anyway, I guess we're done on this
16:31:22	bauzas	#topic Open discussion
16:31:23	elodilles	++
16:31:36	bauzas	geguileo: you had a point :)
16:31:41	bauzas	(geguileo) Change to os-brick's connect_volume idempotency
16:31:47	geguileo	bauzas: yes, thanks
16:31:49	bauzas	#link https://review.opendev.org/c/openstack/os-brick/+/882841
16:31:52	bauzas	#link https://bugs.launchpad.net/nova/+bug/2020699
16:32:11	geguileo	So with the latest CVE on SCSI volumes we decided in os-brick to make some changes
16:32:45	geguileo	Specifically os-brick would remove any existing devices before starting the connect_volume code
16:32:55	geguileo	and then proceeding with the actual attachment
16:33:09	geguileo	this means that connect_volume will no longer be idempotent
16:33:17	geguileo	(which is never something we promised would be)
16:33:26	geguileo	that seems to break some Nova operations
16:33:57	geguileo	particularly the rescue/unrescue operations
16:34:05	dansmith	is there some way we can check to see if we need to run connect_volume()?
16:34:24	bauzas	geguileo: thanks for spotting this in advance
16:34:30	dansmith	because otherwise it's hard to know if we're restarting after a recovery or just a service restart
16:34:39	dansmith	whether or not we should run that
16:35:08	bauzas	yeah that's one of the problems
16:35:22	geguileo	dansmith: that could be solved if Nova didn't stash and then unstash the instance config
16:35:33	geguileo	and instead rebuilt the instance config
16:35:40	dansmith	meaning the guest xml?
16:35:41	geguileo	I believe sean-k-mooney[m] mentioned that
16:35:45	geguileo	dansmith: yes
16:36:00	bauzas	geguileo: we also recreate the XML on stop/start fwiw
16:36:11	dansmith	okay, but if we're running on a readonly-root host or recovering from a disaster, none of that would be available after a restart for us
16:36:27	dansmith	yeah, the guest XML isn't something we can assume hangs around long-term IMHO
16:36:41	geguileo	dansmith: could nova know if it's attaching the volume for the recovery instance on the same host that was running the instance?
16:36:42	dansmith	most of nova is designed to assume that it's tempoary
16:36:53	bauzas	yeah and in general, we don't consider libvirt a source of truth for persisting some instance metadata
16:37:01	dansmith	bauzas: right
16:38:00	dansmith	geguileo: for the rescue case specifically? if the instance was already stopped and you go straight to rescue, we wouldn't really know when the last time the connect was run.. could have been weeks ago and multiple host reboots since
16:38:01	bauzas	would the spec on dangling volumes helping the problem ?
16:38:17	dansmith	bauzas: I don't think so
16:38:19	bauzas	I mean, our BDM/volume reconciliation
16:38:33	dansmith	that's not the problem, it's the underlying host devices
16:38:48	bauzas	yeah I remember the context of the CVE :)
16:39:21	bauzas	and I guess only brick knows whether there is a residue ?
16:39:29	dansmith	geguileo: so let's say we reboot with one instance stopped that points to /dev/vdf,
16:39:49	dansmith	then we reboot, and someone spawns a new instance, we call connect_volume() for that new instance, it gets /dev/vdf,
16:40:09	dansmith	then the user starts the old was-stopped instance, we can't look at the instance xml config and know that the volume is wrong right?
16:40:20	dansmith	because /dev/vdf exists, but it's no longer relevant for this instance
16:40:55	geguileo	dansmith: well, Nova could check the ID of the volume and see if it matches the information returned by os-brick
16:41:07	dansmith	point being, there could be multiple instances with disks that point to stale host devices at any given point..
16:41:09	geguileo	but I don't know if we want Nova to be on the business of doing those checks
16:41:17	dansmith	yeah, ideally not
16:41:19	geguileo	dansmith: yes, that could happen
16:42:05	bauzas	could nova ask brick such thing ?
16:42:08	dansmith	geguileo: maybe everywhere we currently do connect_volume() we do a disconnect first and ignore an error? that generates a lot of churn, but maybe that's safer?
16:42:29	dansmith	bauzas: that's what I was wondering.. if there was some validate_volume() call or something we could run to know if we should run connect
16:42:41	bauzas	like, before doing connect_volume(), do a check_vol()
16:42:43	bauzas	ahah
16:42:54	geguileo	dansmith: os-brick will be doing that already with optimized code
16:43:20	geguileo	dansmith: that's the issue that I'm bringing, that the new code that cleans things up break the idempotency, so there's no need for nova to do that call you mention
16:43:37	geguileo	os-brick cannot do a proper check_vol like we would want
16:43:48	geguileo	because the volume can have changed in the background
16:44:06	geguileo	for example change the size, point a different volume in the backend (like happened in the CVE)
16:44:26	dansmith	okay, I'm just not sure how we can know what the right thing to do is
16:44:27	geguileo	there are a bunch of messes that could happen, and I'm not sure we would want this to depend on nova calling a method

Earlier   Later