Earlier  
Posted Nick Remark
#openstack-nova - 2019-02-25
17:44:12 mriedem i would expect it to *not* change
17:44:18 mriedem since the boot_index is still 0
17:44:32 mriedem and the disk_bus and device_type can't change
17:49:51 sean-k-mooney mriedem: i think the only time we would expect that it could change would be a rebuild with a different image or perhaps a volume retype operation. so if your question was related to the cross cell resize i think that is a safe assumtion to make
17:50:20 mriedem it's not
17:50:32 mriedem it's for Kevin_Zheng's root bdm attach/detach series
17:50:45 sean-k-mooney oh ok
17:51:21 sean-k-mooney am well if you detach attach anoth volume and then attach the root again i guess it could change
17:51:42 sean-k-mooney i dont know that code that well however
17:52:15 mriedem https://review.openstack.org/#/c/614750/34/nova/compute/manager.py
17:52:57 mriedem if i boot from volume and get vda, then attach a data volume which is vdb, then detach the root volume and attach another root volume, would i expect to have that as vda or vdc?
17:53:10 mriedem i would expect vda because the root volume being higher than the data volume seems wrong
17:54:02 dansmith mriedem: yeah, expect the name to remain stable
17:54:25 sean-k-mooney mriedem: that might depend on the os and the udev rules. but i would expect it to stay the same. i dont know if it actully would
17:56:42 mriedem well, we also don't guarantee the device name the user requests is honored by the hypervisor anyway
18:01:04 artom mriedem, interesting, but I feel like that's specific to what you're doing with them (which is fine!)
18:02:19 sean-k-mooney mriedem: for the detach attach root volume spec
18:02:34 sean-k-mooney mriedem: is it the same volume or can it be any volume that is reattached
18:02:37 artom And as far as I can tell _assertEqualObjects doens't handle nested objects
18:03:15 artom Anyways, as I said, it's an annoyance, but a minor one, though it might be worth it to put in the time to do it properly in a single place so that we stop fixing this each in our little corners
18:04:08 sean-k-mooney mriedem: im wondering if we supprot reading the hw_disk_bus key form image metadata on a volume
18:04:55 sean-k-mooney i think the answer is no but im checking
18:10:54 openstackgerrit Merged openstack/os-vif master: Fix nits in brctl removal (vif_plug_linux_bridge) https://review.openstack.org/639099
18:11:57 sean-k-mooney mriedem: it looks like we can get the image meta form the volume https://github.com/openstack/nova/blob/af78b13c24d4abf393d17ac57e9135204ef12b73/nova/utils.py#L928
18:12:25 sean-k-mooney mriedem: so if we are allowing attaching an arbiatry volume as the new root volume the diskbus could change
18:12:37 sean-k-mooney if it has to be the same volume it wont
18:14:09 sean-k-mooney that is called form https://github.com/openstack/nova/blob/5a09c81af3b438ecbcf27fa653095ff55abb3ed4/nova/compute/api.py#L1057
18:24:09 sean-k-mooney mriedem: ah never mind the propsed change stats the detach will be garded by the instance being shelve offloaded
18:24:17 sean-k-mooney ill update my comment on the patch
18:25:18 sean-k-mooney oh thats the mitaka spec...
18:27:07 sean-k-mooney the stein spech allow detach when the instance is powered off which may not work if the iamge changes
18:32:29 openstackgerrit Merged openstack/python-novaclient master: Handle unicode multi-byte characters https://review.openstack.org/632942
18:35:39 melwitt o/
18:35:57 sean-k-mooney melwitt: o/
18:36:11 openstackgerrit Merged openstack/nova master: Pass resource provider mapping to neutronv2 api https://review.openstack.org/616240
18:36:19 openstackgerrit Merged openstack/nova master: Recalculate request group - RP mapping during re-schedule https://review.openstack.org/619529
18:36:29 openstackgerrit Merged openstack/nova master: Add microversion to expose virtual device tags https://review.openstack.org/631948
18:36:42 openstackgerrit Merged openstack/nova master: api-ref: mark os-cells as deprecated https://review.openstack.org/636708
18:36:52 openstackgerrit Merged openstack/nova master: Replace ansible --sudo with --become in live_migration/hooks scripts https://review.openstack.org/635308
18:48:45 openstackgerrit Andrey Volkov proposed openstack/nova master: Check hosts have no instances for AZ rename https://review.openstack.org/509206
18:49:55 mriedem sean-k-mooney: yeah it can be a different volume
18:50:50 sean-k-mooney mriedem: do you think my concern regarding powered off instance is vlaid
18:51:04 sean-k-mooney mriedem: i updated the comment on the patch
18:51:40 mriedem how would powered off be different from when we unshelve the instance with a new root volume?
18:52:06 sean-k-mooney a powered off instace is associated with a host
18:52:16 sean-k-mooney we read the image metadata form volumes
18:52:28 sean-k-mooney so if you can change the voluems you can change the requirement for the host
18:52:50 sean-k-mooney in unshevle we will hit the schuler
18:52:57 sean-k-mooney but for powered off instnace we dont
18:53:11 sean-k-mooney when we we start it again that is
18:55:25 mriedem yes i see the issue, but i don't think he's reading the new root volume image_meta on unshelve either,
18:55:36 sean-k-mooney a concreate example would be if the instance was pinned and the original volume was create from an image with hw:numa_nodes=1 and the new volume was created form an iamge with hw:numa_node=2 it will invalidate the pinnings
18:55:39 mriedem so as far as i know when we unshelve we're not using the new image meta anyway
18:56:04 sean-k-mooney oh well it could be broken in both cases
18:56:11 sean-k-mooney i didnt actully check the code
18:56:54 sean-k-mooney we do get the metadata from the volume when initally spawning the instace
18:57:03 mriedem yeah i know, and on rebuild
18:57:06 sean-k-mooney i assume we would do it again on unselve but maybe not
18:58:15 mriedem no,
18:58:33 mriedem because on unshelve you either boot from the shelve snapshot (if not volume-backed), otherwise you boot from the root volume,
18:58:36 mriedem which before this couldn't change
18:59:32 sean-k-mooney right but on unselve we would have hit the schuler right. i guess it uses the embeded image metadata so it does not have to go back to cinder
18:59:58 mriedem yes unshelve hits the scheduler
19:00:10 mriedem and it would use whatever is in the request spec from the original server create
19:00:42 mriedem which uses the volume image meta here https://github.com/openstack/nova/blob/af78b13c24d4abf393d17ac57e9135204ef12b73/nova/compute/api.py#L886
19:01:10 sean-k-mooney ya. so we could enable this to work for unselve by updating the request spec or just regtiving the metadata form the volume but i dont know how to "fix" start of a powered off instance
19:01:41 sean-k-mooney i would expect the image to go into an error state on start if its requirements chagne
19:01:51 sean-k-mooney but that is not very friendly
19:02:03 sean-k-mooney or start with the old requirement i guess
19:02:46 mriedem inititally the spec never said anything about supporting detach/attach of the root volume for powered off instances
19:02:50 mriedem only shelved offloaded
19:02:56 mriedem which is how hpe wrote it up long ago
19:02:57 sean-k-mooney yep in mitaka
19:03:25 mriedem i do remember that the RequestSpec.image.id is *not* set for volume-backed servers, so some of our scheduler filters don't work on volume-backed servers
19:03:52 mriedem like IsolatedHostsFilter
19:04:21 mriedem during server create, we do calculate the numa topology requirements from the flavor and image meta, and for volume-backed we get that image meta from the root volume, like you said
19:04:34 mriedem so technically on unshelve we could screw up and not honor the image meta in the new root volume
19:04:42 mriedem b/c we don't update the request spec
19:05:57 sean-k-mooney right. i think that is just an oversight however. e.g. readign the spec i would have assume we woudl cater for that edgecase in the implemnation and they jsut chose not to document it in the spec
19:06:29 mriedem i wouldn't be so sure
19:06:41 sean-k-mooney anyway it looks like newton was still only allowing this for shelved instance so its only allowed for powered off instance in the stein spec
19:07:16 mriedem i'm pretty sure i didn't think about the root image meta changing when s10 pushed for supporting attach/detach of the root volume on stopped instances
19:07:17 sean-k-mooney mriedem: im giving people the benifit of the doubt but perhaps it was not taught of
19:09:40 sean-k-mooney so with 2 weeks to feature freeze i dont really want to reopen the design of this given how long people have been waiting for it.
19:10:37 sean-k-mooney i could propose a revision to the spec to reduce it to only shelved insntace or to document the behavior that will happen for poered off instace
19:11:16 sean-k-mooney *powered off
19:11:47 sean-k-mooney im going to grab dinner but ill be back in an hour or so
19:12:08 sean-k-mooney actully i have to push a small revision to a patch then dinner
19:13:47 mriedem sean-k-mooney: i commented on https://review.openstack.org/#/c/614750/ as well to recap our irc discussion. i'll talk with Kevin_Zheng about it tonight since we have a meeting anyway
19:14:55 mriedem i don't think we can support the stopped case - that would essentially get us back to volume-backed rebuild with a new image that we don't really honor
19:16:11 sean-k-mooney mriedem: ya that was my feeling too but i could see use allowing it and refusing to boot the instacne if the requirement could not be supported
19:16:20 sean-k-mooney that felt overly complicated however
19:16:42 sean-k-mooney mriedem: can we shelve a powered off instance?
19:17:24 mriedem yes
19:17:31 mriedem which i think i said in the spec review when this came up,
19:17:46 mriedem just stop the instance, shelve it, swap the root volume, and unshelve,
19:17:47 sean-k-mooney ok so you jsut need to shelve it and then unshelve after changing volumes and you can more or less achive the same use case
19:17:52 mriedem but people thought that was too complicated
19:18:11 sean-k-mooney perhaps but ill take complicated over broken any day
19:22:29 mriedem sean-k-mooney: i guess i did think about this https://review.openstack.org/#/c/600628/7/specs/stein/approved/detach-boot-volume.rst@127

Earlier   Later