[SRU] nova resize doesn't resize(extend) rbd disk files when using rbd disk backend

[Yaguang Tang]

[Impact]

  Instance resize does not work if the target host has a cached copy of
  the root disk. The resize will silently fail but be displayed as
  successful in Nova.

[Test Case]

  1 deploy nova-compute with RBDImageBackend enabled
  2 boot an instance from a QCOW2 image (to guarantee it gets downloaded for reformat prior to re-upload to ceph)
  3 nova resize using flavor with larger root disk
  4 wait for instance resize migration to complete
  5 verify root disk actually resized by checking /proc/partitions in vm
  6 do nova resize-confirm
  7 repeat steps 3-6

[Regression Potential]

 * None

== original description below ==

tested with nova trunk commit eb860c2f219b79e4f4c5984415ee433145197570

Configured Nova to use rbd disk backend

nova.conf

[libvirt]
images_type=rbd

instances booted successfully and instance disks are in rbd pools, when perform a nova resize to an existing instance, memory and CPU changed to be new flavors but instance disks size doesn't change

[Matt Riedemann (mriedem)]

It's probably the same thing causing the failure in the experimental ceph job:

http://logs.openstack.org/30/133530/2/experimental/check-tempest-dsvm-full-ceph/e4e5706/console.html#_2014-11-10_18_41_56_101

[Jon Bernard (jbernard)]

I believe the ceph job failure is different, in that case the original image is deleted and libvirt fails to open the disk as a result during the resize test. But this could be related, I'm not sure. I'll take this bug and see if I can come up with something.

[Nick Jones (yankcrime)]

Seeing the same issue here on Icehouse with the recommended patches [1] applied. There's no failure per se - the resize operation completes successfully and the new details are listed against the instance - but the underlying block device doesn't appear to be changed at all.

[1] https://github.com/angdraug/nova/tree/rbd-ephemeral-clone-stable-icehouse

[Daniel Speichert (dasp)]

I don't see this happen in Juno stable, upstream nova version.

[Matt Jarvis (matt-jarvis)]

What do you mean by upstream nova version ? We're running Juno and this still happens.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[Liang Chen (cbjchen)]

Hi,

I just came across the same problem and took a look at the code. Below is some of my findings.

1. disk_info is not passed to finish_migration, thus no disk resize operation is even attempted.
At https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L6036, network type isn't considered a valid disk type and migrate_disk_and_power_off uses this code to load the disk info that will be eventually feeded to finish_migration to determine if a disk resize is required, https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L6401 .

2. resizing rbd backed disk is not supported

Even if disk_info is successfully passed to finish_migration, the _disk_resize function invoked at https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L6404 doesn't support rbd backed disk. All it does at the end is a qemu-img resize call - https://github.com/openstack/nova/blob/master/nova/virt/disk/api.py#L190.

Hope it helps.

Thank,
Liang

[Nicolas Simonds (nicolas.simonds)]

On point #1, the root cause is that the imagebackend code currently relies on the create_image() method to resize the image as well, and in the Ceph-backed case, it will gleefully do nothing (and claim victory) if the image already exists. Splitting the create and resize operations into two discrete methods takes care of that, and is handled in the patch set https://review.openstack.org/187857

On point #2, you are absolutely correct, there is no actual support on the backend for this, and it's amazing to me that it was released in this state. The support is added in patch set https://review.openstack.org/187395

Thanks,

[Michael Still (mikal)]

Given this bug was present in Juno, is it really reasonable to block a Liberty release until this is fixed?

[Matt Riedemann (mriedem)]

Sounds like this isn't a regression, just a thing that's never worked properly, so yeah, removing the liberty-rc-potential tag.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/liberty
Review: https://review.openstack.org/228504

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/liberty
Review: https://review.openstack.org/228505

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/187857
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=c56bbc61f44529042ae9126a9a0b62affe0276d0
Submitter: Jenkins
Branch: master

commit c56bbc61f44529042ae9126a9a0b62affe0276d0
Author: Nicolas Simonds <email address hidden>
Date: Tue Jun 2 14:59:42 2015 -0700

    libvirt: Split out resize_image logic from create_image

    Rather than rely on the dubiously-named create_image() method to
    handle resizing existing images, let the imagebackend caching layer
    figure out if the resize needs to be done, and call resize_image()
    to do the work when appropriate.

    Partial-Bug: 1369465
    Change-Id: I28ba1970317f3d5691d05283796d4345ff38a804

[John Garbutt (johngarbutt)]

removed from RC2, its too late to get this merged now, and I would rather have both patches, or neither merged.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Matt Riedemann (<email address hidden>) on branch: stable/liberty
Review: https://review.openstack.org/228505
Reason: Abandoning since the master change hasn't been approved, please restore and re-propose once that happens if you still need this for stable/liberty.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Matt Riedemann (<email address hidden>) on branch: stable/liberty
Review: https://review.openstack.org/228504
Reason: This is merged on master but is a dependency for another backport that is not merged on master, and to backport that change to stable/liberty it needs to be approved on master first. And when backporting the changes to stable/liberty, they should be squashed (IMO), or at least be approved together so we don't land something that is unnecessary without the other change.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/187395
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=29476a67d4d4ff7eedd97f589422d47015111448
Submitter: Jenkins
Branch: master

