Consolidation: Deleting old backing files does not work

[GreyKat]

Function get_snapshot_chain is called after consolidation, so it returns just current image of VM. The following fix works for me:

diff --git a/fi-backup.sh b/fi-backup.sh
index a093df2..26474a1 100755
--- a/fi-backup.sh
+++ b/fi-backup.sh
@@ -426,6 +426,15 @@ function consolidate_domain() {
       if [ -n "$backing_file" ]; then
          print_v d "Parent block device: '$backing_file'"

+         snapshot_chain=()
+         #get an array of the snapshot chain starting from last child and iterating backwards
+         # e.g.    [0]     [1]      [2]     [3]
+         #       snap3 <- snap2 <- snap1 <- orig
+         #
+         # blockcommit: orig -> snap1 -> snap2 -> snap3 [becomes] orig
+         # blockpull:   orig -> snap1 -> snap2 -> snap3 [becomes] snap3
+         get_snapshot_chain "$block_device" snapshot_chain
+
          # Consolidate the block device
          #echo "ABOUT TO RUN:" 
          #echo "$VIRSH -q $CONSOLIDATION_METHOD $domain_name $block_device ${CONSOLIDATION_FLAGS[*]}"
@@ -438,15 +447,6 @@ function consolidate_domain() {
             return 1
          fi

-         snapshot_chain=()
-         #get an array of the snapshot chain starting from last child and iterating backwards
-         # e.g.    [0]     [1]      [2]     [3]
-         #       snap3 <- snap2 <- snap1 <- orig
-         #
-         # blockcommit: orig -> snap1 -> snap2 -> snap3 [becomes] orig
-         # blockpull:   orig -> snap1 -> snap2 -> snap3 [becomes] snap3
-         get_snapshot_chain "$block_device" snapshot_chain
-
          if [ "$CONSOLIDATION_METHOD" == "blockcommit" ]; then
             # --delete option for blockcommit doesn't work (tested on
             # LibVirt 1.2.16, QEMU 2.3.0), so we need to manually delete old

[dguerri]

Awesome, thanks! Can you pease submit a pull request I can merge straight away? If not I will do it myself :)

[GreyKat]

Just add it yourself ;)

And thanks for this great tool!

[mnombre]

Hi,

Le 28 juil. 2016 à 17:08, GreyKat notifications@github.com a écrit :

Just add it yourself ;)

And thanks for this great tool!

Yes thanks very much!

NM

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[dguerri]

You are welcome! Unfortunately there is a problem with tests as the libvirt "hack" doesn't work anymore. I will look into this asap, meanwhile if you are need the updated version you can clone the bugfix/deleting-old-bf branch

[AJRepo]

@GreyKat

1) Nice catch!

2) You said the function "get_backing_file" is called after consolidation. However the function get_backing_file IS called before consolidation on line 425. Consolidation doesn't start until line 429. I think you meant the function "get_snapshot_chain"

@dguerri Testing manually was ok except for one time when I ran into this issue ( https://bugzilla.redhat.com/show_bug.cgi?id=1197592 ) using blockcommit and running virsh version 1.3.1 and qemu-img version 2.5.0 (standards on Ubuntu 16.04.1 as of this date). I've not debugged it further because it doesn't happen 100% of the time and I'm time limited for the next few weeks. So This is just a side note for future tests: In theory, if the above libvirt race condition bug occurs again, the consolidation command should bomb out and stop the script before the deletes happen. Should confirm though. Just to be clear this is not a fi-backup.sh bug, but a core issue with libvirt.

[GreyKat]

@AJRepo Yep, you are right, it was a typo (bad copy/paste). The function is get_snapshot_chain

And it is running: CentOS Linux release 7.2.1511 (Core) 3.10.0-327.18.2.el7.x86_64 libVirt version '1.2.17' support is experimental QEMU/KVM version '2.0.0,' is supported

[AJRepo]

This is now working - closing.