Unshelving a VM breaks instance metadata when using qcow2 backed images

Nice analysis.

You're correct that when we shelve an instance, we create a snapshot image, starting in the API:

https://github.com/openstack/nova/blob/b6a245f0425a07be3871a976952646d2bdd44533/nova/compute/api.py#L3244

That snapshot image_id is passed down to the compute service to do the actual snapshot and upload from the virt driver:

https://github.com/openstack/nova/blob/b6a245f0425a07be3871a976952646d2bdd44533/nova/compute/manager.py#L4598

We then store that snapshot image_id in the instance system_metadata for later when it's unshleved:

https://github.com/openstack/nova/blob/b6a245f0425a07be3871a976952646d2bdd44533/nova/compute/manager.py#L4601

When we unshelve, we get that snapshot image from glance:

https://github.com/openstack/nova/blob/b6a245f0425a07be3871a976952646d2bdd44533/nova/conductor/manager.py#L641

We then use that to update the instance.image_ref field to point at the snapshot image:

https://github.com/openstack/nova/blob/b6a245f0425a07be3871a976952646d2bdd44533/nova/compute/manager.py#L4764

It looks like the problem is that we then reset the instance.image_ref to the old image id before we unshelved:

https://github.com/openstack/nova/blob/b6a245f0425a07be3871a976952646d2bdd44533/nova/compute/manager.py#L4796

I have no idea why we do that, and that's probably the bug.

Looks like this was introduced with https://review.openstack.org/#/c/72407/ as maybe a way to pass the shelved image_id via the instance.image_ref field to the driver.spawn() method, but that doesn't make sense now.

Looking at the review comments on https://review.openstack.org/#/c/72407/, it looks like this was intentional:

Nikola Dipanov
Feb 10, 2014

Patch Set 2: I would prefer that you didn't merge this

Looking at the code - I am not sure we actually want to do this.

The instance should keep it's old image in the db once it has been unshelved, but it needs the new image because it will download it in the compute manager when it calls driver.spawn.

At the very least, we should put the actual image back to the instance once the unshelve is done, (so end of manager call).

Going forward, we might want to change what gets passed to spawn so that it can decide what to download. Keep in mind that right now we have the image as a block device in the db (even though we don't use it)

--

I have no idea why we should say the instance is backed by the original image ref rather than the snapshot image ref from the shelved offloaded instance, that's totally confusing and wrong IMO.

Now I'm seeing this:

https://github.com/openstack/nova/blob/b6a245f0425a07be3871a976952646d2bdd44533/nova/compute/manager.py#L4797

So apparently we set the instance.image_ref back to the original image_ref because after we successfully spawn the guest during unshelve, we delete the snapshot image reference for some reason, I have no idea why.

http://lists.openstack.org/pipermail/openstack-dev/2017-December/125124.html

Thanks for the activity on this Matt. I don't think there's an obvious solution with only upsides for this problem.

Changing the image ref to the original image has its upsides. It's end-user friendly, since their instance behaves like it was "shelved", not like it was snapshotted, killed and respawned. E.g. rebuild and rescues of instances work better when it has the original image in the image_ref.

Not deleting the snapshot and having the image_ref as the snapshot might also have other negative side effects like possibly aggressively growing the glance image repo for a tenant. Then some cleanup logic would have to be added, which adds complexity and doesn't sound pretty.

If this only affects the qcow2 storage case (I'm not familiar with all storage backend options), a good compromise could be to

1) If the original image exists in glance, do a qemu-img rebase to that image. Then the VM is in the state we want.

2) If the original image can't be found, update the image-ref to the snapshot, and don't delete the snapshot.

But just my 2c from the admin side :). I'm not familiar enough with the nova codebase to know the feasibility of this.

Fix proposed to branch: master
Review: https://review.openstack.org/524726

I reproduced this bug and found that the directory of base file is stored in disk_info, which is used in finish_migration function. So, may be we can modify this parameter to rebase to original image.

@Sun, unshelve does not call finish_migration() so I'm not sure what you're referring to. I guess because of the relationship between resizing a server *after* unshelving it?

Removing myself as I'm not actively working on this.

Just thought of this, but nova does store the originally requested image_id in the request_specs table in the nova_api database - so if we needed to find that for whatever reason, we could. And then leave the instance.image_ref pointing at the actual current image that the instance is using (which could be a shelved snapshot).

More thoughts in the ML on my idea from comment 11:

http://lists.openstack.org/pipermail/openstack-dev/2018-September/134855.html

Change abandoned by Matt Riedemann (<email address hidden>) on branch: master
Review: https://review.openstack.org/524726
Reason: I'm not pursuing this.

Fix proposed to branch: master
Review: https://review.opendev.org/696084

discuss was reopen here:
http://lists.openstack.org/pipermail/openstack-discuss/2019-November/011070.html

Related fix proposed to branch: master
Review: https://review.opendev.org/743537

Reviewed: https://review.opendev.org/743537
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=ce2203456660083119cdbb7e73c1ad15e6e0a074
Submitter: Zuul
Branch: master

