virsh blockcommit hangs at 100%

[Stefan Reiff]

[Impact]

 * Virsh can't merge snapshots using the Block Commit technique
 * It is only possible to merge base to top image (not the opposite)
 * Combination of libvirt v1.2.2 and QEMU 2 is no good for Block Commit
 * After this fix, virsh won't hang on the opration, will cause an error

[Test Case]

$ virsh list --all
 Id Name State
----------------------------------------------------
 4 guest running

$ virsh domblklist --domain guest
Target Source
------------------------------------------------
vda /var/lib/libvirt/images/guest.1487880011

$ sudo qemu-img info --backing-chain /var/lib/libvirt/images/guest.1487880011 | grep image:
image: /var/lib/libvirt/images/guest.1487880011
image: /var/lib/libvirt/images/guest.qcow2

inaddy@(kvm02):~$ virsh blockcommit --domain=guest --path=/var/lib/libvirt/images/guest.1487880011 --base=/var/lib/libvirt/images/guest.qcow2 --wait --verbose
Block Commit: [100 %] <----- STUCKS HERE

[Regression Potential]

 * Block commit for Active layers would continue not working
 * Block commit for Intermediary layers would not work
 * Its a small fix based on upstream fix (to block this function)
 * Has been tested a lost on verification

[Other Info]

## Original Case Description

virsh blockcommit hangs at 100% and nothing happens.

[Launchpad Janitor (janitor)]

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

[jlord87 (moro-87)]

I can confirm this bug in my 14.04 box.

Don't know if this bug has been solved in the upstream version of libvirt

[jlord87 (moro-87)]

ok, after a small google search I found this blog

http://kashyapc.com/2014/10/07/libvirt-blockcommit-shorten-disk-image-chain-by-live-merging-the-current-active-disk-content/

it seems that an upstream version of libvirt will allow libvirt users to commit back to the first file all the change made to a vm after an external disk snapshot.

We will probably need to wait for libvirt 1.2.9
https://www.redhat.com/archives/libvirt-users/2014-October/msg00071.html

