<div dir="ltr"><div dir="ltr">As for this case, and what Matt mentioned in the patch review:<div><div>> restriction for attaching volumes with a tag because while it's</div><div> > true we don't know if the compute that the instance is unshelved on</div><div> > will support device tags, we also don't know that during initial</div><div> > server create but we still allow bdm tags in that case. In fact, in</div><div> > the server create case, if you try to create a server with bdm tags</div><div> > on a compute host that does not support them, it will fail the</div><div> > build (not even try to reschedule)</div></div><div><br></div><div>There is something I don't quite understand, what will be different for the volumes that are newly attached and</div><div>the existing volumes in case you mentioned? I mean, the existing volumes could also have tags, and when</div><div>we unshelve,  we still have to handle the tags in bdms, no matter it is existing bdms or newly atteched when the</div><div>instance is in ``shelved_offloaded`` status. What is the difference?</div><div><br></div><div>BR,</div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Feb 28, 2019 at 12:08 AM Artom Lifshitz <<a href="mailto:alifshit@redhat.com">alifshit@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, Feb 26, 2019 at 8:23 AM Matt Riedemann <<a href="mailto:mriedemos@gmail.com" target="_blank">mriedemos@gmail.com</a>> wrote:<br>
><br>
> On 2/26/2019 6:40 AM, Zhenyu Zheng wrote:<br>
> > I'm working on a blueprint to support Detach/Attach root volumes. The<br>
> > blueprint has been proposed for quite a while since mitaka[1] in that<br>
> > version of proposal, we only talked about instances in shelved_offloaded<br>
> > status. And in Stein[2] the status of stopped was also added. But now we<br>
> > realized that support detach/attach root volume on a stopped instance<br>
> > could be problemastic since the underlying image could change which<br>
> > might invalidate the current host.[3]<br>
> ><br>
> > So Matt and Sean suggested maybe we could just do it for<br>
> > shelved_offloaded instances, and I have updated the patch according to<br>
> > this comment. And I will update the spec latter, so if anyone have<br>
> > thought on this, please let me know.<br>
><br>
> I mentioned this during the spec review but didn't push on it I guess,<br>
> or must have talked myself out of it. We will also have to handle the<br>
> image potentially changing when attaching a new root volume so that when<br>
> we unshelve, the scheduler filters based on the new image metadata<br>
> rather than the image metadata stored in the RequestSpec from when the<br>
> server was originally created. But for a stopped instance, there is no<br>
> run through the scheduler again so I don't think we can support that<br>
> case. Also, there is no real good way for us (right now) to even compare<br>
> the image ID from the new root volume to what was used to originally<br>
> create the server because for volume-backed servers the<br>
> <a href="http://RequestSpec.image.id" rel="noreferrer" target="_blank">RequestSpec.image.id</a> is not set (I'm not sure why, but that's the way<br>
> it's always been, the <a href="http://image.id" rel="noreferrer" target="_blank">image.id</a> is pop'ed from the metadata [1]). And<br>
> when we detach the root volume, we null out the BDM.volume_id so we<br>
> can't get back to figure out what that previous root volume's image ID<br>
> was to compare, i.e. for a stopped instance we can't enforce that the<br>
> underlying image is the same to support detach/attach root volume. We<br>
> could probably hack stuff up by stashing the old volume_id/image_id in<br>
> system_metadata but I'd rather not play that game.<br>
><br>
> It also occurs to me that the root volume attach code is also not<br>
> verifying that the new root volume is bootable. So we really need to<br>
> re-use this code on root volume attach [2].<br>
><br>
> tl;dr when we attach a new root volume, we need to update the<br>
> RequestSpec.image (ImageMeta) object based on the new root volume's<br>
> underlying volume_image_metadata so that when we unshelve we use that<br>
> image rather than the original image.<br>
><br>
> ><br>
> > Another thing I wanted to discuss is that in the proposal, we will reset<br>
> > some fields in the root_bdm instead of delete the whole record, among<br>
> > those fields, the tag field could be tricky. My idea was to reset it<br>
> > too. But there also could be cases that the users might think that it<br>
> > would not change[4].<br>
><br>
> Yeah I am not sure what to do here. Here is a scenario:<br>
><br>
> User boots from volume with a tag "ubuntu1604vol" to indicate it's the<br>
> root volume with the operating system. Then they shelve offload the<br>
> server and detach the root volume. At this point, the GET<br>
> /servers/{server_id}/os-volume_attachments API is going to show None for<br>
> the volume_id on that BDM but should it show the original tag or also<br>
> show None for that. Kevin currently has the tag field being reset to<br>
> None when the root volume is detached.<br>
><br>
> When the user attaches a new root volume, they can provide a new tag so<br>
> even if we did not reset the tag, the user can overwrite it. As a user,<br>
> would you expect the tag to be reset when the root volume is detached or<br>
> have it persist but be overwritable?<br>
><br>
> If in this scenario the user then attaches a new root volume that is<br>
> CentOS or Ubuntu 18.04 or something like that, but forgets to update the<br>
> tag, then the old tag would be misleading.<br>
<br>
The tag is a Nova concept on the attachment. If you detach a volume<br>
(root or not) then attach a different one (root or not), to me that's<br>
a new attachment, with a new (potentially None) tag. I have no idea<br>
who that fits into the semantics around root volume detach, but that's<br>
my 2 cents.<br>
<br>
><br>
> So it is probably safest to just reset the tag like Kevin's proposed<br>
> code is doing, but we could use some wider feedback here.<br>
><br>
> [1]<br>
> <a href="https://github.com/openstack/nova/blob/33f367ec2f32ce36b00257c11c5084400416774c/nova/utils.py#L943" rel="noreferrer" target="_blank">https://github.com/openstack/nova/blob/33f367ec2f32ce36b00257c11c5084400416774c/nova/utils.py#L943</a><br>
> [2]<br>
> <a href="https://github.com/openstack/nova/blob/33f367ec2f32ce36b00257c11c5084400416774c/nova/compute/api.py#L1091-L1101" rel="noreferrer" target="_blank">https://github.com/openstack/nova/blob/33f367ec2f32ce36b00257c11c5084400416774c/nova/compute/api.py#L1091-L1101</a><br>
><br>
> --<br>
><br>
> Thanks,<br>
><br>
> Matt<br>
><br>
<br>
<br>
-- <br>
--<br>
Artom Lifshitz<br>
Software Engineer, OpenStack Compute DFG<br>
<br>
</blockquote></div>