commit 29476a67d4d4ff7eedd97f589422d47015111448
Author: Nicolas Simonds <email address hidden>
Date: Mon Jun 1 13:58:37 2015 -0700

    libvirt: Fix/implement revert-resize for RBD-backed images

    * Makes a snapshot of Ceph-backed roots prior to resize
    * Rolls back to snapshot on revert
    * Destroys resize snapshots on image cleanup

    Closes-Bug: 1369465
    Closes-Bug: 1314526
    Change-Id: I328d2c41696a9c0f090f822a51ea42fac83f62ec

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Andy Jeffries (8-andy)]

Any chance of a backport to Kilo for this fix?

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/228505
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=eb1f67c4374fd8210829deac8e49d6f3df6cae4b
Submitter: Jenkins
Branch: stable/liberty

commit eb1f67c4374fd8210829deac8e49d6f3df6cae4b
Author: Nicolas Simonds <email address hidden>
Date: Mon Jun 1 13:58:37 2015 -0700

    libvirt: Fix/implement revert-resize for RBD-backed images

    * Makes a snapshot of Ceph-backed roots prior to resize
    * Rolls back to snapshot on revert
    * Destroys resize snapshots on image cleanup

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

    because (I70215fb25ef25422786b96d33c91d8f1d4760a23) isn't on liberty

    (cherry picked from commit 29476a67d4d4ff7eedd97f589422d47015111448)

    Closes-Bug: 1369465
    Closes-Bug: 1314526
    Change-Id: I328d2c41696a9c0f090f822a51ea42fac83f62ec

[Doug Hellmann (doug-hellmann)]

This issue was fixed in the openstack/nova 13.0.0.0b3 development milestone.

[Doug Hellmann (doug-hellmann)]

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

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/kilo
Review: https://review.openstack.org/294599

[OpenStack Infra (hudson-openstack)]

Change abandoned by Liang Chen (<email address hidden>) on branch: stable/kilo
Review: https://review.openstack.org/294599

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "wily-liberty debian patch" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

[Matt Jarvis (matt-jarvis)]

This is still affecting us, any chance of a kilo backport ?

[Liang Chen (cbjchen)]

Matt,
I made debian patch for wily-liberty. But the patch depends on many other commits that kilo doesn't have, and I would be too risky to backport them all to kilo. So I am not going propose a kilo backport. Sorry for the inconvenience.

Thanks,
Liang

[Edward Hope-Morley (hopem)]

This fix already exists in the released Ubuntu package of Liberty nova for both wily-updates and the Trusty/Liberty Ubuntu Cloud Archive so I am going to abandon the SRU.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/liberty
Review: https://review.openstack.org/334074

[Edward Hope-Morley (hopem)]

For clarity, there are two patches associated with this bug. One of them was backported to stable/liberty but the other was not so I am going to re-add the backport-potential tag in reference to https://review.openstack.org/334074. Both of these patches are needed to get resizes working properly in Liberty.

[Edward Hope-Morley (hopem)]

Resubmitting sru as per above.

[Ryan Beisner (1chb1n)]

Hello Yaguang, or anyone else affected,

Accepted nova into liberty-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:liberty-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-liberty-needed to verification-liberty-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-liberty-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Edward Hope-Morley (hopem)]

Ran test sequence on Trusty with proposed Liberty UCA and I no longer see the original problem.

[James Page (james-page)]

The verification of the Stable Release Update for nova has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[James Page (james-page)]

This bug was fixed in the package nova - 2:12.0.4-0ubuntu1~cloud1
---------------

 nova (2:12.0.4-0ubuntu1~cloud1) trusty-liberty; urgency=medium
 .
   * Backport fix for image resize bug (LP: #1369465)
     - d/p/libvirt-Split-out-resize_image-logic-from-create_ima.patch

[Jorge Niedbalski (niedbalski)]

The latest patch for Liberty d/p/libvirt-Split-out-resize_image-logic-from-create_ima.patch introduced this regression https://bugs.launchpad.net/nova/+bug/1608934

As a side note, Please note that this patch is not currently integrated in upstream/liberty.

[Edward Hope-Morley (hopem)]

Hi Jorge, I am unable to reproduce the issue described in bug 1608934. I have tried it with both Trusty Liberty and Trusty Mitaka (which has the same version as Xenial) i.e. boot a vm using a flavor that has a root and ephemeral disk, and the vm boots us just fine. For clarity I have tried with (default) qcow2 imagebackend as well as the raw and rbd imagebackends without any problems at all. Could you please describe the sequence that you have used to trigger your problem and provide some logs?

As a side note, the patch backported here already exists in Mitaka and beyond so if it really does introduce a regression then we should first check if there is a fix from upstream that we can backport to fix this issue and if not then we should propose it ourselves otherwise we are fixing one version but leaving the next broken.

[Edmund Rhudy (erhudy)]

Jorge is reporting it on my behalf; the error occurs for me with images_type=lvm. images_type=rbd is fine.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Matt Riedemann (<email address hidden>) on branch: stable/liberty
Review: https://review.openstack.org/334074
Reason: This doesn't look appropriate for stable/liberty at this point, and it's stale anyway so I'm going to abandon it.