<div dir="ltr"><div>I *think* it is significantly semanticist different to do detach, attach; with swap volume, no events are generated in the guest; that is why it is dangerous to expose to the tenant - if the volume contents is not identical, you get weird corruption as the guess flushes caches.<br><br></div>I think this call only makes sense for migration, and not anything else, and trying to do a version of it that does detach,, attach is both dangerous and unnecessary.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On 31 March 2016 at 05:14, GHANSHYAM MANN <span dir="ltr"><<a href="mailto:ghanshyammann@gmail.com" target="_blank">ghanshyammann@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Thu, Mar 31, 2016 at 10:39 AM, Matt Riedemann<br>
<<a href="mailto:mriedem@linux.vnet.ibm.com">mriedem@linux.vnet.ibm.com</a>> wrote:<br>
><br>
><br>
> On 3/30/2016 8:20 PM, Matt Riedemann wrote:<br>
>><br>
>><br>
>><br>
>> On 3/30/2016 7:56 PM, Matt Riedemann wrote:<br>
>>><br>
>>><br>
>>><br>
>>> On 3/30/2016 7:38 PM, Matt Riedemann wrote:<br>
>>>><br>
>>>><br>
>>>><br>
>>>> On 2/25/2016 5:31 AM, Takashi Natsume wrote:<br>
>>>>><br>
>>>>> Hi Nova and Cinder developers.<br>
>>>>><br>
>>>>> As I reported in a bug report [1], nova swap volume<br>
>>>>> (updating an attached volume) fuction does not work<br>
>>>>> in the case of non admin users by default.<br>
>>>>> (Volumes are stuck.)<br>
>>>>><br>
>>>>> Before I was working for fixing another swap volume bug [2][3].<br>
>>>>> But Ryan fixed it on the Cinder side [4].<br>
>>>>> As a result, admin users can execute swap volume function,<br>
>>>>> but it was not fixed in the case of non admin users.<br>
>>>>> So I reported the bug report [1].<br>
>>>>><br>
>>>>> In the patch[5], I tried to change the default cinder's policy<br>
>>>>> to allow non admin users to execute migrate_volume_completion API.<br>
>>>>> But it was rejected by the cinder project ('-2' was voted).<br>
>>>>><br>
>>>>> In the patch[5], it was suggested to make the swap volume API admin<br>
>>>>> only<br>
>>>>> on the Nova side.<br>
>>>>> But IMO, the swap volume function should be allowed to non admin users<br>
>>>>> because attaching a volume and detaching a volume can be performed<br>
>>>>> by non admin users.<br>
>>>><br>
>>>><br>
>>>> I agree with this. DuncanT said in IRC that he didn't think non-admin<br>
>>>> users should be using the swap-volume API in nova because it can be<br>
>>>> problematic, but I'm not sure why, is there more history or detail<br>
>>>> there? I'd think it shouldn't be any worse than doing a detach/attach in<br>
>>>> quick succession (like in a CI test for example).<br>
>>>><br>
>>>>><br>
>>>>> If migrate_volume_completion is only allowed to admin users<br>
>>>>> by default on the Cinder side, attaching a new volume and<br>
>>>>> detaching an old volume should be performed on the Nova side<br>
>>>>> when swapping volumes.<br>
>>>><br>
>>>><br>
>>>> My understanding of the problem is as follows:<br>
>>>><br>
>>>> 1. Admin-initiated volume migration in Cinder calls off to Nova to<br>
>>>> perform the swap-volume, and then Nova calls back to Cinder's<br>
>>>> migrate_volume_completion API. This is fine since it's an admin that<br>
>>>> initiated this series of operations on the Cinder side (that's by<br>
>>>> default, however, this is broken if the policy file for Cinder is change<br>
>>>> to allow non-admins to migrate volumes).<br>
>>>><br>
>>>> 2. A non-admin swap-volume API call in Nova fails because Nova blindly<br>
>>>> makes the migrate_volume_completion call to Cinder which fails with a<br>
>>>> 403 because the Cinder API policy has that as an admin action by<br>
>>>> default.<br>
>>>><br>
>>>> I don't know the history around when the swap-volume API was added to<br>
>>>> Nova, was it specifically for this volume migration scenario in Cinder?<br>
>>>>   Are there other use cases?  Knowing those would be good to determine<br>
>>>> if Nova should change it's default policy for swap-volume, although,<br>
>>>> again, that's only a default and can be changed per deployment so we<br>
>>>> probably shouldn't rely on it.<br>
>>>><br>
>>>> Ideally we would have implemented this like the nova/neutron server<br>
>>>> events callback API in Nova during vif plugging (nova does the vif plug<br>
>>>> on the host then waits for neutron to update it's database for the port<br>
>>>> status and sends an event (API call) to nova to continue booting the<br>
>>>> server). That server events API in nova is admin-only by default and<br>
>>>> neutron is configured with admin credentials for nova to use it.<br>
>>>><br>
>>>> Another option would be for Nova to handle a 403 response when calling<br>
>>>> Cinder's migrate_volume_completion API and ignore it if we don't have an<br>
>>>> admin context. This is pretty hacky though. It assumes that it's a<br>
>>>> non-admin user initiating the swap-volume operation. It wouldn't be a<br>
>>>> problem for the volume migration operation initiated in Cinder since by<br>
>>>> default that's admin-only, so nova shouldn't get a 403 when calling<br>
>>>> migrate_volume_completion. The trap would be if the cinder policy for<br>
>>>> volume migration was changed to allow non-admins, but if someone did<br>
>>>> that, they should also change the policy for migrate_volume_completion<br>
>>>> to allow non-admin too.<br>
>>>><br>
>>>>><br>
>>>>> If you have a good idea, please let me know it.<br>
>>>>><br>
>>>>> [1] Cinder volumes are stuck when non admin user executes nova swap<br>
>>>>> volume API<br>
>>>>>      <a href="https://bugs.launchpad.net/cinder/+bug/1522705" rel="noreferrer" target="_blank">https://bugs.launchpad.net/cinder/+bug/1522705</a><br>
>>>>><br>
>>>>> [2] Cinder volume stuck in swap_volume<br>
>>>>>      <a href="https://bugs.launchpad.net/nova/+bug/1471098" rel="noreferrer" target="_blank">https://bugs.launchpad.net/nova/+bug/1471098</a><br>
>>>>><br>
>>>>> [3] Fix cinder volume stuck in swap_volume<br>
>>>>>      <a href="https://review.openstack.org/#/c/207385/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/207385/</a><br>
>>>>><br>
>>>>> [4] Fix swap_volume for case without migration<br>
>>>>>      <a href="https://review.openstack.org/#/c/247767/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/247767/</a><br>
>>>>><br>
>>>>> [5] Enable volume owners to execute migrate_volume_completion<br>
>>>>>      <a href="https://review.openstack.org/#/c/253363/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/253363/</a><br>
>>>>><br>
>>>>> Regards,<br>
>>>>> Takashi Natsume<br>
>>>>> NTT Software Innovation Center<br>
>>>>> E-mail: <a href="mailto:natsume.takashi@lab.ntt.co.jp">natsume.takashi@lab.ntt.co.jp</a><br>
>>>>><br>
>>>>><br>
>>>>><br>
>>>>><br>
>>>>><br>
>>>>><br>
>>>>> __________________________________________________________________________<br>
>>>>><br>
>>>>><br>
>>>>><br>
>>>>> OpenStack Development Mailing List (not for usage questions)<br>
>>>>> Unsubscribe:<br>
>>>>> <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
>>>>> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
>>>>><br>
>>>><br>
>>><br>
>>> I also just checked Tempest and apparently we have no coverage for the<br>
>>> swap-volume API in Nova, we should fix that as part of this.<br>
>>><br>
>><br>
>> I've done some more digging. The swap-volume functionality was added to<br>
>> nova here [1].  The cinder use of it for volume migration was added here<br>
>> [2].<br>
>><br>
>> Looking at the cinder volume API for migrate_volume_completion, it<br>
>> expects the source (old) volume to have migration_status set [3].<br>
>><br>
>> So, I think we can easily fix this in Nova by simply not calling<br>
>> volume_migration_completion if old_volume['migration_status'] is None.<br>
>><br>
>> [1]<br>
>><br>
>> <a href="https://github.com/openstack/nova/commit/8f51b120b430c7c21399256f37e1d8f75d030484" rel="noreferrer" target="_blank">https://github.com/openstack/nova/commit/8f51b120b430c7c21399256f37e1d8f75d030484</a><br>
>><br>
>> [2]<br>
>><br>
>> <a href="https://github.com/openstack/nova/commit/0e4bd7f93b9bfadcc2bb6dfaeae7bb5ee00c194b" rel="noreferrer" target="_blank">https://github.com/openstack/nova/commit/0e4bd7f93b9bfadcc2bb6dfaeae7bb5ee00c194b</a><br>
>><br>
>> [3]<br>
>><br>
>> <a href="http://git.openstack.org/cgit/openstack/cinder/tree/cinder/volume/api.py#n1358" rel="noreferrer" target="_blank">http://git.openstack.org/cgit/openstack/cinder/tree/cinder/volume/api.py#n1358</a><br>
>><br>
>><br>
><br>
> Of course that had to be too easy to be true. The volume 'migration_status'<br>
> is only returned for the volume details if you're calling with an admin<br>
> context [1].<br>
><br>
> I think we can still use this, we just can't expect<br>
> volume['migration_status'] to be in the response from the volume GET. If<br>
> it's not there, we can assume we're not doing a migration and we're not an<br>
> admin anyway, so we can't call migrate_volume_completion.<br>
<br>
</div></div>So in that case, we need to attach, detach volume on nova side right?<br>
I mean if migrate_volume_completion is not being called then<br>
new volume attachment and old volume detachment should be initiated explicitly.<br>
<br>
Can we make Nova default policy same as cinder, i mean swap volume<br>
allowed only for admin? Because if there is simple swap initiated from<br>
nova(not cinder migration), Nova allow that operation for non-admin<br>
user and get stuck to attaching/detaching status.<br>
<span class="im HOEnZb"><br>
><br>
> [1]<br>
> <a href="http://git.openstack.org/cgit/openstack/cinder/tree/cinder/api/v2/views/volumes.py#n82-L84" rel="noreferrer" target="_blank">http://git.openstack.org/cgit/openstack/cinder/tree/cinder/api/v2/views/volumes.py#n82-L84</a><br>
><br>
> --<br>
><br>
> Thanks,<br>
><br>
> Matt Riedemann<br>
><br>
><br>
> __________________________________________________________________________<br>
> OpenStack Development Mailing List (not for usage questions)<br>
> Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
> <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br>
</span><div class="HOEnZb"><div class="h5">__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature"><div dir="ltr"><div>-- <br>Duncan Thomas</div></div></div>
</div>