How long will it takes? 16.04? :(
Isn't here anyone able to make an update PPA of libvirt, qemu/kvm?

cheers

francesco

[Serge Hallyn (serge-hallyn)]

I'm hoping we merge hte newest libvirt release next week into 15.04

[Serge Hallyn (serge-hallyn)]

Since this should be fixed by the new libvirt release now in vivid, marking fix-released.

[MarcoB (marcobnn)]

in Ubuntu Trusty 14.04 Server is still present.

looking at official libvirt changelog, this problem seems to be resolved in 1.2.5 version that is not present in ubuntu trusty (last version is 1.2.2)

Are there any workaround?

[Serge Hallyn (serge-hallyn)]

One workaround would be to use the libvirt version in the cloud archive.

[Rafael David Tinoco (rafaeldtinoco)]

I have marked this as duplicate of a newer one simply because the new one already includes QEMU upstream "package", I'll just find missing commits in between libvirt 1.1 and 1.3 and inform there, making other versions as Fix Released.

[Rafael David Tinoco (rafaeldtinoco)]

No, I'm doing the reverse.

[Rafael David Tinoco (rafaeldtinoco)]

For the Live Snapshot merge you can either rely on:

- Block Streaming or
- Block Commit

(Check: http://wiki.qemu-project.org/Features/Snapshots for more info)

WORKAROUND is to use Block Streaming:

(1) Virtual machine uses its qcow2 image disk:

----
inaddy@(kvm01):~$ sudo ls /var/lib/libvirt/images
guest.qcow2
----

(2) External snapshot is done:

----
inaddy@(kvm01):~$ sudo virsh snapshot-create-as --domain guest --disk-only --atomic
Domain snapshot 1486936526 created
----

(3) From this moment on the new data is being committed into the new image. Old image is still referenced since the new image doesn't contain data from the underlying one.

----
inaddy@(kvm01):~$ sudo ls /var/lib/libvirt/images
guest.1486936526 guest.qcow2

inaddy@(kvm01):~$ sudo qemu-img info --backing-chain /var/lib/libvirt/images/guest.1486936526
image: /var/lib/libvirt/images/guest.1486936526
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 4.9M
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.qcow2
backing file format: qcow2
Format specific information:
compat: 1.1
lazy refcounts: false

image: /var/lib/libvirt/images/guest.qcow2
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 2.1G
cluster_size: 65536
Format specific information:
compat: 1.1
lazy refcounts: false
----

This means that you can "copy" (cp/scp/sftp/rsync/tar) the original image (guest.qcow2) since it is not being updated anymore, but, you still can't move or remove it at this point since its being referenced by the new one.

(4) Pull all data from original disk (into the new one, so the dependency is no longer.

----
inaddy@(kvm01):~$ virsh blockpull --domain guest --path /var/lib/libvirt/images/guest.1486936526
Block Pull started

<wait until the blockpull is over>

inaddy@(kvm01):~$ sudo qemu-img info --backing-chain /var/lib/libvirt/images/guest.1486936526
image: /var/lib/libvirt/images/guest.1486936526
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 2.0G
cluster_size: 65536
Format specific information:
compat: 1.1
lazy refcounts: false
----

(5) After blockpull, the VM only depends on the qcow2 image it is referencing:

----
inaddy@(kvm01):~$ virsh domblklist guest
Target Source
------------------------------------------------
vda /var/lib/libvirt/images/guest.1486936526
hda -

inaddy@(kvm01):~$ sudo rm /var/lib/libvirt/images/guest.qcow2
----

Will get back soon with more info on the "Block Commit" support for Trusty.

[Rafael David Tinoco (rafaeldtinoco)]

I was able to successfully Block Commit using QEMU in Trusty. So the problem lives likely in libvirt:

#### QEMU (2.0.0+dfsg-2ubuntu1.32)

## HMP CONSOLE

# create a snapshot from "guest.qcow2" to "guestsnap.qcow2"

(qemu) snapshot_blkdev disk0 /var/lib/libvirt/images/guestsnap.qcow2 qcow2

## QMP CONSOLE

# merge the snapshot created back to the base "guest.qcow2"

(qemu) block-commit device=disk0 top=/var/lib/libvirt/images/guestsnap.qcow2

{u'return': {}}

# check block commit statistics

(qemu) query-blockstats (... many times ...)

{u'return': [
{
u'device': u'disk0'
u'backing': {
 u'stats': {
  u'rd_operations': 3522
  u'flush_total_time_ns': 3176656651
  u'wr_highest_offset': 30066015744
  u'rd_total_time_ns': 458556786
  u'rd_bytes': 63168000
  u'wr_total_time_ns': 787756681
  u'flush_operations': 1251
  u'wr_operations': 2787
  u'wr_bytes': 16806912
  }
 u'parent': {
  u'stats': {u'rd_operations': 0
  u'flush_total_time_ns': 0
  u'wr_highest_offset': 2724658688
  u'rd_total_time_ns': 0
  u'rd_bytes': 0
  u'wr_total_time_ns': 0
  u'flush_operations': 0
  u'wr_operations': 0
  u'wr_bytes': 0
  }
 }
}
u'stats': {
 u'rd_operations': 217
 u'flush_total_time_ns': 1331436801
 u'wr_highest_offset': 30065970688
 u'rd_total_time_ns': 127773661
 u'rd_bytes': 2453504
 u'wr_total_time_ns': 60389213087
 u'flush_operations': 376
 u'wr_operations': 1015
 u'wr_bytes': 57905152
}
u'parent': {
 u'stats': {
  u'rd_operations': 0
  u'flush_total_time_ns': 0
  u'wr_highest_offset': 59375104
  u'rd_total_time_ns': 0
  u'rd_bytes': 0
  u'wr_total_time_ns': 0
  u'flush_operations': 0
  u'wr_operations': 0
  u'wr_bytes': 0
  }
 }
}
]}

# check block-commit job status (100% == offset - len)

(qemu) query-block-jobs

{u'return': [{
u'busy': False,
u'type': u'commit'
u'len': 32212254720
u'paused': False
u'io-status': u'ok'
u'offset': 32212254720
u'device': u'disk0'
u'speed': 0
}]}

## check job was done and pivot was made successfully

(qemu) block-job-complete device=disk0

{u'return': {}}

## check for the block being used for disk0

(qemu) query-block
{'return': [{
'locked': False
'type': 'unknown'
'io-status': 'ok'
'removable': False
'device': 'disk0'
'inserted': {
 'bps_rd': 0
 'backing_file_depth': 0
 'encrypted': False
 'image': {
  'cluster-size': 65536
  'format': 'qcow2'
  'filename': '/var/lib/libvirt/images/guest.qcow2'
  'virtual-size': 32212254720
  'dirty-flag': False
  'format-specific': {
   'data': {'compat': '1.1'
   'lazy-refcounts': False
   }
  'type': 'qcow2'
  }
 'actual-size': 5322117120
 }
'bps_wr': 0
'drv': 'qcow2'
'bps': 0
'iops': 0
'file': '/var/lib/libvirt/images/guest.qcow2'
'iops_rd': 0
'encryption_key_missing': False
'ro': False
'iops_wr': 0
}
}]}

[Rafael David Tinoco (rafaeldtinoco)]

I was able to block commit using libvirt, finally. Found the problem:

inaddy@(kvm02):~$ virsh snapshot-create-as --domain guest --disk-only --atomic
Domain snapshot 1487158333 created

Formatting '/var/lib/libvirt/images/guest.1487158333', fmt=qcow2 size=32212254720 backing_file='/var/lib/libvirt/images/guest.qcow2' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off

inaddy@(kvm02):~$ virsh domblklist --domain guest
Target Source
------------------------------------------------
vda /var/lib/libvirt/images/guest.1487158333

inaddy@(kvm02):~$ virsh domblkinfo --domain guest --device /var/lib/libvirt/images/guest.1487158333
Capacity: 32212254720
Allocation: 987136
Physical: 987136

inaddy@(kvm02):~$ virsh blockcommit --domain guest vda --base /var/lib/libvirt/images/guest.qcow2 --top /var/lib/libvirt/images/guest.1487158333 --wait --verbose
Block Commit: [100 %]

<VIRSH STUCKS HERE>

<in parallel>

inaddy@(kvm02):~$ virsh qemu-monitor-command guest --pretty -- '{ "execute": "query-blockstats" }'
{
    "return": [
        {
            "device": "drive-virtio-disk0",
            "parent": {
                "stats": {
                    "flush_total_time_ns": 0,
                    "wr_highest_offset": 1834496,
                    "wr_total_time_ns": 0,
                    "wr_bytes": 0,
                    "rd_total_time_ns": 0,
                    "flush_operations": 0,
                    "wr_operations": 0,
                    "rd_bytes": 0,
                    "rd_operations": 0
                }
            },
            "stats": {
                "flush_total_time_ns": 371338917,
                "wr_highest_offset": 19362090496,
                "wr_total_time_ns": 2418722218,
                "wr_bytes": 434176,
                "rd_total_time_ns": 18115039,
                "flush_operations": 34,
                "wr_operations": 76,
                "rd_bytes": 385024,
                "rd_operations": 63
            },
            "backing": {
                "parent": {
                    "stats": {
                        "flush_total_time_ns": 0,
                        "wr_highest_offset": 5328862720,
                        "wr_total_time_ns": 0,
                        "wr_bytes": 0,
                        "rd_total_time_ns": 0,
                        "flush_operations": 0,
                        "wr_operations": 0,
                        "rd_bytes": 0,
                        "rd_operations": 0
                    }
                },
                "stats": {
                    "flush_total_time_ns": 42335832,
                    "wr_highest_offset": 23789047296,
                    "wr_total_time_ns": 82449902,
                    "wr_bytes": 926720,
                    "rd_total_time_ns": 318909795,
                    "flush_operations": 18,
                    "wr_operations": 90,
                    "rd_bytes": 62250496,
                    "rd_operations": 3336
                }
            }
        },
        {
            "device": "drive-ide0-0-0",
            "stats": {
                "flush_total_time_ns": 0,
                "wr_highest_offset": 0,
              ...

[Rafael David Tinoco (rafaeldtinoco)]

Somehow libvirt (virsh) is not checking the block-job execution:

<while the first "virsh blockcommit" is stuck, I can do this...>

inaddy@(kvm02):~$ virsh qemu-monitor-command guest --pretty -- '{ "execute": "query-block-jobs" }'
{
    "return": [
        {
            "io-status": "ok",
            "device": "drive-virtio-disk0",
            "busy": false,
            "len": 32212254720,
            "offset": 32212254720,
            "paused": false,
            "speed": 0,
            "type": "commit"
        }
    ],
    "id": "libvirt-511"
}

inaddy@(kvm02):~$ virsh blockjob guest /var/lib/libvirt/images/guest.1487158333
Block Commit: [100 %]

With "query-block-jobs" you can see if it finished (offset - len = 0). Virsh is not doing this, so I have to finish the job by hand:

inaddy@(kvm02):~$ virsh qemu-monitor-command guest --pretty -- '{ "execute": "block-job-complete", "arguments": { "device": "drive-virtio-disk0" } }'
{
    "return": {

    },
    "id": "libvirt-859"
}

And the command returns:

inaddy@(kvm02):~$ virsh blockcommit --domain guest vda --base /var/lib/libvirt/images/guest.qcow2 --top /var/lib/libvirt/images/guest.1487158333 --wait --verbose
Block Commit: [100 %]
Commit complete

Problem is that QEMU has the correct answer for the backing file:

inaddy@(kvm02):~$ virsh qemu-monitor-command guest --pretty -- '{ "execute": "query-block" }'
{
    "return": [
        {
            "io-status": "ok",
            "device": "drive-virtio-disk0",
            "locked": false,
            "removable": false,
            "inserted": {
                "iops_rd": 0,
                "image": {
                    "virtual-size": 32212254720,
                    "filename": "/var/lib/libvirt/images/guest.qcow2",
                    "cluster-size": 65536,
                    "format": "qcow2",
                    "actual-size": 5328867328,
                    "format-specific": {
                        "type": "qcow2",
                        "data": {
                            "compat": "1.1",
                            "lazy-refcounts": false
                        }
                    },
                    "dirty-flag": false
                },
                "iops_wr": 0,
                "ro": false,
                "backing_file_depth": 0,
                "drv": "qcow2",
                "iops": 0,
                "bps_wr": 0,
                "encrypted": false,
                "bps": 0,
                "bps_rd": 0,
                "file": "/var/lib/libvirt/images/guest.qcow2",
                "encryption_key_missing": false
            },
            "type": "unknown"
        },
        {
            "io-status": "ok",
            "device": "drive-ide0-0-0",
            "locked": false,
            "removable": true,
            "tray_open": false,
            "type": "unknown"
        }
    ],
    "id": "libvirt-861"
}

But libvirt still thinks its using the old un-merged snapshot:

inaddy@(kvm02):~$ virsh domblklist --domain guest
Target Source
------------------------------------------------
vda /var/lib/libvirt/images/guest.1487158333
hda -

Will work on libvirt to fix this....

[Rafael David Tinoco (rafaeldtinoco)]

This upstream explains what I discovered in previous comments:

commit e6bcbcd32c70ae394e7b6a530012fe8b07a59b5d
Author: Eric Blake <email address hidden>
Date: Tue May 27 17:07:08 2014 -0600

    qemu: reject rather than hang on blockcommit of active layer

    qemu 2.0 added the ability to commit the active layer, but slightly
    differently than what libvirt had been anticipating in its
    implementation of the virDomainBlockCommit call. As a result, if
    you attempt to do a 'virsh blockcommit $dom vda', qemu gets into a
    state where it is waiting on libvirt to end the job, while libvirt
    is waiting on qemu to end the job, and the guest is effectively
    hung with regards to further commands for that block device.

    I have patches coming down the pipeline that will add full support
    for blockcommit of the active layer when coupled with qemu 2.0 or
    later; but they depend on Peter's improvements to block job handling
    and form enough of a new feature that they are not ready for
    inclusion in the 1.2.5 release. So for now, just reject the
    attempt, rather than letting the user get stuck. This is no worse
    than the behavior of qemu 1.7 rejecting the job.

    * src/qemu/qemu_driver.c (qemuDomainBlockCommit): Reject active
    commit.

    Signed-off-by: Eric Blake <email address hidden>

So basically Trusty libvirt shouldn't be supporting this feature as said in the beginning of the case. But this commit rejects the command saying its unsupported and has to be included in Trusty's QEMU.

[Rafael David Tinoco (rafaeldtinoco)]

I have read libvirt code, specifically:

tools/virsh-domain.c
src/libvirt-domain.c
src/qemu_driver.c

In the execution path of the block commit logic:

vshcommandrun() -> cmdblockcommit() -> virtdomainblockcommit() -> domain->driver(qemu)->domainblockcommit() -> qemudomainblockcommit()

As explained in libvirt git logs, QEMU changed its behavior in 2.0 for committing active layer. After the block layer was merged, QEMU has to wait for what to do: Keep using the actual block layer OR pivot into the merged one (invalidating all subsequent layers).

Best commit explaining this is:

https://gitorious.org/libvirt/libvirt/commit/b2980250631d6bf12a8e5b05f0bdfcb9456dea3c

Unfortunately this logic first appeared in 1.6 and was heavily changed until recent versions.

In order to have an usable (decently) block commit logic I would have to backport the majority of the following commits:

tools/virsh-domain.c

e557bd28a26 virsh: blockcommit: Support --bytes and scaled integers
ceec58ac9ea virsh: fix report of non-active commit completion
78d4c184403 virsh: Fix types for option bandwidth in block*
d8415b54695 virsh: Improve the job type reported of virsh cmd blockcommit
f182da20b09 virsh: expose new active commit controls
b2980250631 blockcommit: document semantics of committing active layer

src/qemu_driver.c

eae59247c59 qemu: Update state of block job to READY only if it actually is ready
f9ea3d60119 qemu: read backing chain names from qemu
ee744b5b387 qemu: block-commit: Mark disk in block jobs only on successful command
e1125cebfc0 qemu: forbid second blockcommit during active commit
3937ef9cf4f getstats: crawl backing chain for qemu
cfb16b8ed7b blockcommit: turn on active commit
232a31bea3b blockcommit: track job type in xml
c29b6529123 qemu: monitor: Add argument for specifying backing name for block commit
40ad7160a2e blockjob: turn on qemu capability bit for active commit
3e3c6ff10fd blockcommit: require base below top
278c51af3a7 blockcommit: update error messages related to block jobs
519181d932f blockcommit: fix regression with explicit top argument
b2980250631 blockcommit: document semantics of committing active layer

AND some of them are even adding arguments to cmdline that don't exist today (like commit: f182da20b09, specifically adding flags to differentiate the merge type - active with pivot).

Per SRU guidelines: https://wiki.ubuntu.com/StableReleaseUpdates, the SRU proposal wouldn't probably not be accepted by our core team. Instead, I'll do what was done upstream right after 1.2.0 release:

e6bcbcd32c70 qemu: reject rather than hang on blockcommit of active layer

Reject block commit for active layers. This means that the merge will either be offline OR by using other merging method: Block Streaming (comment #10).

I'll prepare the debdiff and propose the patch for the SRU.

[Rafael David Tinoco (rafaeldtinoco)]

For the fix to be cherry-picked cleanly I would need several patches:

commit 4f20226664b2d4d9e9496ec1e05c9280581a1ca0
Author: Eric Blake <email address hidden>
Date: Wed Mar 19 11:11:16 2014 -0600

    conf: prepare to track multiple host source files per <disk>

commit ca979b068061bd51696c1128f5aababc424d4293
Author: Jiri Denemark <email address hidden>
Date: Fri Apr 18 14:38:49 2014 +0200

    qemuDomainBlockCommit: Don't track top_canon path separately

Specially those 2 since they change the way qemuDomainBlockCommit organizes data for virDomainDiskDefPtr. Instead I made a small change to the original fix:

commit e6bcbcd32c70ae394e7b6a530012fe8b07a59b5d
Author: Eric Blake <email address hidden>
Date: Tue May 27 17:07:08 2014 -0600

    qemu: reject rather than hang on blockcommit of active layer

I'm attaching the debdiff for review (@cpaelzer) and will test if patch solves the problem likely tomorrow.

[Rafael David Tinoco (rafaeldtinoco)]

inaddy@(kvm02):~$ virsh start guest

inaddy@(kvm02):~$ virsh snapshot-create-as --domain guest --disk-only --atomic
Domain snapshot 1487745012 created

inaddy@(kvm02):~$ virsh domblklist --domain guest

inaddy@(kvm02):~$ virsh domblklist --domain guest
Target Source
------------------------------------------------
vda /var/lib/libvirt/images/guest.1487745012

inaddy@(kvm02):~$ virsh blockcommit --domain guest vda --base /var/lib/libvirt/images/guest.qcow2 --top /var/lib/libvt/images/guest.1487745012 --wait --verbose
Block Commit: [100 %]
<STUCK>

After the new package:

inaddy@(kvm02):~$ virsh blockcommit --domain guest vda --base /var/lib/libvirt/images/guest.qcow2 --top /var/lib/libvirtmages/guest.1487745012 --wait --verbose
error: Operation not supported: committing the active layer not supported yet

And not block jobs are running:

inaddy@(kvm02):~$ virsh qemu-monitor-command guest --pretty -- '{ "execute": "query-block-jobs" }'
{
    "return": [

    ],
    "id": "libvirt-9"
}

So patch works. Attaching it.

[Rafael David Tinoco (rafaeldtinoco)]

@cpaelzer, could u please review ? I think its all covered for the SRU.

[Christian Ehrhardt  (paelzer)]

Hi Rafael,

Not so important but FYI your link in the description is broken - it leads me to some weird catching site, not really an emulatortalk.info with some content.

I like your tests in #12 and #18, but can you think and compare several permutations of commits to non-active layers? Those should have worked before as far as I read the comments.

What would that be base->s1->s2->s3 and then committing s3 to s2 ?
What about inactive guests, would it work then?

You are certainly more into the topic, so I'd like to ask you to create these tests and check that these still behave as intended (or better).
This is meant to catch and issues where the change from:
 if (topSource == &disk->src) {
to
  if (top_meta == disk->backingChain) {
due to the backporting might cause issues.

Move these tests, together with just the commands of #12 into the SRU Teamplate in the description then please. To ease the SRU Teams work.

If you find any other invocation of block commit (e.g. to non active) that works today, list breaking that in regression potential please.

Other than that I really think the patch is fine, and what I asked for can/should be done as part of the verification in proposed IMHO - that said, sponsoring now (after a build test).

[Christian Ehrhardt  (paelzer)]

There was a short "WTF" moment as it seems https://launchpad.net/ubuntu/+source/libvirt/1.2.2-0ubuntu13.1.18 existed.
But it was only in proposed and got removed from there by inactivity to test the case.
There is also https://launchpad.net/ubuntu/+source/libvirt/1.2.2-0ubuntu13.1.19 which faced the same fate.
But well, that "only" means we have to make this .20 - since the others never got released (and so their content) they shall not be part of the changelog.

Ok, it is in the unapproved queue now.

@SRU Team:
I asked on ubuntu-devel but got no resonse, since .18 and .19 never existed for the users I skipped them in the changelog intentionally - please let me know if that should be handled otherwise (like keeping all old entries, and adding mass revert statements).

@Tinoco:
I also want to verify a few more things by testing once it is in proposed, so please document once you think you are fine testing the case (and the extra variants I asked for above) in proposed.
But please wait setting v-done before I also had time to complete my tests.

[Łukasz Zemczak (sil2100)]

Hello Stefan, or anyone else affected,

Accepted libvirt into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/libvirt/1.2.2-0ubuntu13.1.20 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.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-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-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!

[Łukasz Zemczak (sil2100)]

I accepted the package to the -proposed pocket. The .20 version seems ok, I also don't mind the removal of the changelog entries as those never really existed in the main archives (it would only introduce unwanted noise). But good thing that you mentioned the issues with .18 and .19 existing as otherwise I'd have to rise some eyebrows (since the debdiff was really strange because of that).

[Rafael David Tinoco (rafaeldtinoco)]

Answers to @cpaelzer:

>> Not so important but FYI your link in the description is broken - it leads me to some weird catching site, not really an emulatortalk.info with some content.

- Fixed

>> I like your tests in #12 and #18, but can you think and compare several permutations of commits to non-active layers? Those should have worked before as far as I read the comments.
>> What would that be base->s1->s2->s3 and then committing s3 to s2 ?
>> What about inactive guests, would it work then?

- I wasn't expecting SRU to be so fast =). I had in mind to continue tests today (specifically on multi-layer snapshots). Will do for the -proposed.

>> You are certainly more into the topic, so I'd like to ask you to create these tests and check that these still behave as intended (or better).

>> This is meant to catch and issues where the change from:
>> if (topSource == &disk->src) {
>> to
>> if (top_meta == disk->backingChain) {
>> due to the backporting might cause issues.

- Exactly! I just "confirmed" the active would get blocked but I still have to test all the other combinations. Will do.

>> Move these tests, together with just the commands of #12 into the SRU Teamplate in the description then please. To ease the SRU Teams work.

- Will do.

>> If you find any other invocation of block commit (e.g. to non active) that works today, list breaking that in regression potential please.

- Will do.

Other than that I really think the patch is fine, and what I asked for can/should be done as part of the verification in proposed IMHO - that said, sponsoring now (after a build test).

- Cool. I'll finish testing the -proposed version and will mark verification done after updating case desc.

Tks for the review! Will get back to you soon.

[Christian Ehrhardt  (paelzer)]

All my usual regression Tests completed on the proposed version 1.2.2-0ubuntu13.1.20.
Attaching the logs for later reference.

@Tinoco - when you completed all extra steps that I asked for and things are good there as well, feel free to mark as verification-done.

[Rafael David Tinoco (rafaeldtinoco)]

#
# VERIFICATION
#

inaddy@(kvm02):~$ dpkg -l | grep libvirt | awk '{print $2"\t"$3}'
libvirt-bin 1.2.2-0ubuntu13.1.20
libvirt0 1.2.2-0ubuntu13.1.20

inaddy@(kvm02):~$ : virsh domblklist --domain guest
Target Source
------------------------------------------------
vda /var/lib/libvirt/images/guest.1487745012

inaddy@(kvm02):~$ sudo qemu-img info --backing-chain /var/lib/libvirt/images/guest.1487745012
image: /var/lib/libvirt/images/guest.1487745012
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 74M
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.qcow2
backing file format: qcow2
Format specific information:
    compat: 1.1
    lazy refcounts: false

image: /var/lib/libvirt/images/guest.qcow2
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 5.0G
cluster_size: 65536
Format specific information:
    compat: 1.1
    lazy refcounts: false

inaddy@(kvm02):~$ virsh list
 Id Name State
----------------------------------------------------
 2 guest running

# First the original test:

inaddy@(kvm02):~$ virsh blockcommit --domain guest vda --base /var/lib/libvirt/images/guest.qcow2 --top /var/lib/libvirtmages/guest.1487745012 --wait --verbose
error: Operation not supported: committing the active layer not supported yet

[Rafael David Tinoco (rafaeldtinoco)]

# Creating a new snapshot:

inaddy@(kvm02):~$ virsh snapshot-create-as --domain guest --disk-only --atomic
Domain snapshot 1487875788 created

inaddy@(kvm02):~$ virsh domblklist --domain guest
Target Source
------------------------------------------------
vda /var/lib/libvirt/images/guest.1487875788

inaddy@(kvm02):~$ sudo qemu-img info --backing-chain /var/lib/libvirt/images/guest.1487875788
image: /var/lib/libvirt/images/guest.1487875788
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 1.4M
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.1487745012
backing file format: qcow2
Format specific information:
    compat: 1.1
    lazy refcounts: false

image: /var/lib/libvirt/images/guest.1487745012
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 76M
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.qcow2
backing file format: qcow2
Format specific information:
    compat: 1.1
    lazy refcounts: false

image: /var/lib/libvirt/images/guest.qcow2
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 5.0G
cluster_size: 65536
Format specific information:
    compat: 1.1
    lazy refcounts: false

# I have right now: <base=guest.qcow> <intermediate=guest.1487745012> <top=guest.1487875788>

[Rafael David Tinoco (rafaeldtinoco)]

# Testing to merge <intermediate> into <base> (both are offline):

inaddy@(kvm02):~$ virsh blockcommit --domain guest --path=/var/lib/libvirt/images/guest.1487875788 --top=/var/lib/librt/images/guest.1487745012 --base=/var/lib/libvirt/images/guest.qcow2 --wait --verbose
Block Commit: [100 %]
Commit complete

It works:

inaddy@(kvm02):~$ : virsh domblklist --domain guest
Target Source
------------------------------------------------
vda /var/lib/libvirt/images/guest.1487875788

inaddy@(kvm02):~$ sudo qemu-img info --backing-chain /var/lib/libvirt/images/guest.1487875788
image: /var/lib/libvirt/images/guest.1487875788
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 2.4M
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.qcow2
backing file format: qcow2
Format specific information:
    compat: 1.1
    lazy refcounts: false

image: /var/lib/libvirt/images/guest.qcow2
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 5.0G
cluster_size: 65536
Format specific information:
    compat: 1.1
    lazy refcounts: false

You can see that intermediate snapshot is gone. This means that blockcommit can do offline merge but not when using online disk (like we wanted to).

[Rafael David Tinoco (rafaeldtinoco)]

# Testing with more snapshots

inaddy@(kvm02):~$ sudo qemu-img info --backing-chain /var/lib/libvirt/images/guest.1487879587
image: /var/lib/libvirt/images/guest.1487879587
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 3.0M
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.1487877813
backing file format: qcow2
Format specific information:
    compat: 1.1
    lazy refcounts: false

image: /var/lib/libvirt/images/guest.1487877813
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 2.9M
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.1487875788
backing file format: qcow2
Format specific information:
    compat: 1.1
    lazy refcounts: false

image: /var/lib/libvirt/images/guest.1487875788
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 3.0M
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.qcow2
backing file format: qcow2
Format specific information:
    compat: 1.1
    lazy refcounts: false

image: /var/lib/libvirt/images/guest.qcow2
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 5.0G
cluster_size: 65536
Format specific information:
    compat: 1.1
    lazy refcounts: false

inaddy@(kvm02):~$ virsh blockcommit --domain=guest --path=/var/lib/libvirt/images/guest.1487879587 --base /var/lib/libvirt/images/guest.qcow2 --top /var/lib/libvirt/images/guest.1487875788 --wait --verbose
Block Commit: [100 %]
Commit complete

Guest is still running:

inaddy@(guest):~$ uptime
 19:59:05 up 1:09, 1 user, load average: 0.00, 0.01, 0.05

inaddy@(kvm02):~$ sudo qemu-img info --backing-chain /var/lib/libvirt/images/guest.1487879587
image: /var/lib/libvirt/images/guest.1487879587
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 3.5M
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.1487877813
backing file format: qcow2
Format specific information:
    compat: 1.1
    lazy refcounts: false

image: /var/lib/libvirt/images/guest.1487877813
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 2.9M
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.qcow2
backing file format: qcow2
Format specific information:
    compat: 1.1
    lazy refcounts: false

image: /var/lib/libvirt/images/guest.qcow2
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 5.0G
cluster_size: 65536
Format specific information:
    compat: 1.1
    lazy refcounts: false

Merge was done.

[Rafael David Tinoco (rafaeldtinoco)]

# Testing Bigger Merge

inaddy@(kvm02):~$ sudo qemu-img info --backing-chain /var/lib/libvirt/images/guest.1487880011
image: /var/lib/libvirt/images/guest.1487880011
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 196K
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.1487879972
backing file format: qcow2
Format specific information:
    compat: 1.1
    lazy refcounts: false

image: /var/lib/libvirt/images/guest.1487879972
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 466M
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.1487879587
backing file format: qcow2
Format specific information:
    compat: 1.1
    lazy refcounts: false

image: /var/lib/libvirt/images/guest.1487879587
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 3.6M
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.1487877813
backing file format: qcow2
Format specific information:
    compat: 1.1
    lazy refcounts: false

image: /var/lib/libvirt/images/guest.1487877813
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 2.9M
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.qcow2
backing file format: qcow2
Format specific information:
    compat: 1.1
    lazy refcounts: false

image: /var/lib/libvirt/images/guest.qcow2
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 5.0G
cluster_size: 65536
Format specific information:
    compat: 1.1
    lazy refcounts: false

# Writing to guest

inaddy@(guest):~$ dd if=/dev/zero of=./teste bs=1M count=1024
^C465+0 records in
465+0 records out
487587840 bytes (488 MB) copied, 8.74541 s, 55.8 MB/s

# Testing a bigger merge (several snapshots at once)

inaddy@(kvm02):~$ virsh blockcommit --domain=guest --path=/var/lib/libvirt/images/guest.1487880011 --base /var/lib/libvirt/images/guest.qcow2 --top /var/lib/libvirt/images/guest.1487879972 --wait --verbose
Block Commit: [100 %]
Commit complete

# There are only 2 snapshots at the end:

inaddy@(kvm02):~$ sudo qemu-img info --backing-chain /var/lib/libvirt/images/guest.1487880011
image: /var/lib/libvirt/images/guest.1487880011
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 1.1M
cluster_size: 65536
backing file: /var/lib/libvirt/images/guest.qcow2
backing file format: qcow2
Format specific information:
    compat: 1.1
    lazy refcounts: false

image: /var/lib/libvirt/images/guest.qcow2
file format: qcow2
virtual size: 30G (32212254720 bytes)
disk size: 5.4G
cluster_size: 65536
Format specific information:
    compat: 1.1
    lazy refcounts: false

[Rafael David Tinoco (rafaeldtinoco)]

Finishing Verification with @cpaelzer questions:

I like your tests in #12 and #18, but can you think and compare several permutations of commits to non-active layers? Those should have worked before as far as I read the comments.

What would that be base->s1->s2->s3 and then committing s3 to s2 ?

>> Demonstrated with 2, 3 and 4 snapshots ^^^^. Active can't be merged by all other layers can. At the end you will always have AT LEAST a base and the top layers (since the active can't be merged).

What about inactive guests, would it work then?

>> There is no way to merge volumes with inactive guests. It uses QEMU block jobs and it needs to be running.

Move these tests, together with just the commands of #12 into the SRU Teamplate in the description then please. To ease the SRU Teams work.

>> Doing right now.

If you find any other invocation of block commit (e.g. to non active) that works today, list breaking that in regression potential please.

>> All layers in RO mode (intermediate status). Will do.

Considering this verified. Thank you Christian.

[Brian Murray (brian-murray)]

Unsubscribing sponsors since everybody else forgot.

[Rafael David Tinoco (rafaeldtinoco)]

This case is long time waiting for SRU being published.

[Robie Basak (racb)]

20:54 <slashd> For SRU, LP #1317491 has landed in -proposed on "2017-02-22" and has been tag "verification done" the following day "2017-02-23", and the package is languishing in -proposed since then "libvirt | 1.2.2-0ubuntu13.1.20 | trusty-proposed". Could someone please have a look this bug and let tinoco and I (slashd) know the outcome ?

20:59 <nacc> slashd: http://people.canonical.com/~ubuntu-archive/pending-sru.html shows non-green bugs

...

21:01 <nacc> tinoco: there are three bugs listed

21:01 <tinoco> nacc: 2 wont fix and mine, green

21:01 <nacc> ah that might need a special sru reminder, i'm not sure

21:01 <nacc> rbasak: --^ ?

21:02 <rbasak> Yeah if pending-sru doesn't show all green, ~ubuntu-sru will generally ignore it and will need a special prod.

21:02 * rbasak looks

21:03 <nacc> won't fix seems like it should be struck-through or something on the pending-sru page (or there shouldn't be a task for trusty at all)

21:03 <nacc> it seems odd

21:03 <rbasak> Yeah

21:03 <rbasak> The report could do with that fixing.

21:09 <rbasak> Launchpad-Bugs-Fixed: 1317491

21:09 <rbasak> So I'm not sure why the other bugs appear anyway

[Robie Basak (racb)]

I've verified the diff between 1.17 and 1.20 fixes this bug only, and the verification in this bug looks good (and very comprehensive - thank you), so I conclude that this is fine to release.

[Launchpad Janitor (janitor)]

This bug was fixed in the package libvirt - 1.2.2-0ubuntu13.1.20

---------------
libvirt (1.2.2-0ubuntu13.1.20) trusty; urgency=medium

  [ Rafael David Tinoco ]
  * d/p/reject-blockcommit-of-active-layer.patch:
    Block commit code isn't ready for QEMU 2.0 and has to be blocked.
    This avoids virsh to hang forever on blackcommit jobs.
    (LP: #1317491)

 -- Christian Ehrhardt <email address hidden> Wed, 22 Feb 2017 09:44:02 +0100

[Robie Basak (racb)]

The verification of the Stable Release Update for libvirt has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. 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.

[Rafael David Tinoco (rafaeldtinoco)]

Thank you Robie.