Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-27
15:39:00 artom mriedem, it's been a while for me with that code, where exactly do we fail? And is that code path not used when we unshelve? Because that's basically a new instance build, no?
15:39:12 mriedem https://github.com/openstack/nova/blob/6efa3861a5a829ba5883ff191e2552b063028bb0/nova/compute/manager.py#L2114
15:39:16 mriedem ^ is only called during server create
15:39:19 mriedem not unshelve
15:39:30 artom mriedem, yeah, I remember saying "can't do tagged attached with shelved because we have no idea if the eventual compute will support it"
15:39:32 mriedem unshelve is a new spawn of the guest
15:39:47 mriedem sure but we don't know if the eventual compute for server create will support it either
15:40:01 mriedem anyway, see my reply to Kevin_Zheng's email in the ML
15:40:11 jangutter stephenfin: re: https://review.openstack.org/#/c/625311/ I spoke in PM to gibi about this, I think the next review in the series shows why it's needed: the tag is propagated to the scheduler.
15:40:45 artom mriedem, dammit, yeah, _unshelve_instance() straight up driver.spawn()s
15:41:10 mriedem for that matter,
15:41:21 mriedem we could be migrating an instance with device tags to another host that doesn't support it
15:41:29 mriedem the breakdown is that we just have no filtering for device tags in the scheduler
15:41:43 artom mriedem, true. We couldn't, back then
15:41:53 artom Now in New Placement Future, we could add it
15:41:53 mriedem we could, we just didn't
15:42:12 mriedem i think at the time we just said "aggregates?"
15:42:17 mriedem and conveniently forgot about it
15:42:22 artom mriedem, no, we just didn't consider it, I think
15:42:27 artom Well, except for the boot case
15:42:28 mriedem i remember asking about a scheduler filter
15:42:53 artom Well, we already have ComputeCapabilitiesFilter...
15:42:55 mriedem because the server create aborts
15:42:57 mriedem which is pretty severe
15:43:01 artom Should get shoved in there, somewhere, no?
15:43:11 mriedem that would have been the option
15:43:35 mriedem anyway the better answer now is likely based on https://review.openstack.org/#/c/538498/
15:44:01 mriedem with a placement request filter
15:44:34 artom mriedem, I see - New Placement Future indeed ;)
15:45:22 mriedem having said all this, anyone running mixed hypervisors in the same region is probably asking for trouble
15:45:36 mriedem and anyone running only a hypervisor driver that doesn't support tags should be disabling it in the api via policy
15:45:42 mriedem like if you're just a vio shop
15:45:55 artom ... I don't think we have an API policy for it, do we?
15:46:42 mriedem i thought we did but i guess not
15:46:43 mriedem super
15:47:01 artom I'll add a patch(es) on top of aspiers's to fix this.
15:47:07 mriedem anything in the api that isn't supported by all backend compute drivers should probably be controlled via policy...
15:47:18 artom Would this be considered a bug? Ie, is it fair game after FF?
15:47:25 aspiers kashyap: did you see https://bugzilla.redhat.com/show_bug.cgi?id=1683471#c7
15:47:25 openstack bugzilla.redhat.com bug 1683471 in libvirt "getDomainCapabilities claims SEV is supported for pc-i440fx-1.4 machine type" [Unspecified,New] - Assigned to libvirt-maint
15:47:46 mriedem artom: i'm not even sure what we're trying to fix at this point honestly - scheduling?
15:47:52 jroll jaypipes: have a quick question on one of your comments on 635006 while I'm writing tests, if you have a minute
15:48:07 mriedem artom: adding a placement request filter based on compute capabilities traits is likely a blueprint
15:48:21 artom mriedem, yeah - don't schedule an instance with tags on a host that can't tag
15:48:25 mriedem i.e. https://blueprints.launchpad.net/nova/+spec/expose-host-capabilities
15:48:32 kashyap aspiers: Yes, I saw.
15:48:51 mriedem idk, definitely something i wouldn't rush in the 2 weeks of FF
15:48:56 kashyap aspiers: I was incidentally talking to the same libvirt dev on machine types (he too was looking for docs)
15:49:04 mriedem at this point the design impacts of root volume attach are also making this very risky
15:49:15 artom mriedem, hah, indeed - and anyways I'm already rushing NUMA LM
15:49:49 kashyap aspiers: I'll write some common documentation on machine types that serves us (and any other tools that use libvirt/QEMU). Basing off of this: https://kashyapc.fedorapeople.org/v2-gracefully-handle-qemu-machine-types.rst.txt
15:50:00 artom mriedem, have you tried alcoholism?
15:50:06 aspiers haha
15:50:36 artom mriedem, I'll be at PTG, we can hash this out - is root volume attach/detach going to be in Stein?
15:51:06 artom (We need a rule in the Nova rule at PTG: the moment some spends more than 30 seconds talking about how to model someting in placement, they get pied in the face)
15:51:23 artom (... We'll also need a massive pie budget)
15:51:30 artom *Nova room
15:58:03 openstackgerrit Jim Rollenhagen proposed openstack/nova master: ironic: partition compute services by conductor group https://review.openstack.org/635006
15:58:24 mriedem artom: the root volume attach/detach stuff is trying to get into stein
15:58:33 mriedem which is how i came across this issue while reviewing the change
15:58:50 mriedem anyway i'll start by reporting a bug
16:05:11 melwitt o/
16:05:33 mriedem artom: https://bugs.launchpad.net/nova/+bug/1817927
16:05:34 openstack Launchpad bug 1817927 in OpenStack Compute (nova) "device tagging support is not checked during move operations" [Undecided,New]
16:05:46 artom mriedem, ack
16:06:10 artom mriedem, I put a drive-by comment in the ML about root detach, btw
16:10:27 artom "volume workflow since in these cases, we want to keep the BDM record but reset some fields." Can that part change? Is a BDM without a volume ID really still a BDM?
16:10:35 artom (from https://review.openstack.org/#/c/614750/)
16:10:59 mriedem not all bdms are volumes
16:11:00 artom Anyways, I know I'm jumping in here super late and talking out of my butt
16:11:18 mriedem even if you don't boot from volume, there is an image bdm for the local disk
16:11:46 mriedem BDM.is_volume is what tells you if it represents a volume
16:11:51 artom But there's still "something" there, right? image ID, volume ID?
16:12:20 mriedem yes there would be in those other non-volume cases
16:12:26 mriedem not for a detached root volume
16:13:08 artom OK, so my question still makes sense - if a BDM has nothing to "back" it, is it still a BDM?
16:13:26 mriedem it's a placeholder
16:13:50 mriedem there are probably other ways to model this, and i seem to remember some of that discussion during the spec review, but none of it is great
16:14:30 artom It's almost as though it'd need to be split in half: the instance side, and the "backing" side
16:14:59 artom Anyways, yeah, way too late to talk about design at this stage
16:15:40 mriedem there has to be *something* that nova can use to indicate that the root volume is detached
16:15:56 artom (And the tags would move to the "backing" side - because they're a nova concept, but they apply to the volume, not the attachment, strictly speaking)
16:16:00 mriedem in case the user tries to attach a new root volume or do something like shelve offload, detach root, unshelve (without re-attaching a root volume)
16:16:45 artom mriedem, yeah, there needs to be a placeholder, as you said
16:17:34 artom OK, brainstorming here, how about this: split image_id, volume_id, snapshot_id and tag (and whatever else is needed) into a new "BlockDeviceBacking" object or whatever
16:17:42 artom And null *that* on a detached root volume
16:18:31 artom But persist that BlockDeviceBacking
16:18:44 artom And if the same root volume is re-attached, re-join those two objects?
16:21:13 openstackgerrit sean mooney proposed openstack/os-vif master: add addtion check and gate jobs for os-vif https://review.openstack.org/639732
16:21:14 mriedem no thanks
16:21:38 mriedem i need to move on from this before i become too depressed to get much done today
16:26:04 openstackgerrit sean mooney proposed openstack/os-vif master: add addtion check and gate jobs for os-vif https://review.openstack.org/639732
16:27:15 aspiers kashyap: I think I'll make a preliminary patch which extends LibvirtConfigCapsGuest to capture the <emulator> value
16:27:31 dcdawg Beginner question: Can I simply commit spell corrections in the comments of the code or do I have to make a bug report for that?
16:28:17 kashyap aspiers: Alright, I'll keep an eye on that series. (I'm just about to head out for my Dutch class; next 3 1/2 hours)
16:28:48 openstackgerrit sean mooney proposed openstack/os-vif master: add addtion check and gate jobs for os-vif https://review.openstack.org/639732
16:28:50 openstackgerrit Jim Rollenhagen proposed openstack/nova master: ironic: check fresh data when sync_power_state doesn't line up https://review.openstack.org/636699
16:31:37 mriedem dcdawg: definitely don't need a bug
16:32:09 mriedem depending on the comment you're fixing, if the comment is otherwise not something that can be understood or make sense and needs to be re-written for clarity, sure that's fine
16:32:30 mriedem it's just a simple typo but you still understand what the comment is saying, then patches to fix those are mostly just noise, unless you fix a bunch of stuff in a single change

Earlier   Later