commit ce2203456660083119cdbb7e73c1ad15e6e0a074
Author: Alexandre Arents <email address hidden>
Date: Tue Nov 26 10:26:32 2019 +0000

    Make _rebase_with_qemu_img() generic

    Move volume_delete related logic away from this method, in order to make
    it generic and usable elsewhere.

    Change-Id: I17357d85f845d4160cb7c7784772530a1e92af76
    Related-Bug: #1732428

Reviewed: https://review.opendev.org/696084
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=8953a689467f8c3e996086392251de67953a45ba
Submitter: Zuul
Branch: master

commit 8953a689467f8c3e996086392251de67953a45ba
Author: Alexandre Arents <email address hidden>
Date: Tue Nov 26 10:26:32 2019 +0000

    Rebase qcow2 images when unshelving an instance

    During unshelve, instance is spawn with image created by shelve
    and is deleted just after, instance.image_ref still point
    to the original instance build image.

    In qcow2 environment, this is an issue because instance backing file
    don't match anymore instance.image_ref and during live-migration/resize,
    target host will fetch image corresponding to instance.image_ref
    involving instance corruption.

    This change fetches original image and rebase instance disk on it.
    This avoid image_ref mismatch and bring back storage benefit to keep common
    image in cache.

    If original image is no more available in glance, backing file is merged into
    disk(flatten), ensuring instance integrity during next live-migration/resize
    operation.

    Change-Id: I1a33fadf0b7439cf06c06cba2bc06df6cef0945b
    Closes-Bug: #1732428

Related fix proposed to branch: master
Review: https://review.opendev.org/749035

Related fix proposed to branch: master
Review: https://review.opendev.org/749205

Change abandoned by yatin (<email address hidden>) on branch: master
Review: https://review.opendev.org/749035
Reason: In favor of https://review.opendev.org/#/c/749205/

Reviewed: https://review.opendev.org/749205
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d
Submitter: Zuul
Branch: master

commit fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d
Author: Alexandre Arents <email address hidden>
Date: Tue Sep 1 08:26:25 2020 +0000

    Update image_base_image_ref during rebuild.

    In different location we assume system_metadata.image_base_image_ref
    exists, because it is set during instance creation in method
    _populate_instance_for_create

    But once instance is rebuild, all system_metadata image property a dropped
    and replace by new image property and without setting back
    image_base_image_ref.

    This change propose to set image_base_image_ref during rebuild.

    In specific case of shelve/unshelve in Qcow2 backend, image_base_image_ref is
    used to rebase disk image, so we ensure this property is set as instance may
    have been rebuild before the fix was apply.

    Related-Bug: #1732428
    Closes-Bug: #1893618
    Change-Id: Ia3031ea1f7db8b398f02d2080ca603ded8970200

Related fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/c/openstack/nova/+/787943

Related fix proposed to branch: stable/train
Review: https://review.opendev.org/c/openstack/nova/+/787945

Reviewed: https://review.opendev.org/c/openstack/nova/+/785625
Committed: https://opendev.org/openstack/nova/commit/7003618884b5e033f822685541da6bc7955fe55a
Submitter: "Zuul (22348)"
Branch: stable/ussuri

commit 7003618884b5e033f822685541da6bc7955fe55a
Author: Alexandre Arents <email address hidden>
Date: Tue Nov 26 10:26:32 2019 +0000

    Rebase qcow2 images when unshelving an instance

    During unshelve, instance is spawn with image created by shelve
    and is deleted just after, instance.image_ref still point
    to the original instance build image.

    In qcow2 environment, this is an issue because instance backing file
    don't match anymore instance.image_ref and during live-migration/resize,
    target host will fetch image corresponding to instance.image_ref
    involving instance corruption.

    This change fetches original image and rebase instance disk on it.
    This avoid image_ref mismatch and bring back storage benefit to keep common
    image in cache.

    If original image is no more available in glance, backing file is merged into
    disk(flatten), ensuring instance integrity during next live-migration/resize
    operation.

    Change-Id: I1a33fadf0b7439cf06c06cba2bc06df6cef0945b
    Closes-Bug: #1732428
    (cherry picked from commit 8953a689467f8c3e996086392251de67953a45ba)

Reviewed: https://review.opendev.org/c/openstack/nova/+/787943
Committed: https://opendev.org/openstack/nova/commit/560414036d9f2ebcfeb8626029d0bd849d6cad44
Submitter: "Zuul (22348)"
Branch: stable/ussuri

commit 560414036d9f2ebcfeb8626029d0bd849d6cad44
Author: Alexandre Arents <email address hidden>
Date: Tue Sep 1 08:26:25 2020 +0000

    Update image_base_image_ref during rebuild.

    In different location we assume system_metadata.image_base_image_ref
    exists, because it is set during instance creation in method
    _populate_instance_for_create

    But once instance is rebuild, all system_metadata image property a dropped
    and replace by new image property and without setting back
    image_base_image_ref.

    This change propose to set image_base_image_ref during rebuild.

    In specific case of shelve/unshelve in Qcow2 backend, image_base_image_ref is
    used to rebase disk image, so we ensure this property is set as instance may
    have been rebuild before the fix was apply.

    NOTE(lyarwood): An additional change is required within the
    ShelvePolicyTestV21 test on stable/ussuri to avoid the system_metadata
    of the fake instance being lazy loaded during the test.

    Related-Bug: #1732428
    Closes-Bug: #1893618
    Change-Id: Ia3031ea1f7db8b398f02d2080ca603ded8970200
    (cherry picked from commit fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d)

This issue was fixed in the openstack/nova 21.2.1 release.

Reviewed: https://review.opendev.org/c/openstack/nova/+/780782
Committed: https://opendev.org/openstack/nova/commit/c61ceac5d0d0844db85791da167cf224ce360a5f
Submitter: "Zuul (22348)"
Branch: stable/train

commit c61ceac5d0d0844db85791da167cf224ce360a5f
Author: Alexandre Arents <email address hidden>
Date: Tue Nov 26 10:26:32 2019 +0000

    Make _rebase_with_qemu_img() generic

    Move volume_delete related logic away from this method, in order to make
    it generic and usable elsewhere.

    NOTE(lyarwood): Conflict caused by I52fbbcac9dc386f24ee81b3321dd0d8355e01976
    landing in stbale/ussuri.

    Conflicts:
      nova/tests/unit/virt/libvirt/test_driver.py

    Change-Id: I17357d85f845d4160cb7c7784772530a1e92af76
    Related-Bug: #1732428
    (cherry picked from commit ce2203456660083119cdbb7e73c1ad15e6e0a074)
    (cherry picked from commit 2e89699c3301bf801784c637b6919752fcd3503f)

Reviewed: https://review.opendev.org/c/openstack/nova/+/785627
Committed: https://opendev.org/openstack/nova/commit/3bc1502f009a8d188ddee9c46888c3bac88180fe
Submitter: "Zuul (22348)"
Branch: stable/train

commit 3bc1502f009a8d188ddee9c46888c3bac88180fe
Author: Alexandre Arents <email address hidden>
Date: Tue Nov 26 10:26:32 2019 +0000

    Rebase qcow2 images when unshelving an instance

    During unshelve, instance is spawn with image created by shelve
    and is deleted just after, instance.image_ref still point
    to the original instance build image.

    In qcow2 environment, this is an issue because instance backing file
    don't match anymore instance.image_ref and during live-migration/resize,
    target host will fetch image corresponding to instance.image_ref
    involving instance corruption.

    This change fetches original image and rebase instance disk on it.
    This avoid image_ref mismatch and bring back storage benefit to keep common
    image in cache.

    If original image is no more available in glance, backing file is merged into
    disk(flatten), ensuring instance integrity during next live-migration/resize
    operation.

    NOTE(lyarwood): Test conflicts caused by If56842da51688 not being
    present in stable/train.

    Conflicts:
      nova/tests/unit/virt/libvirt/test_driver.py

    Change-Id: I1a33fadf0b7439cf06c06cba2bc06df6cef0945b
    Closes-Bug: #1732428
    (cherry picked from commit 8953a689467f8c3e996086392251de67953a45ba)
    (cherry picked from commit 7003618884b5e033f822685541da6bc7955fe55a)

Reviewed: https://review.opendev.org/c/openstack/nova/+/787945
Committed: https://opendev.org/openstack/nova/commit/8a01a58a60e7f4f2c189f8ca065d8c21ccbde04f
Submitter: "Zuul (22348)"
Branch: stable/train

commit 8a01a58a60e7f4f2c189f8ca065d8c21ccbde04f
Author: Alexandre Arents <email address hidden>
Date: Tue Sep 1 08:26:25 2020 +0000

    Update image_base_image_ref during rebuild.

    In different location we assume system_metadata.image_base_image_ref
    exists, because it is set during instance creation in method
    _populate_instance_for_create

    But once instance is rebuild, all system_metadata image property a dropped
    and replace by new image property and without setting back
    image_base_image_ref.

    This change propose to set image_base_image_ref during rebuild.

    In specific case of shelve/unshelve in Qcow2 backend, image_base_image_ref is
    used to rebase disk image, so we ensure this property is set as instance may
    have been rebuild before the fix was apply.

    Related-Bug: #1732428
    Closes-Bug: #1893618
    Change-Id: Ia3031ea1f7db8b398f02d2080ca603ded8970200
    (cherry picked from commit fe52b6c25bebdd1b459c7a59fbb8d9f6de200c9d)
    (cherry picked from commit 560414036d9f2ebcfeb8626029d0bd849d6cad44)

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/queens
Review: https://review.opendev.org/c/openstack/nova/+/780789
Reason: This branch transitioned to End of Life for this project, open patches needs to be closed to be able to delete the branch.

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/rocky
Review: https://review.opendev.org/c/openstack/nova/+/780787
Reason: This branch transitioned to End of Life for this project, open patches needs to be closed to be able to delete the branch.

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/stein
Review: https://review.opendev.org/c/openstack/nova/+/780784
Reason: This branch transitioned to End of Life for this project, open patches needs to be closed to be able to delete the branch.

This issue was fixed in the openstack/nova train-eol release.