Re: [PATCH 0/5] block/dirty-bitmap: check number and size constraints against queued bitmaps

[Vladimir Sementsov-Ogievskiy]

09.10.2019 23:44, John Snow wrote:
> 
> 
> On 10/9/19 2:57 PM, Eric Blake wrote:
>> On 6/6/19 1:41 PM, John Snow wrote:
>>> When adding new persistent dirty bitmaps, we only check constraints
>>> against currently stored bitmaps, and ignore the pending number and size
>>> of any bitmaps yet to be stored.
>>>
>>> Rework the "can_store" and "remove" interface to explicit "add" and
>>> "remove",
>>> and begin keeping track of the queued burden when adding new bitmaps.
>>>
>>> Reported-by: aihua liang 
>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1712636
>>>
>>> John Snow (5):
>>>     block/qcow2-bitmap: allow bitmap_list_load to return an error code
>>>     block/dirty-bitmap: Refactor bdrv_can_store_new_bitmap
>>>     block/dirty-bitmap: rework bdrv_remove_persistent_dirty_bitmap
>>>     block/qcow2-bitmap: Count queued bitmaps towards nb_bitmaps
>>>     block/qcow2-bitmap: Count queued bitmaps towards directory_size
>>
>> Is this series worth reviving as a v2, as it solves a corner-case bug?
>>
>>
> 
> Yes, but IIRC there were some disagreements about the methodology for
> the fix, but can't recall exactly what right now.
> 
> The way I remember it is that I wanted to make our qcow2 functions more
> like "do_store_bitmap" and "do_remove_bitmap" for direct addition and
> removal as I find that conceptual model 'simpler'.
> 
> (I think it had something to do with additional complexities in the
> different contexts that list_load is used in for when and if it performs
> certain consistency checks...?)
> 
> I think Vladimir wanted to use a pending-flush style cache to check
> requirements against instead of actual writes.
> 

Actually, here is my counter-proposal:

[PATCH] qcow2-bitmaps: fix qcow2_can_store_new_dirty_bitmap
 <20190607184802.100945-1-vsement...@virtuozzo.com>
 https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg01696.html


-- 
Best regards,
Vladimir

RE: [PATCH v6 4/4] colo: Update Documentation for continuous replication

[Zhang, Chen]

> -Original Message-
> From: Lukas Straub 
> Sent: Wednesday, October 9, 2019 11:17 PM
> To: Zhang, Chen 
> Cc: qemu-devel ; Jason Wang
> ; Wen Congyang ;
> Xie Changlong ; Kevin Wolf
> ; Max Reitz ; qemu-block
> 
> Subject: Re: [PATCH v6 4/4] colo: Update Documentation for continuous
> replication
> 
> On Wed, 9 Oct 2019 08:36:52 +
> "Zhang, Chen"  wrote:
> 
> > > -Original Message-
> > > From: Lukas Straub 
> > > Sent: Saturday, October 5, 2019 9:06 PM
> > > To: qemu-devel 
> > > Cc: Zhang, Chen ; Jason Wang
> > > ; Wen Congyang
> ; Xie
> > > Changlong ; Kevin Wolf
> ;
> > > Max Reitz ; qemu-block  bl...@nongnu.org>
> > > Subject: [PATCH v6 4/4] colo: Update Documentation for continuous
> > > replication
> > >
> > > Document the qemu command-line and qmp commands for continuous
> > > replication
> > >
> > > Signed-off-by: Lukas Straub 
> > > ---
> > >  docs/COLO-FT.txt   | 213 +++--
> > >  docs/block-replication.txt |  28 +++--
> > >  2 files changed, 174 insertions(+), 67 deletions(-)
> > >
> > > diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index
> > > ad24680d13..bc1a0ccb99 100644
> > > --- a/docs/COLO-FT.txt
> > > +++ b/docs/COLO-FT.txt
> > > @@ -145,35 +145,65 @@ The diagram just shows the main qmp
> command,
> > > you can get the detail  in test procedure.
> > >
> > > ...
> > >
> > > +Note: Here we are running both instances on the same Host for
> > > +testing, change the IP Addresses if you want to run it on two
> > > +Hosts. Initally
> > > +127.0.0.1 is the Primary Host and 127.0.0.2 is the Secondary Host.
> > > +
> > > +== Startup qemu ==
> > > +1. Primary:
> > > +Note: Initally, $imagefolder/primary.qcow2 needs to be copied to all
> Hosts.
> > > +# imagefolder="/mnt/vms/colo-test-primary"
> > > +
> > > +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -
> smp
> > > 1 -qmp stdio \
> > > +   -device piix3-usb-uhci -device usb-tablet -name primary \
> > > +   -netdev
> > > + tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper
> > > \
> > > +   -device rtl8139,id=e0,netdev=hn0 \
> > > +   -chardev socket,id=mirror0,host=0.0.0.0,port=9003,server,nowait \
> > > +   -chardev socket,id=compare1,host=0.0.0.0,port=9004,server,wait \
> >
> > We should change the host=127.0.0.1 consistent with the expression below.
> 
> Hi,
> This (and the IPs below in the QMP commands) needs to be this way,
> because it's a listening port and with 127.0.0.1 it would only listen on the
> loopback ip and wouldn't be reachable from another node for example. With
> 0.0.0.0 it will listen on all Interfaces.

Yes, I know.  For this command demo, maybe use 192.168.0.1/192.168.0.2 are more 
clear.

> 
> > > +   -chardev socket,id=compare0,host=127.0.0.1,port=9001,server,nowait
> \
> > > +   -chardev socket,id=compare0-0,host=127.0.0.1,port=9001 \
> > > +   -chardev
> > > + socket,id=compare_out,host=127.0.0.1,port=9005,server,nowait
> > > \
> > > +   -chardev socket,id=compare_out0,host=127.0.0.1,port=9005 \
> > > +   -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0 \
> > > +   -object filter-
> > > redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out \
> > > +   -object filter-
> > > redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0 \
> > > +   -object iothread,id=iothread1 \
> > > +   -object
> > > +colo-compare,id=comp0,primary_in=compare0-
> > > 0,secondary_in=compare1,\
> > > +outdev=compare_out0,iothread=iothread1 \
> > > +   -drive
> > > +if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold
> > > +=1,\
> > > +children.0.file.filename=$imagefolder/primary.qcow2,children.0.driv
> > > +er=q
> > > +cow2 -S
> > > +
> > > +2. Secondary:
> > > +# imagefolder="/mnt/vms/colo-test-secondary"
> > > +# primary_ip=127.0.0.1
> > > +
> > > +# qemu-img create -f qcow2 $imagefolder/secondary-active.qcow2 10G
> > > +
> > > +# qemu-img create -f qcow2 $imagefolder/secondary-hidden.qcow2
> 10G
> > > +
> >
> > The active disk and hidden disk just need create one time, we can note that
> here.
> 
> Ok, I will Note that. But I will wait until the block changes are reviewed
> before sending the next version.

That's fine for me.

Thanks
Zhang Chen

> 
> Regards,
> Lukas Straub
> 
> > > +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -
> smp
> > > 1 -qmp stdio \
> > > +   -device piix3-usb-uhci -device usb-tablet -name secondary \
> > > +   -netdev
> > > + tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper
> > > \
> > > +   -device rtl8139,id=e0,netdev=hn0 \
> > > +   -chardev socket,id=red0,host=$primary_ip,port=9003,reconnect=1 \
> > > +   -chardev socket,id=red1,host=$primary_ip,port=9004,reconnect=1 \
> > > +   -object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0 \
> > > +   -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1 \
> > > +   -object filter-rewriter,id=rew0,netdev=hn0,queue=all \
> > > +   -drive
> > > 

[PATCH 0/2] qcow2: Limit total allocation range to INT_MAX

[Max Reitz]

Hi,

While looking for why handle_alloc_space() seems to cause issues on
ppc64le+XFS (performance degradation and data corruption), I spotted
this other issue.  It isn’t as bad, but still needs fixing.

See patch 1 for what is fixed and patch 2 for what breaks otherwise.


Max Reitz (2):
  qcow2: Limit total allocation range to INT_MAX
  iotests: Test large write request to qcow2 file

 block/qcow2-cluster.c  |  5 ++-
 tests/qemu-iotests/268 | 83 ++
 tests/qemu-iotests/268.out |  9 +
 tests/qemu-iotests/group   |  1 +
 4 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100755 tests/qemu-iotests/268
 create mode 100644 tests/qemu-iotests/268.out

-- 
2.21.0

[PATCH 2/2] iotests: Test large write request to qcow2 file

[Max Reitz]

Without HEAD^, the following happens when you attempt a large write
request to a qcow2 file such that the number of bytes covered by all
clusters involved in a single allocation will exceed INT_MAX:

(A) handle_alloc_space() decides to fill the whole area with zeroes and
fails because bdrv_co_pwrite_zeroes() fails (the request is too
large).

(B) If handle_alloc_space() does not do anything, but merge_cow()
decides that the requests can be merged, it will create a too long
IOV that later cannot be written.

(C) Otherwise, all parts will be written separately, so those requests
will work.

In either B or C, though, qcow2_alloc_cluster_link_l2() will have an
overflow: We use an int (i) to iterate over nb_clusters, and then
calculate the L2 entry based on "i << s->cluster_bits" -- which will
overflow if the range covers more than INT_MAX bytes.  This then leads
to image corruption because the L2 entry will be wrong (it will be
recognized as a compressed cluster).

Even if that were not the case, the .cow_end area would be empty
(because handle_alloc() will cap avail_bytes and nb_bytes at INT_MAX, so
their difference (which is the .cow_end size) will be 0).

So this test checks that on such large requests, the image will not be
corrupted.  Unfortunately, we cannot check whether COW will be handled
correctly, because that data is discarded when it is written to null-co
(but we have to use null-co, because writing 2 GB of data in a test is
not quite reasonable).

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/268 | 83 ++
 tests/qemu-iotests/268.out |  9 +
 tests/qemu-iotests/group   |  1 +
 3 files changed, 93 insertions(+)
 create mode 100755 tests/qemu-iotests/268
 create mode 100644 tests/qemu-iotests/268.out

diff --git a/tests/qemu-iotests/268 b/tests/qemu-iotests/268
new file mode 100755
index 00..b9a12b908c
--- /dev/null
+++ b/tests/qemu-iotests/268
@@ -0,0 +1,83 @@
+#!/usr/bin/env bash
+#
+# Test large write to a qcow2 image
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+seq=$(basename "$0")
+echo "QA output created by $seq"
+
+status=1   # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# This is a qcow2 regression test
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+# We use our own external data file and our own cluster size, and we
+# require v3 images
+_unsupported_imgopts data_file cluster_size 'compat=0.10'
+
+
+# We need a backing file so that handle_alloc_space() will not do
+# anything.  (If it were to do anything, it would simply fail its
+# write-zeroes request because the request range is too large.)
+TEST_IMG="$TEST_IMG.base" _make_test_img 4G
+$QEMU_IO -c 'write 0 512' "$TEST_IMG.base" | _filter_qemu_io
+
+# (Use .orig because _cleanup_test_img will remove that file)
+# We need a large cluster size, see below for why (above the $QEMU_IO
+# invocation)
+_make_test_img -o cluster_size=2M,data_file="$TEST_IMG.orig" \
+-b "$TEST_IMG.base" 4G
+
+# We want a null-co as the data file, because it allows us to quickly
+# "write" 2G of data without using any space.
+# (qemu-img create does not like it, though, because null-co does not
+# support image creation.)
+$QEMU_IMG amend -o data_file="json:{'driver':'null-co',,'size':'4294967296'}" \
+"$TEST_IMG"
+
+# This gives us a range of:
+#   2^31 - 512 + 768 - 1 = 2^31 + 255 > 2^31
+# until the beginning of the end COW block.  (The total allocation
+# size depends on the cluster size, but all that is important is that
+# it exceeds INT_MAX.)
+#
+# 2^31 - 512 is the maximum request size.  We want this to result in a
+# single allocation, and because the qcow2 driver splits allocations
+# on L2 boundaries, we need large L2 tables; hence the cluster size of
+# 2 MB.  (Anything from 256 kB should work, though, because then one L2
+# table covers 8 GB.)
+$QEMU_IO -c "write 768 $((2 ** 31 - 512))" "$TEST_IMG" | _filter_qemu_io
+
+_check_test_img
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/268.out b/tests/qemu-iotests/268.out
new file mode 100644
index 00..35d4f9e3e9
--- /dev/null
+++ 

[PATCH 1/2] qcow2: Limit total allocation range to INT_MAX

[Max Reitz]

When the COW areas are included, the size of an allocation can exceed
INT_MAX.  This is kind of limited by handle_alloc() in that it already
caps avail_bytes at INT_MAX, but the number of clusters still reflects
the original length.

This can have all sorts of effects, ranging from the storage layer write
call failing to image corruption.  (If there were no image corruption,
then I suppose there would be data loss because the .cow_end area is
forced to be empty, even though there might be something we need to
COW.)

Fix all of it by limiting nb_clusters so the equivalent number of bytes
will not exceed INT_MAX.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 8d5fa1539c..8982b7b762 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1330,6 +1330,9 @@ static int handle_alloc(BlockDriverState *bs, uint64_t 
guest_offset,
 nb_clusters = MIN(nb_clusters, s->l2_slice_size - l2_index);
 assert(nb_clusters <= INT_MAX);
 
+/* Limit total allocation byte count to INT_MAX */
+nb_clusters = MIN(nb_clusters, INT_MAX >> s->cluster_bits);
+
 /* Find L2 entry for the first involved cluster */
 ret = get_cluster_table(bs, guest_offset, _slice, _index);
 if (ret < 0) {
@@ -1412,7 +1415,7 @@ static int handle_alloc(BlockDriverState *bs, uint64_t 
guest_offset,
  * request actually writes to (excluding COW at the end)
  */
 uint64_t requested_bytes = *bytes + offset_into_cluster(s, guest_offset);
-int avail_bytes = MIN(INT_MAX, nb_clusters << s->cluster_bits);
+int avail_bytes = nb_clusters << s->cluster_bits;
 int nb_bytes = MIN(requested_bytes, avail_bytes);
 QCowL2Meta *old_m = *m;
 
-- 
2.21.0

Re: [PATCH v2 1/1] IDE: deprecate ide-drive

[Thomas Huth]

On 10/10/2019 00.43, John Snow wrote:
> It's an old compatibility shim that just delegates to ide-cd or ide-hd.
> I'd like to refactor these some day, and getting rid of the super-object
> will make that easier.
> 
> Either way, we don't need this.
> 
> Libvirt-checked-by: Peter Krempa 
> Signed-off-by: John Snow 
> ---
>  qemu-deprecated.texi  | 5 +
>  hw/ide/qdev.c | 3 +++
>  tests/qemu-iotests/051.pc.out | 6 --
>  3 files changed, 12 insertions(+), 2 deletions(-)

Reviewed-by: Thomas Huth 

Re: [PATCH v2 1/1] IDE: deprecate ide-drive

[Markus Armbruster]

John Snow  writes:

> It's an old compatibility shim that just delegates to ide-cd or ide-hd.
> I'd like to refactor these some day, and getting rid of the super-object
> will make that easier.
>
> Either way, we don't need this.
>
> Libvirt-checked-by: Peter Krempa 
> Signed-off-by: John Snow 

Reviewed-by: Markus Armbruster 

[PULL 07/36] block/backup: fix backup_cow_with_offload for last cluster

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

We shouldn't try to copy bytes beyond EOF. Fix it.

Fixes: 9ded4a0114968e
Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
Message-id: 20190920142056.12778-3-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block/backup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/backup.c b/block/backup.c
index db20249063..99177f03f8 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -161,7 +161,7 @@ static int coroutine_fn 
backup_cow_with_offload(BackupBlockJob *job,
 
 assert(QEMU_IS_ALIGNED(job->copy_range_size, job->cluster_size));
 assert(QEMU_IS_ALIGNED(start, job->cluster_size));
-nbytes = MIN(job->copy_range_size, end - start);
+nbytes = MIN(job->copy_range_size, MIN(end, job->len) - start);
 nr_clusters = DIV_ROUND_UP(nbytes, job->cluster_size);
 bdrv_reset_dirty_bitmap(job->copy_bitmap, start,
 job->cluster_size * nr_clusters);
-- 
2.21.0

[PULL 16/36] iotests: 257: drop device_add

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

SCSI devices are unused in test, drop them.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Message-id: 20190920142056.12778-12-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/257 |  8 ---
 tests/qemu-iotests/257.out | 44 --
 2 files changed, 52 deletions(-)

diff --git a/tests/qemu-iotests/257 b/tests/qemu-iotests/257
index 5d77202157..de8b45f094 100755
--- a/tests/qemu-iotests/257
+++ b/tests/qemu-iotests/257
@@ -325,12 +325,6 @@ def test_bitmap_sync(bsync_mode, msync_mode='bitmap', 
failure=None):
node_name=drive0.node,
driver=drive0.fmt,
file=file_config)
-# Use share-rw to allow writes directly to the node;
-# The anonymous block-backend for this configuration prevents us
-# from using HMP's qemu-io commands to address the device.
-vm.qmp_log("device_add", id='device0',
-   drive=drive0.node, driver="scsi-hd",
-   share_rw=True)
 log('')
 
 # 0 - Writes and Reference Backup
@@ -467,8 +461,6 @@ def test_backup_api():
node_name=drive0.node,
driver=drive0.fmt,
file=file_config)
-vm.qmp_log("device_add", id='device0',
-   drive=drive0.node, driver="scsi-hd")
 log('')
 
 target0 = Drive(backup_path, vm=vm)
diff --git a/tests/qemu-iotests/257.out b/tests/qemu-iotests/257.out
index c9b4b68232..ec7e25877b 100644
--- a/tests/qemu-iotests/257.out
+++ b/tests/qemu-iotests/257.out
@@ -5,8 +5,6 @@
 
 {"execute": "blockdev-add", "arguments": {"driver": "qcow2", "file": 
{"driver": "file", "filename": "TEST_DIR/PID-img"}, "node-name": "drive0"}}
 {"return": {}}
-{"execute": "device_add", "arguments": {"drive": "drive0", "driver": 
"scsi-hd", "id": "device0", "share-rw": true}}
-{"return": {}}
 
 --- Write #0 ---
 
@@ -267,8 +265,6 @@ qemu_img compare "TEST_DIR/PID-img" "TEST_DIR/PID-fbackup2" 
==> Identical, OK!
 
 {"execute": "blockdev-add", "arguments": {"driver": "qcow2", "file": 
{"driver": "blkdebug", "image": {"driver": "file", "filename": 
"TEST_DIR/PID-img"}, "inject-error": [{"errno": 5, "event": "read_aio", 
"immediately": false, "once": true, "state": 3}], "set-state": [{"event": 
"flush_to_disk", "new-state": 2, "state": 1}, {"event": "read_aio", 
"new-state": 3, "state": 2}]}, "node-name": "drive0"}}
 {"return": {}}
-{"execute": "device_add", "arguments": {"drive": "drive0", "driver": 
"scsi-hd", "id": "device0", "share-rw": true}}
-{"return": {}}
 
 --- Write #0 ---
 
@@ -480,8 +476,6 @@ qemu_img compare "TEST_DIR/PID-img" "TEST_DIR/PID-fbackup2" 
==> Identical, OK!
 
 {"execute": "blockdev-add", "arguments": {"driver": "qcow2", "file": 
{"driver": "file", "filename": "TEST_DIR/PID-img"}, "node-name": "drive0"}}
 {"return": {}}
-{"execute": "device_add", "arguments": {"drive": "drive0", "driver": 
"scsi-hd", "id": "device0", "share-rw": true}}
-{"return": {}}
 
 --- Write #0 ---
 
@@ -742,8 +736,6 @@ qemu_img compare "TEST_DIR/PID-img" "TEST_DIR/PID-fbackup2" 
==> Identical, OK!
 
 {"execute": "blockdev-add", "arguments": {"driver": "qcow2", "file": 
{"driver": "file", "filename": "TEST_DIR/PID-img"}, "node-name": "drive0"}}
 {"return": {}}
-{"execute": "device_add", "arguments": {"drive": "drive0", "driver": 
"scsi-hd", "id": "device0", "share-rw": true}}
-{"return": {}}
 
 --- Write #0 ---
 
@@ -1004,8 +996,6 @@ qemu_img compare "TEST_DIR/PID-img" 
"TEST_DIR/PID-fbackup2" ==> Identical, OK!
 
 {"execute": "blockdev-add", "arguments": {"driver": "qcow2", "file": 
{"driver": "blkdebug", "image": {"driver": "file", "filename": 
"TEST_DIR/PID-img"}, "inject-error": [{"errno": 5, "event": "read_aio", 
"immediately": false, "once": true, "state": 3}], "set-state": [{"event": 
"flush_to_disk", "new-state": 2, "state": 1}, {"event": "read_aio", 
"new-state": 3, "state": 2}]}, "node-name": "drive0"}}
 {"return": {}}
-{"execute": "device_add", "arguments": {"drive": "drive0", "driver": 
"scsi-hd", "id": "device0", "share-rw": true}}
-{"return": {}}
 
 --- Write #0 ---
 
@@ -1217,8 +1207,6 @@ qemu_img compare "TEST_DIR/PID-img" 
"TEST_DIR/PID-fbackup2" ==> Identical, OK!
 
 {"execute": "blockdev-add", "arguments": {"driver": "qcow2", "file": 
{"driver": "file", "filename": "TEST_DIR/PID-img"}, "node-name": "drive0"}}
 {"return": {}}
-{"execute": "device_add", "arguments": {"drive": "drive0", "driver": 
"scsi-hd", "id": "device0", "share-rw": true}}
-{"return": {}}
 
 --- Write #0 ---
 
@@ -1479,8 +1467,6 @@ qemu_img compare "TEST_DIR/PID-img" 
"TEST_DIR/PID-fbackup2" ==> Identical, OK!
 
 {"execute": "blockdev-add", "arguments": {"driver": "qcow2", "file": 
{"driver": "file", "filename": "TEST_DIR/PID-img"}, "node-name": "drive0"}}
 {"return": {}}
-{"execute": "device_add", "arguments": {"drive": "drive0", "driver": 
"scsi-hd", "id": "device0", 

[PULL 06/36] block/backup: fix max_transfer handling for copy_range

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Of course, QEMU_ALIGN_UP is a typo, it should be QEMU_ALIGN_DOWN, as we
are trying to find aligned size which satisfy both source and target.
Also, don't ignore too small max_transfer. In this case seems safer to
disable copy_range.

Fixes: 9ded4a0114968e
Signed-off-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20190920142056.12778-2-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block/backup.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 763f0d7ff6..db20249063 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -741,12 +741,19 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 job->cluster_size = cluster_size;
 job->copy_bitmap = copy_bitmap;
 copy_bitmap = NULL;
-job->use_copy_range = !compress; /* compression isn't supported for it */
 job->copy_range_size = MIN_NON_ZERO(blk_get_max_transfer(job->common.blk),
 blk_get_max_transfer(job->target));
-job->copy_range_size = MAX(job->cluster_size,
-   QEMU_ALIGN_UP(job->copy_range_size,
- job->cluster_size));
+job->copy_range_size = QEMU_ALIGN_DOWN(job->copy_range_size,
+   job->cluster_size);
+/*
+ * Set use_copy_range, consider the following:
+ * 1. Compression is not supported for copy_range.
+ * 2. copy_range does not respect max_transfer (it's a TODO), so we factor
+ *that in here. If max_transfer is smaller than the job->cluster_size,
+ *we do not use copy_range (in that case it's zero after aligning down
+ *above).
+ */
+job->use_copy_range = !compress && job->copy_range_size > 0;
 
 /* Required permissions are already taken with target's blk_new() */
 block_job_add_bdrv(>common, "target", target, 0, BLK_PERM_ALL,
-- 
2.21.0

Re: [PATCH v2 1/1] IDE: deprecate ide-drive

[Philippe Mathieu-Daudé]

On 10/10/19 1:26 PM, Peter Krempa wrote:

On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote:

On 10/10/19 12:43 AM, John Snow wrote:

It's an old compatibility shim that just delegates to ide-cd or ide-hd.
I'd like to refactor these some day, and getting rid of the super-object
will make that easier.

Either way, we don't need this.

Libvirt-checked-by: Peter Krempa 


Peter made a comment regarding Laszlo's Regression-tested-by tag:

   [...] nobody else is using
   this convention (there are exactly 0 instances of
   "Regression-tested-by" in the project git log as far as
   I can see), and so in practice people reading the commits
   won't really know what you meant by it. Everybody else
   on the project uses "Tested-by" to mean either of the
   two cases you describe above, without distinction...

It probably applies to 'Libvirt-checked-by' too.


I certainly didn't test it. So feel free to drop that line altogether.


But you reviewed it, can we use your 'Reviewed-by' instead?

[PULL 28/36] iotests: Use stat -c %b in 125

[Max Reitz]

125 should not use qemu-img to get the on-disk image size, because that
reports it in a human-readable format that is useless to us.  Just use
stat instead (like we do to get the image file length).

Signed-off-by: Max Reitz 
Message-id: 20190925183231.11196-4-mre...@redhat.com
Reviewed-by: Eric Blake 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/125 | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/125 b/tests/qemu-iotests/125
index 0ef51f1e21..4e31aa4e5f 100755
--- a/tests/qemu-iotests/125
+++ b/tests/qemu-iotests/125
@@ -34,8 +34,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 get_image_size_on_host()
 {
-$QEMU_IMG info -f "$IMGFMT" "$TEST_IMG" | grep "disk size" \
-| sed -e 's/^[^0-9]*\([0-9]\+\).*$/\1/'
+echo $(($(stat -c '%b * %B' "$TEST_IMG_FILE")))
 }
 
 # get standard environment and filters
-- 
2.21.0

[PULL 36/36] iotests/162: Fix for newer Linux 5.3+

[Max Reitz]

Linux 5.3 has made 0.0.0.0/8 a working IPv4 subnet.  As such, "42" is
now a valid host, and the connection to it will (hopefully) time out
over a long period rather than quickly return with EINVAL.

So let us use a negative integer for testing that NBD will not crash
when it receives integer hosts.  This way, the connection will again
fail quickly and reliably.

Signed-off-by: Max Reitz 
Message-id: 20191002174052.5773-1-mre...@redhat.com
Reviewed-by: Eric Blake 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/162 | 2 +-
 tests/qemu-iotests/162.out | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/162 b/tests/qemu-iotests/162
index 2d719afbed..c0053ed975 100755
--- a/tests/qemu-iotests/162
+++ b/tests/qemu-iotests/162
@@ -46,7 +46,7 @@ echo '=== NBD ==='
 # NBD expects all of its arguments to be strings
 
 # So this should not crash
-$QEMU_IMG info 'json:{"driver": "nbd", "host": 42}'
+$QEMU_IMG info 'json:{"driver": "nbd", "host": -1}'
 
 # And this should not treat @port as if it had not been specified
 # (We need to set up a server here, because the error message for "Connection
diff --git a/tests/qemu-iotests/162.out b/tests/qemu-iotests/162.out
index 3c5be2c569..5a00d36d17 100644
--- a/tests/qemu-iotests/162.out
+++ b/tests/qemu-iotests/162.out
@@ -1,7 +1,7 @@
 QA output created by 162
 
 === NBD ===
-qemu-img: Could not open 'json:{"driver": "nbd", "host": 42}': Failed to 
connect socket: Invalid argument
+qemu-img: Could not open 'json:{"driver": "nbd", "host": -1}': address 
resolution failed for -1:10809: Name or service not known
 image: nbd://localhost:PORT
 image: nbd+unix://?socket=42
 
-- 
2.21.0

[PULL 29/36] block/backup: move in-flight requests handling from backup to block-copy

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Move synchronization mechanism to block-copy, to be able to use one
block-copy instance from backup job and backup-top filter in parallel.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20191001131409.14202-2-vsement...@virtuozzo.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 include/block/block-copy.h |  8 ++
 block/backup.c | 52 --
 block/block-copy.c | 43 +++
 3 files changed, 51 insertions(+), 52 deletions(-)

diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index 54f90d0c9a..962f91056a 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -17,6 +17,13 @@
 
 #include "block/block.h"
 
+typedef struct BlockCopyInFlightReq {
+int64_t start_byte;
+int64_t end_byte;
+QLIST_ENTRY(BlockCopyInFlightReq) list;
+CoQueue wait_queue; /* coroutines blocked on this request */
+} BlockCopyInFlightReq;
+
 typedef void (*ProgressBytesCallbackFunc)(int64_t bytes, void *opaque);
 typedef void (*ProgressResetCallbackFunc)(void *opaque);
 typedef struct BlockCopyState {
@@ -27,6 +34,7 @@ typedef struct BlockCopyState {
 bool use_copy_range;
 int64_t copy_range_size;
 uint64_t len;
+QLIST_HEAD(, BlockCopyInFlightReq) inflight_reqs;
 
 BdrvRequestFlags write_flags;
 
diff --git a/block/backup.c b/block/backup.c
index 4613b8c88d..d918836f1d 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -29,13 +29,6 @@
 
 #define BACKUP_CLUSTER_SIZE_DEFAULT (1 << 16)
 
-typedef struct CowRequest {
-int64_t start_byte;
-int64_t end_byte;
-QLIST_ENTRY(CowRequest) list;
-CoQueue wait_queue; /* coroutines blocked on this request */
-} CowRequest;
-
 typedef struct BackupBlockJob {
 BlockJob common;
 BlockDriverState *source_bs;
@@ -51,50 +44,12 @@ typedef struct BackupBlockJob {
 uint64_t bytes_read;
 int64_t cluster_size;
 NotifierWithReturn before_write;
-QLIST_HEAD(, CowRequest) inflight_reqs;
 
 BlockCopyState *bcs;
 } BackupBlockJob;
 
 static const BlockJobDriver backup_job_driver;
 
-/* See if in-flight requests overlap and wait for them to complete */
-static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job,
-   int64_t start,
-   int64_t end)
-{
-CowRequest *req;
-bool retry;
-
-do {
-retry = false;
-QLIST_FOREACH(req, >inflight_reqs, list) {
-if (end > req->start_byte && start < req->end_byte) {
-qemu_co_queue_wait(>wait_queue, NULL);
-retry = true;
-break;
-}
-}
-} while (retry);
-}
-
-/* Keep track of an in-flight request */
-static void cow_request_begin(CowRequest *req, BackupBlockJob *job,
-  int64_t start, int64_t end)
-{
-req->start_byte = start;
-req->end_byte = end;
-qemu_co_queue_init(>wait_queue);
-QLIST_INSERT_HEAD(>inflight_reqs, req, list);
-}
-
-/* Forget about a completed request */
-static void cow_request_end(CowRequest *req)
-{
-QLIST_REMOVE(req, list);
-qemu_co_queue_restart_all(>wait_queue);
-}
-
 static void backup_progress_bytes_callback(int64_t bytes, void *opaque)
 {
 BackupBlockJob *s = opaque;
@@ -116,7 +71,6 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
   bool *error_is_read,
   bool is_write_notifier)
 {
-CowRequest cow_request;
 int ret = 0;
 int64_t start, end; /* bytes */
 
@@ -127,14 +81,9 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
 
 trace_backup_do_cow_enter(job, start, offset, bytes);
 
-wait_for_overlapping_requests(job, start, end);
-cow_request_begin(_request, job, start, end);
-
 ret = block_copy(job->bcs, start, end - start, error_is_read,
  is_write_notifier);
 
-cow_request_end(_request);
-
 trace_backup_do_cow_return(job, offset, bytes, ret);
 
 qemu_co_rwlock_unlock(>flush_rwlock);
@@ -316,7 +265,6 @@ static int coroutine_fn backup_run(Job *job, Error **errp)
 BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
 int ret = 0;
 
-QLIST_INIT(>inflight_reqs);
 qemu_co_rwlock_init(>flush_rwlock);
 
 backup_init_copy_bitmap(s);
diff --git a/block/block-copy.c b/block/block-copy.c
index 3fc9152853..61e5ea5f46 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -19,6 +19,41 @@
 #include "block/block-copy.h"
 #include "sysemu/block-backend.h"
 
+static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s,
+   int64_t start,
+   int64_t end)
+{
+BlockCopyInFlightReq *req;
+bool waited;
+
+do {
+waited = 

[PULL 11/36] block/backup: fix block-comment style

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

We need to fix comment style around block-copy functions before further
moving them to separate file to satisfy checkpatch. But do more: fix
all comments style. Also, seems like doubled first asterisk is not
forbidden, but drop it too for consistency.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Message-id: 20190920142056.12778-7-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block/backup.c | 32 +---
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 5dda1673ca..f5125984db 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -223,8 +223,10 @@ fail:
 return NULL;
 }
 
-/* Copy range to target with a bounce buffer and return the bytes copied. If
- * error occurred, return a negative error number */
+/*
+ * Copy range to target with a bounce buffer and return the bytes copied. If
+ * error occurred, return a negative error number
+ */
 static int coroutine_fn block_copy_with_bounce_buffer(BlockCopyState *s,
   int64_t start,
   int64_t end,
@@ -269,8 +271,10 @@ fail:
 
 }
 
-/* Copy range to target and return the bytes copied. If error occurred, return 
a
- * negative error number. */
+/*
+ * Copy range to target and return the bytes copied. If error occurred, return 
a
+ * negative error number.
+ */
 static int coroutine_fn block_copy_with_offload(BlockCopyState *s,
 int64_t start,
 int64_t end,
@@ -341,7 +345,7 @@ static int block_copy_is_cluster_allocated(BlockCopyState 
*s, int64_t offset,
 }
 }
 
-/**
+/*
  * Reset bits in copy_bitmap starting at offset if they represent unallocated
  * data in the image. May reset subsequent contiguous bits.
  * @return 0 when the cluster at @offset was unallocated,
@@ -592,8 +596,10 @@ static bool coroutine_fn yield_and_check(BackupBlockJob 
*job)
 return true;
 }
 
-/* We need to yield even for delay_ns = 0 so that bdrv_drain_all() can
- * return. Without a yield, the VM would not reboot. */
+/*
+ * We need to yield even for delay_ns = 0 so that bdrv_drain_all() can
+ * return. Without a yield, the VM would not reboot.
+ */
 delay_ns = block_job_ratelimit_get_delay(>common, job->bytes_read);
 job->bytes_read = 0;
 job_sleep_ns(>common.job, delay_ns);
@@ -692,11 +698,15 @@ static int coroutine_fn backup_run(Job *job, Error **errp)
 }
 
 if (s->sync_mode == MIRROR_SYNC_MODE_NONE) {
-/* All bits are set in copy_bitmap to allow any cluster to be copied.
- * This does not actually require them to be copied. */
+/*
+ * All bits are set in copy_bitmap to allow any cluster to be copied.
+ * This does not actually require them to be copied.
+ */
 while (!job_is_cancelled(job)) {
-/* Yield until the job is cancelled.  We just let our before_write
- * notify callback service CoW requests. */
+/*
+ * Yield until the job is cancelled.  We just let our before_write
+ * notify callback service CoW requests.
+ */
 job_yield(job);
 }
 } else {
-- 
2.21.0

[PULL 22/36] scsi: move unmap error checking to the complete callback

[Max Reitz]

From: Anton Nefedov 

This will help to account the operation in the following commit.

The difference is that we don't call scsi_disk_req_check_error() before
the 1st discard iteration anymore. That function also checks if
the request is cancelled, however it shouldn't get canceled until it
yields in blk_aio() functions anyway.
Same approach is already used for emulate_write_same.

Signed-off-by: Anton Nefedov 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Alberto Garcia 
Message-id: 20190923121737.83281-7-anton.nefe...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 hw/scsi/scsi-disk.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index b3dd21800d..a002fdabe8 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1610,9 +1610,6 @@ static void scsi_unmap_complete_noio(UnmapCBData *data, 
int ret)
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
 
 assert(r->req.aiocb == NULL);
-if (scsi_disk_req_check_error(r, ret, false)) {
-goto done;
-}
 
 if (data->count > 0) {
 r->sector = ldq_be_p(>inbuf[0])
@@ -1650,7 +1647,12 @@ static void scsi_unmap_complete(void *opaque, int ret)
 r->req.aiocb = NULL;
 
 aio_context_acquire(blk_get_aio_context(s->qdev.conf.blk));
-scsi_unmap_complete_noio(data, ret);
+if (scsi_disk_req_check_error(r, ret, false)) {
+scsi_req_unref(>req);
+g_free(data);
+} else {
+scsi_unmap_complete_noio(data, ret);
+}
 aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
 }
 
-- 
2.21.0

[PULL 21/36] scsi: store unmap offset and nb_sectors in request struct

[Max Reitz]

From: Anton Nefedov 

it allows to report it in the error handler

Signed-off-by: Anton Nefedov 
Message-id: 20190923121737.83281-6-anton.nefe...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 hw/scsi/scsi-disk.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 915641a0f1..b3dd21800d 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1608,8 +1608,6 @@ static void scsi_unmap_complete_noio(UnmapCBData *data, 
int ret)
 {
 SCSIDiskReq *r = data->r;
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
-uint64_t sector_num;
-uint32_t nb_sectors;
 
 assert(r->req.aiocb == NULL);
 if (scsi_disk_req_check_error(r, ret, false)) {
@@ -1617,16 +1615,18 @@ static void scsi_unmap_complete_noio(UnmapCBData *data, 
int ret)
 }
 
 if (data->count > 0) {
-sector_num = ldq_be_p(>inbuf[0]);
-nb_sectors = ldl_be_p(>inbuf[8]) & 0xULL;
-if (!check_lba_range(s, sector_num, nb_sectors)) {
+r->sector = ldq_be_p(>inbuf[0])
+* (s->qdev.blocksize / BDRV_SECTOR_SIZE);
+r->sector_count = (ldl_be_p(>inbuf[8]) & 0xULL)
+* (s->qdev.blocksize / BDRV_SECTOR_SIZE);
+if (!check_lba_range(s, r->sector, r->sector_count)) {
 scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
 goto done;
 }
 
 r->req.aiocb = blk_aio_pdiscard(s->qdev.conf.blk,
-sector_num * s->qdev.blocksize,
-nb_sectors * s->qdev.blocksize,
+r->sector * BDRV_SECTOR_SIZE,
+r->sector_count * BDRV_SECTOR_SIZE,
 scsi_unmap_complete, data);
 data->count--;
 data->inbuf += 16;
-- 
2.21.0

[PULL 32/36] block: introduce backup-top filter driver

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Backup-top filter caches write operations and does copy-before-write
operations.

The driver will be used in backup instead of write-notifiers.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20191001131409.14202-5-vsement...@virtuozzo.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 block/Makefile.objs |   1 +
 block/backup-top.h  |  41 +++
 block/backup-top.c  | 281 
 3 files changed, 323 insertions(+)
 create mode 100644 block/backup-top.h
 create mode 100644 block/backup-top.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index f06f1fa1ac..e394fe0b6c 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -42,6 +42,7 @@ block-obj-y += block-copy.o
 block-obj-y += crypto.o
 
 block-obj-y += aio_task.o
+block-obj-y += backup-top.o
 
 common-obj-y += stream.o
 
diff --git a/block/backup-top.h b/block/backup-top.h
new file mode 100644
index 00..e5cabfa197
--- /dev/null
+++ b/block/backup-top.h
@@ -0,0 +1,41 @@
+/*
+ * backup-top filter driver
+ *
+ * The driver performs Copy-Before-Write (CBW) operation: it is injected above
+ * some node, and before each write it copies _old_ data to the target node.
+ *
+ * Copyright (c) 2018-2019 Virtuozzo International GmbH.
+ *
+ * Author:
+ *  Sementsov-Ogievskiy Vladimir 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see .
+ */
+
+#ifndef BACKUP_TOP_H
+#define BACKUP_TOP_H
+
+#include "block/block_int.h"
+#include "block/block-copy.h"
+
+BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
+ BlockDriverState *target,
+ const char *filter_node_name,
+ uint64_t cluster_size,
+ BdrvRequestFlags write_flags,
+ BlockCopyState **bcs,
+ Error **errp);
+void bdrv_backup_top_drop(BlockDriverState *bs);
+
+#endif /* BACKUP_TOP_H */
diff --git a/block/backup-top.c b/block/backup-top.c
new file mode 100644
index 00..75a315744d
--- /dev/null
+++ b/block/backup-top.c
@@ -0,0 +1,281 @@
+/*
+ * backup-top filter driver
+ *
+ * The driver performs Copy-Before-Write (CBW) operation: it is injected above
+ * some node, and before each write it copies _old_ data to the target node.
+ *
+ * Copyright (c) 2018-2019 Virtuozzo International GmbH.
+ *
+ * Author:
+ *  Sementsov-Ogievskiy Vladimir 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see .
+ */
+
+#include "qemu/osdep.h"
+
+#include "sysemu/block-backend.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
+#include "block/block_int.h"
+#include "block/qdict.h"
+#include "block/block-copy.h"
+
+#include "block/backup-top.h"
+
+typedef struct BDRVBackupTopState {
+BlockCopyState *bcs;
+BdrvChild *target;
+bool active;
+} BDRVBackupTopState;
+
+static coroutine_fn int backup_top_co_preadv(
+BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+QEMUIOVector *qiov, int flags)
+{
+return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags);
+}
+
+static coroutine_fn int backup_top_cbw(BlockDriverState *bs, uint64_t offset,
+   uint64_t bytes)
+{
+/*
+ * Here we'd like to use block_copy(), but block-copy need to be moved to
+ * use BdrvChildren to correctly use it in backup-top filter. It's a TODO.
+ */
+
+abort();
+}
+
+static int coroutine_fn backup_top_co_pdiscard(BlockDriverState *bs,
+   int64_t offset, int bytes)
+{
+int ret = backup_top_cbw(bs, offset, bytes);
+if (ret < 0) {
+return ret;
+}
+
+   

Re: [RFC PATCH] virtio-blk: advertise F_WCE (F_FLUSH) if F_CONFIG_WCE is also advertised

[Evgeny Yakovlev]

On 09.10.2019 22:14, Michael S. Tsirkin wrote:

On Tue, Oct 08, 2019 at 02:24:16PM +0100, Stefan Hajnoczi wrote:

On Fri, Sep 20, 2019 at 02:56:30PM +0300, Evgeny Yakovlev wrote:

Virtio spec 1.1 (and earlier), 5.2.5.1 Driver Requirements: Device
Initialization:

"Devices SHOULD always offer VIRTIO_BLK_F_FLUSH, and MUST offer it if
they offer VIRTIO_BLK_F_CONFIG_WCE.

It looks like currently F_CONFIG_WCE and F_WCE are not connected to each
other. qemu will advertise F_CONFIG_WCE if config-wce argument is
set for virtio-blk device. While F_WCE is advertised if underlying block
backend actually has it's caching enabled.
Those two things are not related to each other.

Signed-off-by: Evgeny Yakovlev 
---
  hw/block/virtio-blk.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 1885160..b45dc0c 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -990,7 +990,8 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, 
uint64_t features,
  virtio_add_feature(, VIRTIO_BLK_F_SCSI);
  }
  
-if (blk_enable_write_cache(s->blk)) {

+if (blk_enable_write_cache(s->blk) ||
+virtio_has_feature(features, VIRTIO_BLK_F_CONFIG_WCE)) {
  virtio_add_feature(, VIRTIO_BLK_F_WCE);
  }
  if (blk_is_read_only(s->blk)) {
--
2.7.4

Sorry for the very late response.  I have been ill and am still
recovering.

The motivation for this change looks correct but this patch may cause
host_features to change across live migration to a newer QEMU version.
If the guest accesses VIRTIO_PCI_HOST_FEATURES before and after live
migration it may see different values, which is unexpected.

The safe way of introducing guest-visible changes like this is to make
the change conditional on the machine type version so that old guests
see old behavior and new guests see new behavior.

Live migration compatibility can be guaranteed by adding a new property
to virtio_blk_properties[]:

   DEFINE_PROP_BOOL("enable-wce-if-config-wce", VirtIOBlock,
conf.enable_wce_if_config_wce, true),


is this a useful thing for users to control?
If not we don't need to make this property part of
the stable API - blacklist it by prefixing x- to the name:
x-enable-wce-if-config-wce


Then tweak the patch:

   if (blk_enable_write_cache(s->blk) ||
   (s->conf.enable_wce_if_config_wce &&
virtio_has_feature(features, VIRTIO_BLK_F_CONFIG_WCE))) {

And finally disable enable_wce_if_config_wce for older machine types to
retain compatibility:

   GlobalProperty hw_compat_4_2[] = {
   { "virtio-blk-device", "enable-wce-if-config-wce", "off" },
   };

(I have omitted some steps like defining
VirtIOBlkConf.enable_wce_if_config_wce field and hooking up
hw_compat_4_2[], but you can figure that out from the existing code.)

Stefan



Thanks, will do as you guys both suggested.

[PULL 31/36] block/block-copy: split block_copy_set_callbacks function

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Split block_copy_set_callbacks out of block_copy_state_new. It's needed
for further commit: block-copy will use BdrvChildren of backup-top
filter, so it will be created from backup-top filter creation function.
But callbacks will still belong to backup job and will be set in
separate.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20191001131409.14202-4-vsement...@virtuozzo.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 include/block/block-copy.h | 13 +
 block/backup.c |  6 --
 block/block-copy.c | 24 +++-
 3 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index 962f91056a..340d856246 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -66,12 +66,17 @@ typedef struct BlockCopyState {
 void *progress_opaque;
 } BlockCopyState;
 
-BlockCopyState *block_copy_state_new(
-BlockDriverState *source, BlockDriverState *target,
-int64_t cluster_size, BdrvRequestFlags write_flags,
+BlockCopyState *block_copy_state_new(BlockDriverState *source,
+ BlockDriverState *target,
+ int64_t cluster_size,
+ BdrvRequestFlags write_flags,
+ Error **errp);
+
+void block_copy_set_callbacks(
+BlockCopyState *s,
 ProgressBytesCallbackFunc progress_bytes_callback,
 ProgressResetCallbackFunc progress_reset_callback,
-void *progress_opaque, Error **errp);
+void *progress_opaque);
 
 void block_copy_state_free(BlockCopyState *s);
 
diff --git a/block/backup.c b/block/backup.c
index b5b7939356..1057ed0a4e 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -478,8 +478,7 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 job->bitmap_mode = bitmap_mode;
 
 job->bcs = block_copy_state_new(bs, target, cluster_size, write_flags,
-backup_progress_bytes_callback,
-backup_progress_reset_callback, job, errp);
+errp);
 if (!job->bcs) {
 goto error;
 }
@@ -487,6 +486,9 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 job->cluster_size = cluster_size;
 job->len = len;
 
+block_copy_set_callbacks(job->bcs, backup_progress_bytes_callback,
+ backup_progress_reset_callback, job);
+
 /* Required permissions are already taken by block-copy-state target */
 block_job_add_bdrv(>common, "target", target, 0, BLK_PERM_ALL,
_abort);
diff --git a/block/block-copy.c b/block/block-copy.c
index 61e5ea5f46..fcb112da14 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -66,12 +66,10 @@ void block_copy_state_free(BlockCopyState *s)
 g_free(s);
 }
 
-BlockCopyState *block_copy_state_new(
-BlockDriverState *source, BlockDriverState *target,
-int64_t cluster_size, BdrvRequestFlags write_flags,
-ProgressBytesCallbackFunc progress_bytes_callback,
-ProgressResetCallbackFunc progress_reset_callback,
-void *progress_opaque, Error **errp)
+BlockCopyState *block_copy_state_new(BlockDriverState *source,
+ BlockDriverState *target,
+ int64_t cluster_size,
+ BdrvRequestFlags write_flags, Error 
**errp)
 {
 BlockCopyState *s;
 int ret;
@@ -95,9 +93,6 @@ BlockCopyState *block_copy_state_new(
 .cluster_size = cluster_size,
 .len = bdrv_dirty_bitmap_size(copy_bitmap),
 .write_flags = write_flags,
-.progress_bytes_callback = progress_bytes_callback,
-.progress_reset_callback = progress_reset_callback,
-.progress_opaque = progress_opaque,
 };
 
 s->copy_range_size = QEMU_ALIGN_DOWN(MIN(blk_get_max_transfer(s->source),
@@ -144,6 +139,17 @@ fail:
 return NULL;
 }
 
+void block_copy_set_callbacks(
+BlockCopyState *s,
+ProgressBytesCallbackFunc progress_bytes_callback,
+ProgressResetCallbackFunc progress_reset_callback,
+void *progress_opaque)
+{
+s->progress_bytes_callback = progress_bytes_callback;
+s->progress_reset_callback = progress_reset_callback;
+s->progress_opaque = progress_opaque;
+}
+
 /*
  * Copy range to target with a bounce buffer and return the bytes copied. If
  * error occurred, return a negative error number
-- 
2.21.0

[PATCH v3 5/8] hw/ide/via82c: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

The VIA82C686B IDE controller is a PCI device, it will be reset
when the PCI bus it stands on is reset.

Convert its reset handler into a proper Device reset method.

Reviewed-by: Li Qiang 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ide/via.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/ide/via.c b/hw/ide/via.c
index 7087dc676e..053622bd82 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -29,7 +29,6 @@
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "sysemu/dma.h"
-#include "sysemu/reset.h"
 
 #include "hw/ide/pci.h"
 #include "trace.h"
@@ -120,10 +119,10 @@ static void via_ide_set_irq(void *opaque, int n, int 
level)
 }
 }
 
-static void via_ide_reset(void *opaque)
+static void via_ide_reset(DeviceState *dev)
 {
-PCIIDEState *d = opaque;
-PCIDevice *pd = PCI_DEVICE(d);
+PCIIDEState *d = PCI_IDE(dev);
+PCIDevice *pd = PCI_DEVICE(dev);
 uint8_t *pci_conf = pd->config;
 int i;
 
@@ -172,8 +171,6 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
 pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
 dev->wmask[PCI_INTERRUPT_LINE] = 0xf;
 
-qemu_register_reset(via_ide_reset, d);
-
 memory_region_init_io(>data_bar[0], OBJECT(d), _ide_data_le_ops,
   >bus[0], "via-ide0-data", 8);
 pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, >data_bar[0]);
@@ -229,6 +226,7 @@ static void via_ide_class_init(ObjectClass *klass, void 
*data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
+dc->reset = via_ide_reset;
 k->realize = via_ide_realize;
 k->exit = via_ide_exitfn;
 k->vendor_id = PCI_VENDOR_ID_VIA;
-- 
2.21.0

[PULL 30/36] block/backup: move write_flags calculation inside backup_job_create

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

This is logic-less refactoring, which simplifies further patch, as
we'll need write_flags for backup-top filter creation and backup-top
should be created before block job creation.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20191001131409.14202-3-vsement...@virtuozzo.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 block/backup.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index d918836f1d..b5b7939356 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -446,20 +446,6 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 goto error;
 }
 
-/* job->len is fixed, so we can't allow resize */
-job = block_job_create(job_id, _job_driver, txn, bs, 0, 
BLK_PERM_ALL,
-   speed, creation_flags, cb, opaque, errp);
-if (!job) {
-goto error;
-}
-
-job->source_bs = bs;
-job->on_source_error = on_source_error;
-job->on_target_error = on_target_error;
-job->sync_mode = sync_mode;
-job->sync_bitmap = sync_bitmap;
-job->bitmap_mode = bitmap_mode;
-
 /*
  * If source is in backing chain of target assume that target is going to 
be
  * used for "image fleecing", i.e. it should represent a kind of snapshot 
of
@@ -477,6 +463,20 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 write_flags = (bdrv_chain_contains(target, bs) ? BDRV_REQ_SERIALISING : 0) 
|
   (compress ? BDRV_REQ_WRITE_COMPRESSED : 0),
 
+/* job->len is fixed, so we can't allow resize */
+job = block_job_create(job_id, _job_driver, txn, bs, 0, 
BLK_PERM_ALL,
+   speed, creation_flags, cb, opaque, errp);
+if (!job) {
+goto error;
+}
+
+job->source_bs = bs;
+job->on_source_error = on_source_error;
+job->on_target_error = on_target_error;
+job->sync_mode = sync_mode;
+job->sync_bitmap = sync_bitmap;
+job->bitmap_mode = bitmap_mode;
+
 job->bcs = block_copy_state_new(bs, target, cluster_size, write_flags,
 backup_progress_bytes_callback,
 backup_progress_reset_callback, job, errp);
@@ -485,11 +485,11 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 }
 
 job->cluster_size = cluster_size;
+job->len = len;
 
 /* Required permissions are already taken by block-copy-state target */
 block_job_add_bdrv(>common, "target", target, 0, BLK_PERM_ALL,
_abort);
-job->len = len;
 
 return >common;
 
-- 
2.21.0

Re: [PATCH v2 1/1] IDE: deprecate ide-drive

[Peter Krempa]

On Thu, Oct 10, 2019 at 13:42:26 +0200, Philippe Mathieu-Daudé wrote:
> On 10/10/19 1:26 PM, Peter Krempa wrote:
> > On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote:
> > > On 10/10/19 12:43 AM, John Snow wrote:
> > > > It's an old compatibility shim that just delegates to ide-cd or ide-hd.
> > > > I'd like to refactor these some day, and getting rid of the super-object
> > > > will make that easier.
> > > > 
> > > > Either way, we don't need this.
> > > > 
> > > > Libvirt-checked-by: Peter Krempa 
> > > 
> > > Peter made a comment regarding Laszlo's Regression-tested-by tag:
> > > 
> > >[...] nobody else is using
> > >this convention (there are exactly 0 instances of
> > >"Regression-tested-by" in the project git log as far as
> > >I can see), and so in practice people reading the commits
> > >won't really know what you meant by it. Everybody else
> > >on the project uses "Tested-by" to mean either of the
> > >two cases you describe above, without distinction...
> > > 
> > > It probably applies to 'Libvirt-checked-by' too.
> > 
> > I certainly didn't test it. So feel free to drop that line altogether.
> 
> But you reviewed it, can we use your 'Reviewed-by' instead?

To be honest, I didn't really review the code nor the documentation.
I actually reviewed only the idea itself in the context of integration
with libvirt and that's why I didn't go for 'Reviewed-by:'.

The gist of the citation above is that we should stick to well known
tags with their well known meanings and I think that considering this a
'review' would be a stretch of the definiton.

[PULL 34/36] nbd: add empty .bdrv_reopen_prepare

[Max Reitz]

From: Maxim Levitsky 

Fixes commit job / qemu-img commit, when
commiting qcow2 file which is based on nbd export.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1718727
Signed-off-by: Maxim Levitsky 
Message-id: 20190930213820.29777-2-mlevi...@redhat.com
Signed-off-by: Max Reitz 
---
 block/nbd.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/block/nbd.c b/block/nbd.c
index 813c40d8f0..fd78e5f330 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1158,6 +1158,18 @@ static int coroutine_fn nbd_client_co_block_status(
 BDRV_BLOCK_OFFSET_VALID;
 }
 
+static int nbd_client_reopen_prepare(BDRVReopenState *state,
+ BlockReopenQueue *queue, Error **errp)
+{
+BDRVNBDState *s = (BDRVNBDState *)state->bs->opaque;
+
+if ((state->flags & BDRV_O_RDWR) && (s->info.flags & NBD_FLAG_READ_ONLY)) {
+error_setg(errp, "Can't reopen read-only NBD mount as read/write");
+return -EACCES;
+}
+return 0;
+}
+
 static void nbd_client_close(BlockDriverState *bs)
 {
 BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
@@ -1798,6 +1810,7 @@ static BlockDriver bdrv_nbd = {
 .instance_size  = sizeof(BDRVNBDState),
 .bdrv_parse_filename= nbd_parse_filename,
 .bdrv_file_open = nbd_open,
+.bdrv_reopen_prepare= nbd_client_reopen_prepare,
 .bdrv_co_preadv = nbd_client_co_preadv,
 .bdrv_co_pwritev= nbd_client_co_pwritev,
 .bdrv_co_pwrite_zeroes  = nbd_client_co_pwrite_zeroes,
@@ -1820,6 +1833,7 @@ static BlockDriver bdrv_nbd_tcp = {
 .instance_size  = sizeof(BDRVNBDState),
 .bdrv_parse_filename= nbd_parse_filename,
 .bdrv_file_open = nbd_open,
+.bdrv_reopen_prepare= nbd_client_reopen_prepare,
 .bdrv_co_preadv = nbd_client_co_preadv,
 .bdrv_co_pwritev= nbd_client_co_pwritev,
 .bdrv_co_pwrite_zeroes  = nbd_client_co_pwrite_zeroes,
@@ -1842,6 +1856,7 @@ static BlockDriver bdrv_nbd_unix = {
 .instance_size  = sizeof(BDRVNBDState),
 .bdrv_parse_filename= nbd_parse_filename,
 .bdrv_file_open = nbd_open,
+.bdrv_reopen_prepare= nbd_client_reopen_prepare,
 .bdrv_co_preadv = nbd_client_co_preadv,
 .bdrv_co_pwritev= nbd_client_co_pwritev,
 .bdrv_co_pwrite_zeroes  = nbd_client_co_pwrite_zeroes,
-- 
2.21.0

[PATCH v3 7/8] hw/input/lm832x: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

The LM8323 key-scan controller is a I2C device, it will be reset
when the I2C bus it stands on is reset.

Convert its reset handler into a proper Device reset method.

Reviewed-by: Li Qiang 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/input/lm832x.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/input/lm832x.c b/hw/input/lm832x.c
index a37eb854b9..aa629ddbf1 100644
--- a/hw/input/lm832x.c
+++ b/hw/input/lm832x.c
@@ -24,7 +24,6 @@
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "qemu/timer.h"
-#include "sysemu/reset.h"
 #include "ui/console.h"
 
 #define TYPE_LM8323 "lm8323"
@@ -94,8 +93,10 @@ static void lm_kbd_gpio_update(LM823KbdState *s)
 {
 }
 
-static void lm_kbd_reset(LM823KbdState *s)
+static void lm_kbd_reset(DeviceState *dev)
 {
+LM823KbdState *s = LM8323(dev);
+
 s->config = 0x80;
 s->status = INT_NOINIT;
 s->acttime = 125;
@@ -273,7 +274,7 @@ static void lm_kbd_write(LM823KbdState *s, int reg, int 
byte, uint8_t value)
 
 case LM832x_CMD_RESET:
 if (value == 0xaa)
-lm_kbd_reset(s);
+lm_kbd_reset(DEVICE(s));
 else
 lm_kbd_error(s, ERR_BADPAR);
 s->reg = LM832x_GENERAL_ERROR;
@@ -476,10 +477,6 @@ static void lm8323_realize(DeviceState *dev, Error **errp)
 s->pwm.tm[1] = timer_new_ns(QEMU_CLOCK_VIRTUAL, lm_kbd_pwm1_tick, s);
 s->pwm.tm[2] = timer_new_ns(QEMU_CLOCK_VIRTUAL, lm_kbd_pwm2_tick, s);
 qdev_init_gpio_out(dev, >nirq, 1);
-
-lm_kbd_reset(s);
-
-qemu_register_reset((void *) lm_kbd_reset, s);
 }
 
 void lm832x_key_event(DeviceState *dev, int key, int state)
@@ -507,6 +504,7 @@ static void lm8323_class_init(ObjectClass *klass, void 
*data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
 
+dc->reset = lm_kbd_reset;
 dc->realize = lm8323_realize;
 k->event = lm_i2c_event;
 k->recv = lm_i2c_rx;
-- 
2.21.0

Re: [PATCH v2 1/1] IDE: deprecate ide-drive

[Peter Krempa]

On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote:
> On 10/10/19 12:43 AM, John Snow wrote:
> > It's an old compatibility shim that just delegates to ide-cd or ide-hd.
> > I'd like to refactor these some day, and getting rid of the super-object
> > will make that easier.
> > 
> > Either way, we don't need this.
> > 
> > Libvirt-checked-by: Peter Krempa 
> 
> Peter made a comment regarding Laszlo's Regression-tested-by tag:
> 
>   [...] nobody else is using
>   this convention (there are exactly 0 instances of
>   "Regression-tested-by" in the project git log as far as
>   I can see), and so in practice people reading the commits
>   won't really know what you meant by it. Everybody else
>   on the project uses "Tested-by" to mean either of the
>   two cases you describe above, without distinction...
> 
> It probably applies to 'Libvirt-checked-by' too.

I certainly didn't test it. So feel free to drop that line altogether.

[PULL 00/36] Block patches

[Max Reitz]

The following changes since commit 98b2e3c9ab3abfe476a2b02f8f51813edb90e72d:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into 
staging (2019-10-08 16:08:35 +0100)

are available in the Git repository at:

  https://github.com/XanClic/qemu.git tags/pull-block-2019-10-10

for you to fetch changes up to 35f05b2e2ee59e077bf949057dc0959ddd6e5249:

  iotests/162: Fix for newer Linux 5.3+ (2019-10-10 12:13:23 +0200)


Block patches:
- Parallelized request handling for qcow2
- Backup job refactoring to use a filter node instead of before-write
  notifiers
- Add discard accounting information to file-posix nodes
- Allow trivial reopening of nbd nodes
- Some iotest fixes


Anton Nefedov (9):
  qapi: group BlockDeviceStats fields
  qapi: add unmap to BlockDeviceStats
  block: add empty account cookie type
  ide: account UNMAP (TRIM) operations
  scsi: store unmap offset and nb_sectors in request struct
  scsi: move unmap error checking to the complete callback
  scsi: account unmap operations
  file-posix: account discard operations
  qapi: query-blockstat: add driver specific file-posix stats

Daniel P. Berrangé (1):
  tests: fix I/O test for hosts defaulting to LUKSv2

Max Reitz (4):
  iotests: Fix 125 for growth_mode = metadata
  iotests: Disable 125 on broken XFS versions
  iotests: Use stat -c %b in 125
  iotests/162: Fix for newer Linux 5.3+

Maxim Levitsky (1):
  nbd: add empty .bdrv_reopen_prepare

Vladimir Sementsov-Ogievskiy (21):
  qemu-iotests: ignore leaks on failure paths in 026
  block: introduce aio task pool
  block/qcow2: refactor qcow2_co_preadv_part
  block/qcow2: refactor qcow2_co_pwritev_part
  block/qcow2: introduce parallel subrequest handling in read and write
  block/backup: fix max_transfer handling for copy_range
  block/backup: fix backup_cow_with_offload for last cluster
  block/backup: split shareable copying part from backup_do_cow
  block/backup: improve comment about image fleecing
  block/backup: introduce BlockCopyState
  block/backup: fix block-comment style
  block: move block_copy from block/backup.c to separate file
  block: teach bdrv_debug_breakpoint skip filters with backing
  iotests: prepare 124 and 257 bitmap querying for backup-top filter
  iotests: 257: drop unused Drive.device field
  iotests: 257: drop device_add
  block/backup: move in-flight requests handling from backup to
block-copy
  block/backup: move write_flags calculation inside backup_job_create
  block/block-copy: split block_copy_set_callbacks function
  block: introduce backup-top filter driver
  block/backup: use backup-top instead of write notifiers

 block/Makefile.objs|   4 +
 qapi/block-core.json   |  89 +++-
 block/backup-top.h |  41 ++
 block/qcow2.h  |   3 +
 include/block/accounting.h |   2 +
 include/block/aio_task.h   |  54 +++
 include/block/block-copy.h |  93 
 include/block/block.h  |   1 +
 include/block/block_int.h  |   2 +
 block.c|  43 +-
 block/accounting.c |   6 +
 block/aio_task.c   | 124 +
 block/backup-top.c | 276 +++
 block/backup.c | 443 --
 block/block-copy.c | 345 ++
 block/file-posix.c |  54 ++-
 block/nbd.c|  15 +
 block/qapi.c   |  11 +
 block/qcow2.c  | 466 ---
 block/replication.c|   2 +-
 blockdev.c |   1 +
 hw/ide/core.c  |  12 +
 hw/scsi/scsi-disk.c|  34 +-
 block/trace-events |  15 +-
 tests/qemu-iotests/026 |   6 +-
 tests/qemu-iotests/026.out |  80 +---
 tests/qemu-iotests/026.out.nocache |  80 +---
 tests/qemu-iotests/056 |   8 +-
 tests/qemu-iotests/124 |  83 ++--
 tests/qemu-iotests/125 |  45 +-
 tests/qemu-iotests/141.out |   2 +-
 tests/qemu-iotests/149 |   2 +-
 tests/qemu-iotests/149.out |  44 +-
 tests/qemu-iotests/162 |   2 +-
 tests/qemu-iotests/162.out |   2 +-
 tests/qemu-iotests/227.out |  18 +
 tests/qemu-iotests/257 |  91 ++--
 tests/qemu-iotests/257.out | 714 -
 tests/qemu-iotests/common.rc   |  17 +
 tests/qemu-iotests/iotests.py  |  27 ++
 40 files changed, 2108 insertions(+), 1249 deletions(-)
 create mode 100644 block/backup-top.h
 create mode 100644 include/block/aio_task.h
 create mode 100644 include/block/block-copy.h
 create mode 100644 block/aio_task.c
 create mode 100644 block/backup-top.c
 create mode 100644 block/block-copy.c

-- 
2.21.0

[PULL 03/36] block/qcow2: refactor qcow2_co_preadv_part

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Further patch will run partial requests of iterations of
qcow2_co_preadv in parallel for performance reasons. To prepare for
this, separate part which may be parallelized into separate function
(qcow2_co_preadv_task).

While being here, also separate encrypted clusters reading to own
function, like it is done for compressed reading.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Message-id: 20190916175324.18478-4-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block/qcow2.c | 209 +++---
 1 file changed, 113 insertions(+), 96 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 4d16393e61..6feb169f7c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1972,17 +1972,117 @@ out:
 return ret;
 }
 
+static coroutine_fn int
+qcow2_co_preadv_encrypted(BlockDriverState *bs,
+   uint64_t file_cluster_offset,
+   uint64_t offset,
+   uint64_t bytes,
+   QEMUIOVector *qiov,
+   uint64_t qiov_offset)
+{
+int ret;
+BDRVQcow2State *s = bs->opaque;
+uint8_t *buf;
+
+assert(bs->encrypted && s->crypto);
+assert(bytes <= QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
+
+/*
+ * For encrypted images, read everything into a temporary
+ * contiguous buffer on which the AES functions can work.
+ * Also, decryption in a separate buffer is better as it
+ * prevents the guest from learning information about the
+ * encrypted nature of the virtual disk.
+ */
+
+buf = qemu_try_blockalign(s->data_file->bs, bytes);
+if (buf == NULL) {
+return -ENOMEM;
+}
+
+BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
+ret = bdrv_co_pread(s->data_file,
+file_cluster_offset + offset_into_cluster(s, offset),
+bytes, buf, 0);
+if (ret < 0) {
+goto fail;
+}
+
+assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE));
+assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE));
+if (qcow2_co_decrypt(bs,
+ file_cluster_offset + offset_into_cluster(s, offset),
+ offset, buf, bytes) < 0)
+{
+ret = -EIO;
+goto fail;
+}
+qemu_iovec_from_buf(qiov, qiov_offset, buf, bytes);
+
+fail:
+qemu_vfree(buf);
+
+return ret;
+}
+
+static coroutine_fn int qcow2_co_preadv_task(BlockDriverState *bs,
+ QCow2ClusterType cluster_type,
+ uint64_t file_cluster_offset,
+ uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov,
+ size_t qiov_offset)
+{
+BDRVQcow2State *s = bs->opaque;
+int offset_in_cluster = offset_into_cluster(s, offset);
+
+switch (cluster_type) {
+case QCOW2_CLUSTER_ZERO_PLAIN:
+case QCOW2_CLUSTER_ZERO_ALLOC:
+/* Both zero types are handled in qcow2_co_preadv_part */
+g_assert_not_reached();
+
+case QCOW2_CLUSTER_UNALLOCATED:
+assert(bs->backing); /* otherwise handled in qcow2_co_preadv_part */
+
+BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
+return bdrv_co_preadv_part(bs->backing, offset, bytes,
+   qiov, qiov_offset, 0);
+
+case QCOW2_CLUSTER_COMPRESSED:
+return qcow2_co_preadv_compressed(bs, file_cluster_offset,
+  offset, bytes, qiov, qiov_offset);
+
+case QCOW2_CLUSTER_NORMAL:
+if ((file_cluster_offset & 511) != 0) {
+return -EIO;
+}
+
+if (bs->encrypted) {
+return qcow2_co_preadv_encrypted(bs, file_cluster_offset,
+ offset, bytes, qiov, qiov_offset);
+}
+
+BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
+return bdrv_co_preadv_part(s->data_file,
+   file_cluster_offset + offset_in_cluster,
+   bytes, qiov, qiov_offset, 0);
+
+default:
+g_assert_not_reached();
+}
+
+g_assert_not_reached();
+}
+
 static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,
  uint64_t offset, uint64_t bytes,
  QEMUIOVector *qiov,
  size_t qiov_offset, int flags)
 {
 BDRVQcow2State *s = bs->opaque;
-int offset_in_cluster;
 int ret;
 unsigned int cur_bytes; /* number of bytes in current iteration */
 uint64_t cluster_offset = 0;
-uint8_t *cluster_data = NULL;
 
 while (bytes != 0) {
 
@@ -1997,112 +2097,29 @@ static coroutine_fn int 
qcow2_co_preadv_part(BlockDriverState *bs,
 ret = 

[PULL 02/36] block: introduce aio task pool

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Common interface for aio task loops. To be used for improving
performance of synchronous io loops in qcow2, block-stream,
copy-on-read, and may be other places.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Message-id: 20190916175324.18478-3-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block/Makefile.objs  |   2 +
 include/block/aio_task.h |  54 +
 block/aio_task.c | 124 +++
 3 files changed, 180 insertions(+)
 create mode 100644 include/block/aio_task.h
 create mode 100644 block/aio_task.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 35f3bca4d9..c2eb8c8769 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -40,6 +40,8 @@ block-obj-y += throttle.o copy-on-read.o
 
 block-obj-y += crypto.o
 
+block-obj-y += aio_task.o
+
 common-obj-y += stream.o
 
 nfs.o-libs := $(LIBNFS_LIBS)
diff --git a/include/block/aio_task.h b/include/block/aio_task.h
new file mode 100644
index 00..50bc1e1817
--- /dev/null
+++ b/include/block/aio_task.h
@@ -0,0 +1,54 @@
+/*
+ * Aio tasks loops
+ *
+ * Copyright (c) 2019 Virtuozzo International GmbH.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef BLOCK_AIO_TASK_H
+#define BLOCK_AIO_TASK_H
+
+#include "qemu/coroutine.h"
+
+typedef struct AioTaskPool AioTaskPool;
+typedef struct AioTask AioTask;
+typedef int coroutine_fn (*AioTaskFunc)(AioTask *task);
+struct AioTask {
+AioTaskPool *pool;
+AioTaskFunc func;
+int ret;
+};
+
+AioTaskPool *coroutine_fn aio_task_pool_new(int max_busy_tasks);
+void aio_task_pool_free(AioTaskPool *);
+
+/* error code of failed task or 0 if all is OK */
+int aio_task_pool_status(AioTaskPool *pool);
+
+bool aio_task_pool_empty(AioTaskPool *pool);
+
+/* User provides filled @task, however task->pool will be set automatically */
+void coroutine_fn aio_task_pool_start_task(AioTaskPool *pool, AioTask *task);
+
+void coroutine_fn aio_task_pool_wait_slot(AioTaskPool *pool);
+void coroutine_fn aio_task_pool_wait_one(AioTaskPool *pool);
+void coroutine_fn aio_task_pool_wait_all(AioTaskPool *pool);
+
+#endif /* BLOCK_AIO_TASK_H */
diff --git a/block/aio_task.c b/block/aio_task.c
new file mode 100644
index 00..88989fa248
--- /dev/null
+++ b/block/aio_task.c
@@ -0,0 +1,124 @@
+/*
+ * Aio tasks loops
+ *
+ * Copyright (c) 2019 Virtuozzo International GmbH.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "block/aio.h"
+#include "block/aio_task.h"
+
+struct AioTaskPool {
+Coroutine *main_co;
+int status;
+int max_busy_tasks;
+int busy_tasks;
+bool waiting;
+};
+
+static void coroutine_fn aio_task_co(void *opaque)
+{
+AioTask *task = opaque;
+AioTaskPool *pool = task->pool;
+
+assert(pool->busy_tasks < pool->max_busy_tasks);
+

[PULL 14/36] iotests: prepare 124 and 257 bitmap querying for backup-top filter

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

After backup-top filter appearing it's not possible to see dirty
bitmaps in top node, so use node-name instead.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Message-id: 20190920142056.12778-10-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/124|  83 
 tests/qemu-iotests/257|  49 ++---
 tests/qemu-iotests/257.out| 364 +-
 tests/qemu-iotests/iotests.py |  27 +++
 4 files changed, 219 insertions(+), 304 deletions(-)

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index ca40ba3be2..d3e851e1ae 100755
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -105,7 +105,7 @@ class TestIncrementalBackupBase(iotests.QMPTestCase):
 # Create a base image with a distinctive patterning
 drive0 = self.add_node('drive0')
 self.img_create(drive0['file'], drive0['fmt'])
-self.vm.add_drive(drive0['file'])
+self.vm.add_drive(drive0['file'], opts='node-name=node0')
 self.write_default_pattern(drive0['file'])
 self.vm.launch()
 
@@ -348,12 +348,14 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
 ('0xfe', '16M', '256k'),
 ('0x64', '32736k', '64k')))
 # Check the dirty bitmap stats
-result = self.vm.qmp('query-block')
-self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/name', 'bitmap0')
-self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/count', 458752)
-self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/granularity', 
65536)
-self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/status', 'active')
-self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/persistent', False)
+self.assertTrue(self.vm.check_bitmap_status(
+'node0', bitmap0.name, {
+'name': 'bitmap0',
+'count': 458752,
+'granularity': 65536,
+'status': 'active',
+'persistent': False
+}))
 
 # Prepare a cluster_size=128k backup target without a backing file.
 (target, _) = bitmap0.new_target()
@@ -670,9 +672,8 @@ class 
TestIncrementalBackupBlkdebug(TestIncrementalBackupBase):
 """
 
 drive0 = self.drives[0]
-# NB: The blkdebug script here looks for a "flush, read, read" pattern.
-# The flush occurs in hmp_io_writes, the first read in device_add, and
-# the last read during the block job.
+# NB: The blkdebug script here looks for a "flush, read" pattern.
+# The flush occurs in hmp_io_writes, and the read during the block job.
 result = self.vm.qmp('blockdev-add',
  node_name=drive0['id'],
  driver=drive0['fmt'],
@@ -686,15 +687,11 @@ class 
TestIncrementalBackupBlkdebug(TestIncrementalBackupBase):
  'event': 'flush_to_disk',
  'state': 1,
  'new_state': 2
- },{
- 'event': 'read_aio',
- 'state': 2,
- 'new_state': 3
  }],
  'inject-error': [{
  'event': 'read_aio',
  'errno': 5,
- 'state': 3,
+ 'state': 2,
  'immediately': False,
  'once': True
  }],
@@ -708,23 +705,15 @@ class 
TestIncrementalBackupBlkdebug(TestIncrementalBackupBase):
   ('0xfe', '16M', '256k'),
   ('0x64', '32736k', '64k')))
 
-# For the purposes of query-block visibility of bitmaps, add a drive
-# frontend after we've written data; otherwise we can't use hmp-io
-result = self.vm.qmp("device_add",
- id="device0",
- drive=drive0['id'],
- driver="virtio-blk")
-self.assert_qmp(result, 'return', {})
-
 # Bitmap Status Check
-query = self.vm.qmp('query-block')
-ret = [bmap for bmap in query['return'][0]['dirty-bitmaps']
-   if bmap.get('name') == bitmap.name][0]
-self.assert_qmp(ret, 'count', 458752)
-self.assert_qmp(ret, 'granularity', 65536)
-self.assert_qmp(ret, 'status', 'active')
-self.assert_qmp(ret, 'busy', False)
-self.assert_qmp(ret, 'recording', True)
+self.assertTrue(self.vm.check_bitmap_status(
+drive0['id'], bitmap.name, {
+'count': 458752,
+

[PULL 01/36] qemu-iotests: ignore leaks on failure paths in 026

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Upcoming asynchronous handling of sub-parts of qcow2 requests will
change number of leaked clusters and even make it racy. As a
preparation, ignore leaks on failure parts in 026.

It's not trivial to just grep or substitute qemu-img output for such
thing. Instead do better: 3 is a error code of qemu-img check, if only
leaks are found. Catch this case and print success output.

Suggested-by: Anton Nefedov 
Signed-off-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20190916175324.18478-2-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/026 |  6 +--
 tests/qemu-iotests/026.out | 80 --
 tests/qemu-iotests/026.out.nocache | 80 --
 tests/qemu-iotests/common.rc   | 17 +++
 4 files changed, 60 insertions(+), 123 deletions(-)

diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026
index ffb18ab6b5..3430029ed6 100755
--- a/tests/qemu-iotests/026
+++ b/tests/qemu-iotests/026
@@ -107,7 +107,7 @@ if [ "$event" == "l2_load" ]; then
 $QEMU_IO -c "read $vmstate 0 128k " "$BLKDBG_TEST_IMG" | _filter_qemu_io
 fi
 
-_check_test_img 2>&1 | grep -v "refcount=1 reference=0"
+_check_test_img_ignore_leaks 2>&1 | grep -v "refcount=1 reference=0"
 
 done
 done
@@ -152,7 +152,7 @@ echo
 echo "Event: $event; errno: $errno; imm: $imm; once: $once; write $vmstate"
 $QEMU_IO -c "write $vmstate 0 64M" "$BLKDBG_TEST_IMG" | _filter_qemu_io
 
-_check_test_img 2>&1 | grep -v "refcount=1 reference=0"
+_check_test_img_ignore_leaks 2>&1 | grep -v "refcount=1 reference=0"
 
 done
 done
@@ -191,7 +191,7 @@ echo
 echo "Event: $event; errno: $errno; imm: $imm; once: $once"
 $QEMU_IO -c "write -b 0 64k" "$BLKDBG_TEST_IMG" | _filter_qemu_io
 
-_check_test_img 2>&1 | grep -v "refcount=1 reference=0"
+_check_test_img_ignore_leaks 2>&1 | grep -v "refcount=1 reference=0"
 
 done
 done
diff --git a/tests/qemu-iotests/026.out b/tests/qemu-iotests/026.out
index fb89b8480c..ff0817b6f2 100644
--- a/tests/qemu-iotests/026.out
+++ b/tests/qemu-iotests/026.out
@@ -17,18 +17,14 @@ Event: l1_update; errno: 5; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l1_update; errno: 5; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l1_update; errno: 28; imm: off; once: on; write
@@ -45,18 +41,14 @@ Event: l1_update; errno: 28; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l1_update; errno: 28; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l2_load; errno: 5; imm: off; once: on; write
@@ -137,18 +129,14 @@ Event: l2_update; errno: 5; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-
-127 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l2_update; errno: 5; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-
-127 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l2_update; errno: 28; imm: off; once: on; write
@@ -165,18 +153,14 @@ Event: l2_update; errno: 28; imm: off; once: off; write
 qemu-io: 

[PULL 15/36] iotests: 257: drop unused Drive.device field

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

After previous commit Drive.device is actually unused. Drop it together
with .name property.  While being here reuse .node in qmp commands
instead of writing 'drive0' twice.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Message-id: 20190920142056.12778-11-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/257 | 37 +++--
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/tests/qemu-iotests/257 b/tests/qemu-iotests/257
index 6b368e1e70..5d77202157 100755
--- a/tests/qemu-iotests/257
+++ b/tests/qemu-iotests/257
@@ -148,11 +148,6 @@ class Drive:
 self.fmt = None
 self.size = None
 self.node = None
-self.device = None
-
-@property
-def name(self):
-return self.node or self.device
 
 def img_create(self, fmt, size):
 self.fmt = fmt
@@ -201,7 +196,7 @@ def blockdev_backup(vm, device, target, sync, **kwargs):
 def blockdev_backup_mktarget(drive, target_id, filepath, sync, **kwargs):
 target_drive = Drive(filepath, vm=drive.vm)
 target_drive.create_target(target_id, drive.fmt, drive.size)
-blockdev_backup(drive.vm, drive.name, target_id, sync, **kwargs)
+blockdev_backup(drive.vm, drive.node, target_id, sync, **kwargs)
 
 def reference_backup(drive, n, filepath):
 log("--- Reference Backup #{:d} ---\n".format(n))
@@ -229,7 +224,7 @@ def perform_writes(drive, n):
 pattern.offset,
 pattern.size)
 log(cmd)
-log(drive.vm.hmp_qemu_io(drive.name, cmd))
+log(drive.vm.hmp_qemu_io(drive.node, cmd))
 bitmaps = drive.vm.query_bitmaps()
 log({'bitmaps': bitmaps}, indent=2)
 log('')
@@ -324,18 +319,17 @@ def test_bitmap_sync(bsync_mode, msync_mode='bitmap', 
failure=None):
 }]
 }
 
+drive0.node = 'drive0'
 vm.qmp_log('blockdev-add',
filters=[iotests.filter_qmp_testfiles],
-   node_name="drive0",
+   node_name=drive0.node,
driver=drive0.fmt,
file=file_config)
-drive0.node = 'drive0'
-drive0.device = 'device0'
 # Use share-rw to allow writes directly to the node;
 # The anonymous block-backend for this configuration prevents us
 # from using HMP's qemu-io commands to address the device.
-vm.qmp_log("device_add", id=drive0.device,
-   drive=drive0.name, driver="scsi-hd",
+vm.qmp_log("device_add", id='device0',
+   drive=drive0.node, driver="scsi-hd",
share_rw=True)
 log('')
 
@@ -343,7 +337,7 @@ def test_bitmap_sync(bsync_mode, msync_mode='bitmap', 
failure=None):
 perform_writes(drive0, 0)
 reference_backup(drive0, 0, fbackup0)
 log('--- Add Bitmap ---\n')
-vm.qmp_log("block-dirty-bitmap-add", node=drive0.name,
+vm.qmp_log("block-dirty-bitmap-add", node=drive0.node,
name="bitmap0", granularity=GRANULARITY)
 log('')
 ebitmap = EmulatedBitmap()
@@ -358,7 +352,7 @@ def test_bitmap_sync(bsync_mode, msync_mode='bitmap', 
failure=None):
 # 1 - Test Backup (w/ Optional induced failure)
 if failure == 'intermediate':
 # Activate blkdebug induced failure for second-to-next read
-log(vm.hmp_qemu_io(drive0.name, 'flush'))
+log(vm.hmp_qemu_io(drive0.node, 'flush'))
 log('')
 job = backup(drive0, 1, bsync1, msync_mode,
  bitmap="bitmap0", bitmap_mode=bsync_mode)
@@ -426,7 +420,7 @@ def test_bitmap_sync(bsync_mode, msync_mode='bitmap', 
failure=None):
 
 log('--- Cleanup ---\n')
 vm.qmp_log("block-dirty-bitmap-remove",
-   node=drive0.name, name="bitmap0")
+   node=drive0.node, name="bitmap0")
 bitmaps = vm.query_bitmaps()
 log({'bitmaps': bitmaps}, indent=2)
 vm.shutdown()
@@ -467,22 +461,21 @@ def test_backup_api():
 'filename': drive0.path
 }
 
+drive0.node = 'drive0'
 vm.qmp_log('blockdev-add',
filters=[iotests.filter_qmp_testfiles],
-   node_name="drive0",
+   node_name=drive0.node,
driver=drive0.fmt,
file=file_config)
-drive0.node = 'drive0'
-drive0.device = 'device0'
-vm.qmp_log("device_add", id=drive0.device,
-   drive=drive0.name, driver="scsi-hd")
+vm.qmp_log("device_add", id='device0',
+   drive=drive0.node, driver="scsi-hd")
 log('')
 
 target0 = Drive(backup_path, vm=vm)
 target0.create_target("backup_target", drive0.fmt, drive0.size)
 log('')
 
-vm.qmp_log("block-dirty-bitmap-add", node=drive0.name,
+vm.qmp_log("block-dirty-bitmap-add", 

[PULL 35/36] tests: fix I/O test for hosts defaulting to LUKSv2

[Max Reitz]

From: Daniel P. Berrangé 

Some distros are now defaulting to LUKS version 2 which QEMU cannot
process. For our I/O test that validates interoperability between the
kernel/cryptsetup and QEMU, we need to explicitly ask for version 1
of the LUKS format.

Signed-off-by: Daniel P. Berrangé 
Message-id: 20190927101155.25896-1-berra...@redhat.com
Tested-by: Maxim Levitsky 
Reviewed-by: Maxim Levitsky 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/149 |  2 +-
 tests/qemu-iotests/149.out | 44 +++---
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
index 4f363f295f..8ab42e94c6 100755
--- a/tests/qemu-iotests/149
+++ b/tests/qemu-iotests/149
@@ -153,7 +153,7 @@ def cryptsetup_format(config):
 
 (password, slot) = config.first_password()
 
-args = ["luksFormat"]
+args = ["luksFormat", "--type", "luks1"]
 cipher = config.cipher + "-" + config.mode + "-" + config.ivgen
 if config.ivgen_hash is not None:
 cipher = cipher + ":" + config.ivgen_hash
diff --git a/tests/qemu-iotests/149.out b/tests/qemu-iotests/149.out
index 1407ce6dad..6877ab6c4a 100644
--- a/tests/qemu-iotests/149.out
+++ b/tests/qemu-iotests/149.out
@@ -2,7 +2,7 @@
 # Create image
 truncate TEST_DIR/luks-aes-256-xts-plain64-sha1.img --size 4194304MB
 # Format image
-sudo cryptsetup -q -v luksFormat --cipher aes-xts-plain64 --key-size 512 
--hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-aes-256-xts-plain64-sha1.img
+sudo cryptsetup -q -v luksFormat --type luks1 --cipher aes-xts-plain64 
--key-size 512 --hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-aes-256-xts-plain64-sha1.img
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-xts-plain64-sha1.img 
qiotest-145-aes-256-xts-plain64-sha1
 # Write test pattern 0xa7
@@ -122,7 +122,7 @@ unlink TEST_DIR/luks-aes-256-xts-plain64-sha1.img
 # Create image
 truncate TEST_DIR/luks-twofish-256-xts-plain64-sha1.img --size 4194304MB
 # Format image
-sudo cryptsetup -q -v luksFormat --cipher twofish-xts-plain64 --key-size 512 
--hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-twofish-256-xts-plain64-sha1.img
+sudo cryptsetup -q -v luksFormat --type luks1 --cipher twofish-xts-plain64 
--key-size 512 --hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-twofish-256-xts-plain64-sha1.img
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-twofish-256-xts-plain64-sha1.img 
qiotest-145-twofish-256-xts-plain64-sha1
 # Write test pattern 0xa7
@@ -242,7 +242,7 @@ unlink TEST_DIR/luks-twofish-256-xts-plain64-sha1.img
 # Create image
 truncate TEST_DIR/luks-serpent-256-xts-plain64-sha1.img --size 4194304MB
 # Format image
-sudo cryptsetup -q -v luksFormat --cipher serpent-xts-plain64 --key-size 512 
--hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-serpent-256-xts-plain64-sha1.img
+sudo cryptsetup -q -v luksFormat --type luks1 --cipher serpent-xts-plain64 
--key-size 512 --hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-serpent-256-xts-plain64-sha1.img
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-serpent-256-xts-plain64-sha1.img 
qiotest-145-serpent-256-xts-plain64-sha1
 # Write test pattern 0xa7
@@ -362,7 +362,7 @@ unlink TEST_DIR/luks-serpent-256-xts-plain64-sha1.img
 # Create image
 truncate TEST_DIR/luks-cast5-128-cbc-plain64-sha1.img --size 4194304MB
 # Format image
-sudo cryptsetup -q -v luksFormat --cipher cast5-cbc-plain64 --key-size 128 
--hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-cast5-128-cbc-plain64-sha1.img
+sudo cryptsetup -q -v luksFormat --type luks1 --cipher cast5-cbc-plain64 
--key-size 128 --hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-cast5-128-cbc-plain64-sha1.img
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-cast5-128-cbc-plain64-sha1.img 
qiotest-145-cast5-128-cbc-plain64-sha1
 # Write test pattern 0xa7
@@ -483,7 +483,7 @@ Skipping cast6-256-xts-plain64-sha1 in blacklist
 # Create image
 truncate TEST_DIR/luks-aes-256-cbc-plain-sha1.img --size 4194304MB
 # Format image
-sudo cryptsetup -q -v luksFormat --cipher aes-cbc-plain --key-size 256 --hash 
sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-aes-256-cbc-plain-sha1.img
+sudo cryptsetup -q -v luksFormat --type luks1 --cipher aes-cbc-plain 
--key-size 256 --hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-aes-256-cbc-plain-sha1.img
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-cbc-plain-sha1.img 
qiotest-145-aes-256-cbc-plain-sha1
 # Write test pattern 0xa7
@@ -603,7 +603,7 @@ unlink TEST_DIR/luks-aes-256-cbc-plain-sha1.img
 # Create image
 truncate TEST_DIR/luks-aes-256-cbc-plain64-sha1.img --size 4194304MB
 # Format image
-sudo cryptsetup -q -v luksFormat --cipher aes-cbc-plain64 --key-size 256 
--hash sha1 --key-slot 0 --key-file - --iter-time 10 

[PULL 25/36] qapi: query-blockstat: add driver specific file-posix stats

[Max Reitz]

From: Anton Nefedov 

A block driver can provide a callback to report driver-specific
statistics.

file-posix driver now reports discard statistics

Signed-off-by: Anton Nefedov 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Acked-by: Markus Armbruster 
Message-id: 20190923121737.83281-10-anton.nefe...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 qapi/block-core.json  | 38 ++
 include/block/block.h |  1 +
 include/block/block_int.h |  1 +
 block.c   |  9 +
 block/file-posix.c| 32 
 block/qapi.c  |  5 +
 6 files changed, 86 insertions(+)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7d3e05891c..859acea014 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -960,6 +960,41 @@
'*wr_latency_histogram': 'BlockLatencyHistogramInfo',
'*flush_latency_histogram': 'BlockLatencyHistogramInfo' } }
 
+##
+# @BlockStatsSpecificFile:
+#
+# File driver statistics
+#
+# @discard-nb-ok: The number of successful discard operations performed by
+# the driver.
+#
+# @discard-nb-failed: The number of failed discard operations performed by
+# the driver.
+#
+# @discard-bytes-ok: The number of bytes discarded by the driver.
+#
+# Since: 4.2
+##
+{ 'struct': 'BlockStatsSpecificFile',
+  'data': {
+  'discard-nb-ok': 'uint64',
+  'discard-nb-failed': 'uint64',
+  'discard-bytes-ok': 'uint64' } }
+
+##
+# @BlockStatsSpecific:
+#
+# Block driver specific statistics
+#
+# Since: 4.2
+##
+{ 'union': 'BlockStatsSpecific',
+  'base': { 'driver': 'BlockdevDriver' },
+  'discriminator': 'driver',
+  'data': {
+  'file': 'BlockStatsSpecificFile',
+  'host_device': 'BlockStatsSpecificFile' } }
+
 ##
 # @BlockStats:
 #
@@ -975,6 +1010,8 @@
 #
 # @stats:  A @BlockDeviceStats for the device.
 #
+# @driver-specific: Optional driver-specific stats. (Since 4.2)
+#
 # @parent: This describes the file block device if it has one.
 #  Contains recursively the statistics of the underlying
 #  protocol (e.g. the host file for a qcow2 image). If there is
@@ -988,6 +1025,7 @@
 { 'struct': 'BlockStats',
   'data': {'*device': 'str', '*qdev': 'str', '*node-name': 'str',
'stats': 'BlockDeviceStats',
+   '*driver-specific': 'BlockStatsSpecific',
'*parent': 'BlockStats',
'*backing': 'BlockStats'} }
 
diff --git a/include/block/block.h b/include/block/block.h
index 37c9de7446..792bb826db 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -501,6 +501,7 @@ int bdrv_get_flags(BlockDriverState *bs);
 int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
 ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs,
   Error **errp);
+BlockStatsSpecific *bdrv_get_specific_stats(BlockDriverState *bs);
 void bdrv_round_to_clusters(BlockDriverState *bs,
 int64_t offset, int64_t bytes,
 int64_t *cluster_offset,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 0422acdf1c..2b113eb3c7 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -366,6 +366,7 @@ struct BlockDriver {
 int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi);
 ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs,
  Error **errp);
+BlockStatsSpecific *(*bdrv_get_specific_stats)(BlockDriverState *bs);
 
 int coroutine_fn (*bdrv_save_vmstate)(BlockDriverState *bs,
   QEMUIOVector *qiov,
diff --git a/block.c b/block.c
index 1c7c199849..1946fc6f57 100644
--- a/block.c
+++ b/block.c
@@ -5155,6 +5155,15 @@ ImageInfoSpecific 
*bdrv_get_specific_info(BlockDriverState *bs,
 return NULL;
 }
 
+BlockStatsSpecific *bdrv_get_specific_stats(BlockDriverState *bs)
+{
+BlockDriver *drv = bs->drv;
+if (!drv || !drv->bdrv_get_specific_stats) {
+return NULL;
+}
+return drv->bdrv_get_specific_stats(bs);
+}
+
 void bdrv_debug_event(BlockDriverState *bs, BlkdebugEvent event)
 {
 if (!bs || !bs->drv || !bs->drv->bdrv_debug_event) {
diff --git a/block/file-posix.c b/block/file-posix.c
index f3934c4e10..695fcf740d 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2753,6 +2753,36 @@ static int raw_get_info(BlockDriverState *bs, 
BlockDriverInfo *bdi)
 return 0;
 }
 
+static BlockStatsSpecificFile get_blockstats_specific_file(BlockDriverState 
*bs)
+{
+BDRVRawState *s = bs->opaque;
+return (BlockStatsSpecificFile) {
+.discard_nb_ok = s->stats.discard_nb_ok,
+.discard_nb_failed = s->stats.discard_nb_failed,
+.discard_bytes_ok = s->stats.discard_bytes_ok,
+};
+}
+
+static BlockStatsSpecific *raw_get_specific_stats(BlockDriverState *bs)
+{
+BlockStatsSpecific *stats = 

[PULL 27/36] iotests: Disable 125 on broken XFS versions

[Max Reitz]

And by that I mean all XFS versions, as far as I can tell.  All details
are in the comment below.

We never noticed this problem because we only read the first number from
qemu-img info's "disk size" output -- and that is effectively useless,
because qemu-img prints a human-readable value (which generally includes
a decimal point).  That will be fixed in the next patch.

Signed-off-by: Max Reitz 
Message-id: 20190925183231.11196-3-mre...@redhat.com
Reviewed-by: Eric Blake 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/125 | 40 
 1 file changed, 40 insertions(+)

diff --git a/tests/qemu-iotests/125 b/tests/qemu-iotests/125
index df328a63a6..0ef51f1e21 100755
--- a/tests/qemu-iotests/125
+++ b/tests/qemu-iotests/125
@@ -49,6 +49,46 @@ if [ -z "$TEST_IMG_FILE" ]; then
 TEST_IMG_FILE=$TEST_IMG
 fi
 
+# Test whether we are running on a broken XFS version.  There is this
+# bug:
+
+# $ rm -f foo
+# $ touch foo
+# $ block_size=4096 # Your FS's block size
+# $ fallocate -o $((block_size / 2)) -l $block_size foo
+# $ LANG=C xfs_bmap foo | grep hole
+# 1: [8..15]: hole
+#
+# The problem is that the XFS driver rounds down the offset and
+# rounds up the length to the block size, but independently.  As
+# such, it only allocates the first block in the example above,
+# even though it should allocate the first two blocks (because our
+# request is to fallocate something that touches both the first
+# two blocks).
+#
+# This means that when you then write to the beginning of the
+# second block, the disk usage of the first two blocks grows.
+#
+# That is precisely what fallocate() promises, though: That when you
+# write to an area that you have fallocated, no new blocks will have
+# to be allocated.
+
+touch "$TEST_IMG_FILE"
+# Assuming there is no FS with a block size greater than 64k
+fallocate -o 65535 -l 2 "$TEST_IMG_FILE"
+len0=$(get_image_size_on_host)
+
+# Write to something that in theory we have just fallocated
+# (Thus, the on-disk size should not increase)
+poke_file "$TEST_IMG_FILE" 65536 42
+len1=$(get_image_size_on_host)
+
+if [ $len1 -gt $len0 ]; then
+_notrun "the test filesystem's fallocate() is broken"
+fi
+
+rm -f "$TEST_IMG_FILE"
+
 # Generally, we create some image with or without existing preallocation and
 # then resize it. Then we write some data into the image and verify that its
 # size does not change if we have used preallocation.
-- 
2.21.0

Re: [PATCH v2 0/8] hw: Convert various reset() handler to DeviceReset

[Philippe Mathieu-Daudé]

On 10/10/19 3:05 AM, Li Qiang wrote:
Philippe Mathieu-Daudé mailto:phi...@redhat.com>> 于 
2019年10月10日周四 上午3:54写道：


Hi Li,

On 10/9/19 4:28 AM, Li Qiang wrote:
 > Philippe Mathieu-Daudé mailto:phi...@redhat.com> >> 于
 > 2019年10月8日周二 下午10:47写道：
 >
 >     Since v1:
 >     - Removed the pci-host devices
 >
 >
 > Hello  I want to know why  remove this?

I haven't removed the devices, I simply remove the patches converting
them to DeviceReset, 



Yes, I mean the patches.

basically because I've not enough time to check if
the are on a bus that would reset them. 



IIUC, they are right.

I added these devices on my TODO
list for later, so meanwhile the other devices can be easily reviewed
and merged. When few patches from a series are not reviewed or
incorrect, sometime the rest of the series is not merged, so I
prefer to
split it and get these patches merged.


As far as I can see, most of the devices' usage of qemu_register_reset 
function can be

convert to 'dc->reset'. In the main function.

qemu_register_reset(qbus_reset_all_fn, sysbus_get_default());

The 'qbus_reset_all_fn' calls 'qbus_reset_all' from the 'main-sys-bus'. 
Then 'qdev_reset_one'
will call 'device_reset'. So IIUC every bus attached to 'main-sys-bus' 
can be reset through 'dc->reset'


So I'm quite sure most of the cases that devices use 
'qemu_register_reset' can be changed to 'dc->reset'.
Seems you're busy,  If you don't mind, I can do some of the work to 
convert 'reset' callback(not a patchset, one by one).


I guess we are all busy ;) I'm just trying to prioritize here.
This series is not very important because what we have today works, and 
I would rather not introduce regressions. What happened then is it is 
the 3rd time at least I get confuse with the qemu_register_reset() 
function while reviewing code. Then my rule of thumb is if an 
improvement takes less than 1h, then I just do it and keep going, else 
if I postpone it I'll never go back to it. When a series start to take 
too much rework it means I might started it wrongly, or I underestimated 
the time required. Time that is taken of other more important tasks.
Today I prefer merged a subset of the series that is correct, rather 
than aiming for the whole and never get it merged.


If you are interested in respining/fixing the pci-host devices I'd be 
very thankful! I appreciate your help (and reviews).


Regards,

Phil.

PD: I'll respin as v3 with your tags and the PIIX4_IDE fix.


 >
 >     - Removed the vmcoreinfo conversion (elmarco) but add a comment.
 >     - Added Igor's R-b tag.
 >
 >     Following the thread discussion between Peter/Markus/Damien about
 >     reset handlers:
 > https://www.mail-archive.com/qemu-devel@nongnu.org/msg617103.html
 >     I started to remove qemu_register_reset() calls from few
qdevified
 >     devices (the trivial ones).
 >
 >     Regards,
 >
 >     Phil.
 >
 >     v1:
https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg06367.html
 >
 >     Philippe Mathieu-Daudé (8):
 >        hw/acpi/piix4: Convert reset handler to DeviceReset
 >        hw/isa/piix4: Convert reset handler to DeviceReset
 >        hw/ide/piix: Convert reset handler to DeviceReset
 >        hw/ide/sii3112: Convert reset handler to DeviceReset
 >        hw/ide/via82c: Convert reset handler to DeviceReset
 >        hw/isa/vt82c686: Convert reset handler to DeviceReset
 >        hw/input/lm832x: Convert reset handler to DeviceReset
 >        hw/misc/vmcoreinfo: Document its reset handler

[PATCH v3 6/8] hw/isa/vt82c686: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

The VIA VT82C686 Southbridge is a PCI device, it will be reset
when the PCI bus it stands on is reset.

Convert its reset handler into a proper Device reset method.

Reviewed-by: Li Qiang 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/isa/vt82c686.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 50bd28fa82..616f67f347 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -23,7 +23,6 @@
 #include "hw/isa/apm.h"
 #include "hw/acpi/acpi.h"
 #include "hw/i2c/pm_smbus.h"
-#include "sysemu/reset.h"
 #include "qemu/module.h"
 #include "qemu/timer.h"
 #include "exec/address-spaces.h"
@@ -116,11 +115,10 @@ static const MemoryRegionOps superio_ops = {
 },
 };
 
-static void vt82c686b_reset(void * opaque)
+static void vt82c686b_isa_reset(DeviceState *dev)
 {
-PCIDevice *d = opaque;
-uint8_t *pci_conf = d->config;
-VT82C686BState *vt82c = VT82C686B_DEVICE(d);
+VT82C686BState *vt82c = VT82C686B_DEVICE(dev);
+uint8_t *pci_conf = vt82c->dev.config;
 
 pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
 pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
@@ -476,8 +474,6 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
  * But we do not emulate a floppy, so just set it here. */
 memory_region_add_subregion(isa_bus->address_space_io, 0x3f0,
 >superio);
-
-qemu_register_reset(vt82c686b_reset, d);
 }
 
 ISABus *vt82c686b_isa_init(PCIBus *bus, int devfn)
@@ -501,6 +497,7 @@ static void via_class_init(ObjectClass *klass, void *data)
 k->device_id = PCI_DEVICE_ID_VIA_ISA_BRIDGE;
 k->class_id = PCI_CLASS_BRIDGE_ISA;
 k->revision = 0x40;
+dc->reset = vt82c686b_isa_reset;
 dc->desc = "ISA bridge";
 dc->vmsd = _via;
 /*
-- 
2.21.0

Re: [PATCH v2 1/1] IDE: deprecate ide-drive

[Philippe Mathieu-Daudé]

On 10/10/19 12:43 AM, John Snow wrote:

It's an old compatibility shim that just delegates to ide-cd or ide-hd.
I'd like to refactor these some day, and getting rid of the super-object
will make that easier.

Either way, we don't need this.

Libvirt-checked-by: Peter Krempa 


Peter made a comment regarding Laszlo's Regression-tested-by tag:

  [...] nobody else is using
  this convention (there are exactly 0 instances of
  "Regression-tested-by" in the project git log as far as
  I can see), and so in practice people reading the commits
  won't really know what you meant by it. Everybody else
  on the project uses "Tested-by" to mean either of the
  two cases you describe above, without distinction...

It probably applies to 'Libvirt-checked-by' too.

https://www.mail-archive.com/qemu-devel@nongnu.org/msg632705.html


Signed-off-by: John Snow 
---

[PULL 17/36] qapi: group BlockDeviceStats fields

[Max Reitz]

From: Anton Nefedov 

Make the stat fields definition slightly more readable.
Also reorder total_time_ns stats read-write-flush as done elsewhere.
Cosmetic change only.

Signed-off-by: Anton Nefedov 
Reviewed-by: Alberto Garcia 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20190923121737.83281-2-anton.nefe...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 qapi/block-core.json | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index e6edd641f1..5ab554b54a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -867,12 +867,12 @@
 # @flush_operations: The number of cache flush operations performed by the
 #device (since 0.15.0)
 #
-# @flush_total_time_ns: Total time spend on cache flushes in nano-seconds
-#   (since 0.15.0).
+# @rd_total_time_ns: Total time spent on reads in nanoseconds (since 0.15.0).
 #
-# @wr_total_time_ns: Total time spend on writes in nano-seconds (since 0.15.0).
+# @wr_total_time_ns: Total time spent on writes in nanoseconds (since 0.15.0).
 #
-# @rd_total_time_ns: Total_time_spend on reads in nano-seconds (since 0.15.0).
+# @flush_total_time_ns: Total time spent on cache flushes in nanoseconds
+#   (since 0.15.0).
 #
 # @wr_highest_offset: The offset after the greatest byte written to the
 # device.  The intended use of this information is for
@@ -925,14 +925,18 @@
 # Since: 0.14.0
 ##
 { 'struct': 'BlockDeviceStats',
-  'data': {'rd_bytes': 'int', 'wr_bytes': 'int', 'rd_operations': 'int',
-   'wr_operations': 'int', 'flush_operations': 'int',
-   'flush_total_time_ns': 'int', 'wr_total_time_ns': 'int',
-   'rd_total_time_ns': 'int', 'wr_highest_offset': 'int',
-   'rd_merged': 'int', 'wr_merged': 'int', '*idle_time_ns': 'int',
+  'data': {'rd_bytes': 'int', 'wr_bytes': 'int',
+   'rd_operations': 'int', 'wr_operations': 'int',
+   'flush_operations': 'int',
+   'rd_total_time_ns': 'int', 'wr_total_time_ns': 'int',
+   'flush_total_time_ns': 'int',
+   'wr_highest_offset': 'int',
+   'rd_merged': 'int', 'wr_merged': 'int',
+   '*idle_time_ns': 'int',
'failed_rd_operations': 'int', 'failed_wr_operations': 'int',
-   'failed_flush_operations': 'int', 'invalid_rd_operations': 'int',
-   'invalid_wr_operations': 'int', 'invalid_flush_operations': 'int',
+   'failed_flush_operations': 'int',
+   'invalid_rd_operations': 'int', 'invalid_wr_operations': 'int',
+   'invalid_flush_operations': 'int',
'account_invalid': 'bool', 'account_failed': 'bool',
'timed_stats': ['BlockDeviceTimedStats'],
'*rd_latency_histogram': 'BlockLatencyHistogramInfo',
-- 
2.21.0

[PULL 13/36] block: teach bdrv_debug_breakpoint skip filters with backing

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Teach bdrv_debug_breakpoint and bdrv_debug_remove_breakpoint skip
filters with backing. This is needed to implement and use in backup job
it's own backup_top filter driver (like mirror already has one), and
without this improvement, breakpoint removal will fail at least in 55
iotest.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Message-id: 20190920142056.12778-9-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block.c | 34 ++
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index 5944124845..1c7c199849 100644
--- a/block.c
+++ b/block.c
@@ -5164,14 +5164,35 @@ void bdrv_debug_event(BlockDriverState *bs, 
BlkdebugEvent event)
 bs->drv->bdrv_debug_event(bs, event);
 }
 
-int bdrv_debug_breakpoint(BlockDriverState *bs, const char *event,
-  const char *tag)
+static BlockDriverState *bdrv_find_debug_node(BlockDriverState *bs)
 {
 while (bs && bs->drv && !bs->drv->bdrv_debug_breakpoint) {
-bs = bs->file ? bs->file->bs : NULL;
+if (bs->file) {
+bs = bs->file->bs;
+continue;
+}
+
+if (bs->drv->is_filter && bs->backing) {
+bs = bs->backing->bs;
+continue;
+}
+
+break;
 }
 
 if (bs && bs->drv && bs->drv->bdrv_debug_breakpoint) {
+assert(bs->drv->bdrv_debug_remove_breakpoint);
+return bs;
+}
+
+return NULL;
+}
+
+int bdrv_debug_breakpoint(BlockDriverState *bs, const char *event,
+  const char *tag)
+{
+bs = bdrv_find_debug_node(bs);
+if (bs) {
 return bs->drv->bdrv_debug_breakpoint(bs, event, tag);
 }
 
@@ -5180,11 +5201,8 @@ int bdrv_debug_breakpoint(BlockDriverState *bs, const 
char *event,
 
 int bdrv_debug_remove_breakpoint(BlockDriverState *bs, const char *tag)
 {
-while (bs && bs->drv && !bs->drv->bdrv_debug_remove_breakpoint) {
-bs = bs->file ? bs->file->bs : NULL;
-}
-
-if (bs && bs->drv && bs->drv->bdrv_debug_remove_breakpoint) {
+bs = bdrv_find_debug_node(bs);
+if (bs) {
 return bs->drv->bdrv_debug_remove_breakpoint(bs, tag);
 }
 
-- 
2.21.0

[PULL 23/36] scsi: account unmap operations

[Max Reitz]

From: Anton Nefedov 

Signed-off-by: Anton Nefedov 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20190923121737.83281-8-anton.nefe...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 hw/scsi/scsi-disk.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index a002fdabe8..68b1675fd9 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1617,10 +1617,16 @@ static void scsi_unmap_complete_noio(UnmapCBData *data, 
int ret)
 r->sector_count = (ldl_be_p(>inbuf[8]) & 0xULL)
 * (s->qdev.blocksize / BDRV_SECTOR_SIZE);
 if (!check_lba_range(s, r->sector, r->sector_count)) {
+block_acct_invalid(blk_get_stats(s->qdev.conf.blk),
+   BLOCK_ACCT_UNMAP);
 scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
 goto done;
 }
 
+block_acct_start(blk_get_stats(s->qdev.conf.blk), >acct,
+ r->sector_count * BDRV_SECTOR_SIZE,
+ BLOCK_ACCT_UNMAP);
+
 r->req.aiocb = blk_aio_pdiscard(s->qdev.conf.blk,
 r->sector * BDRV_SECTOR_SIZE,
 r->sector_count * BDRV_SECTOR_SIZE,
@@ -1647,10 +1653,11 @@ static void scsi_unmap_complete(void *opaque, int ret)
 r->req.aiocb = NULL;
 
 aio_context_acquire(blk_get_aio_context(s->qdev.conf.blk));
-if (scsi_disk_req_check_error(r, ret, false)) {
+if (scsi_disk_req_check_error(r, ret, true)) {
 scsi_req_unref(>req);
 g_free(data);
 } else {
+block_acct_done(blk_get_stats(s->qdev.conf.blk), >acct);
 scsi_unmap_complete_noio(data, ret);
 }
 aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
@@ -1682,6 +1689,7 @@ static void scsi_disk_emulate_unmap(SCSIDiskReq *r, 
uint8_t *inbuf)
 }
 
 if (blk_is_read_only(s->qdev.conf.blk)) {
+block_acct_invalid(blk_get_stats(s->qdev.conf.blk), BLOCK_ACCT_UNMAP);
 scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED));
 return;
 }
@@ -1697,10 +1705,12 @@ static void scsi_disk_emulate_unmap(SCSIDiskReq *r, 
uint8_t *inbuf)
 return;
 
 invalid_param_len:
+block_acct_invalid(blk_get_stats(s->qdev.conf.blk), BLOCK_ACCT_UNMAP);
 scsi_check_condition(r, SENSE_CODE(INVALID_PARAM_LEN));
 return;
 
 invalid_field:
+block_acct_invalid(blk_get_stats(s->qdev.conf.blk), BLOCK_ACCT_UNMAP);
 scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
 }
 
-- 
2.21.0

[PULL 26/36] iotests: Fix 125 for growth_mode = metadata

[Max Reitz]

If we use growth_mode = metadata, it is very much possible that the file
uses more disk space after we have written something to the added area.
We did indeed want to test for this case, but unfortunately we evidently
just copied the code from the "Test creation preallocation" section and
forgot to replace "$create_mode" by "$growth_mode".

We never noticed because we only read the first number from qemu-img
info's "disk size" output -- and that is effectively useless, because
qemu-img prints a human-readable value (which generally includes a
decimal point).  That will be fixed in the patch after the next one.

Signed-off-by: Max Reitz 
Message-id: 20190925183231.11196-2-mre...@redhat.com
Reviewed-by: Eric Blake 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/125 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/125 b/tests/qemu-iotests/125
index dc4b8f5fb9..df328a63a6 100755
--- a/tests/qemu-iotests/125
+++ b/tests/qemu-iotests/125
@@ -111,7 +111,7 @@ for GROWTH_SIZE in 16 48 80; do
 if [ $file_length_2 -gt $file_length_1 ]; then
 echo "ERROR (grow): Image length has grown from 
$file_length_1 to $file_length_2"
 fi
-if [ $create_mode != metadata ]; then
+if [ $growth_mode != metadata ]; then
 # The host size should not have grown either
 if [ $host_size_2 -gt $host_size_1 ]; then
 echo "ERROR (grow): Host size has grown from 
$host_size_1 to $host_size_2"
-- 
2.21.0

[PATCH v3 0/8] hw: Convert various reset() handler to DeviceReset

[Philippe Mathieu-Daudé]

Only patch 3/8 is missing review:
- hw/ide/piix: Convert reset handler to DeviceReset

Since v2:
- Fixed PIIX_IDE conversion (Li)
- Added more R-b tag.

Since v1:
- Removed the pci-host devices
- Removed the vmcoreinfo conversion (elmarco) but add a comment.
- Added Igor's R-b tag.

Following the thread discussion between Peter/Markus/Damien about
reset handlers:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg617103.html
I started to remove qemu_register_reset() calls from few qdevified
devices (the trivial ones).

Regards,

Phil.

v2: https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg01677.html
v1: https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg06367.html

Philippe Mathieu-Daudé (8):
  hw/acpi/piix4: Convert reset handler to DeviceReset
  hw/isa/piix4: Convert reset handler to DeviceReset
  hw/ide/piix: Convert reset handler to DeviceReset
  hw/ide/sii3112: Convert reset handler to DeviceReset
  hw/ide/via82c: Convert reset handler to DeviceReset
  hw/isa/vt82c686: Convert reset handler to DeviceReset
  hw/input/lm832x: Convert reset handler to DeviceReset
  hw/misc/vmcoreinfo: Add comment about reset handler

 hw/acpi/piix4.c  |  7 +++
 hw/ide/piix.c|  9 -
 hw/ide/sii3112.c |  7 +++
 hw/ide/via.c | 10 --
 hw/input/lm832x.c| 12 +---
 hw/isa/piix4.c   |  7 +++
 hw/isa/vt82c686.c| 11 ---
 hw/misc/vmcoreinfo.c |  4 
 8 files changed, 30 insertions(+), 37 deletions(-)

-- 
2.21.0

[PULL 09/36] block/backup: improve comment about image fleecing

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Message-id: 20190920142056.12778-5-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block/backup.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 98d7f7a905..ae28849660 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -747,9 +747,18 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 job->bitmap_mode = bitmap_mode;
 
 /*
- * Set write flags:
- * 1. Detect image-fleecing (and similar) schemes
- * 2. Handle compression
+ * If source is in backing chain of target assume that target is going to 
be
+ * used for "image fleecing", i.e. it should represent a kind of snapshot 
of
+ * source at backup-start point in time. And target is going to be read by
+ * somebody (for example, used as NBD export) during backup job.
+ *
+ * In this case, we need to add BDRV_REQ_SERIALISING write flag to avoid
+ * intersection of backup writes and third party reads from target,
+ * otherwise reading from target we may occasionally read already updated 
by
+ * guest data.
+ *
+ * For more information see commit f8d59dfb40bb and test
+ * tests/qemu-iotests/222
  */
 job->write_flags =
 (bdrv_chain_contains(target, bs) ? BDRV_REQ_SERIALISING : 0) |
-- 
2.21.0

[PULL 08/36] block/backup: split shareable copying part from backup_do_cow

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Split copying logic which will be shared with backup-top filter.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Message-id: 20190920142056.12778-4-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block/backup.c | 47 ---
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 99177f03f8..98d7f7a905 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -248,26 +248,18 @@ static int64_t 
backup_bitmap_reset_unallocated(BackupBlockJob *s,
 return ret;
 }
 
-static int coroutine_fn backup_do_cow(BackupBlockJob *job,
-  int64_t offset, uint64_t bytes,
-  bool *error_is_read,
-  bool is_write_notifier)
+static int coroutine_fn backup_do_copy(BackupBlockJob *job,
+   int64_t start, uint64_t bytes,
+   bool *error_is_read,
+   bool is_write_notifier)
 {
-CowRequest cow_request;
 int ret = 0;
-int64_t start, end; /* bytes */
+int64_t end = bytes + start; /* bytes */
 void *bounce_buffer = NULL;
 int64_t status_bytes;
 
-qemu_co_rwlock_rdlock(>flush_rwlock);
-
-start = QEMU_ALIGN_DOWN(offset, job->cluster_size);
-end = QEMU_ALIGN_UP(bytes + offset, job->cluster_size);
-
-trace_backup_do_cow_enter(job, start, offset, bytes);
-
-wait_for_overlapping_requests(job, start, end);
-cow_request_begin(_request, job, start, end);
+assert(QEMU_IS_ALIGNED(start, job->cluster_size));
+assert(QEMU_IS_ALIGNED(end, job->cluster_size));
 
 while (start < end) {
 int64_t dirty_end;
@@ -326,6 +318,31 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
 qemu_vfree(bounce_buffer);
 }
 
+return ret;
+}
+
+static int coroutine_fn backup_do_cow(BackupBlockJob *job,
+  int64_t offset, uint64_t bytes,
+  bool *error_is_read,
+  bool is_write_notifier)
+{
+CowRequest cow_request;
+int ret = 0;
+int64_t start, end; /* bytes */
+
+qemu_co_rwlock_rdlock(>flush_rwlock);
+
+start = QEMU_ALIGN_DOWN(offset, job->cluster_size);
+end = QEMU_ALIGN_UP(bytes + offset, job->cluster_size);
+
+trace_backup_do_cow_enter(job, start, offset, bytes);
+
+wait_for_overlapping_requests(job, start, end);
+cow_request_begin(_request, job, start, end);
+
+ret = backup_do_copy(job, start, end - start, error_is_read,
+ is_write_notifier);
+
 cow_request_end(_request);
 
 trace_backup_do_cow_return(job, offset, bytes, ret);
-- 
2.21.0

[PULL 19/36] block: add empty account cookie type

[Max Reitz]

From: Anton Nefedov 

Each block_acct_done/failed call is designed to correspond to a
previous block_acct_start call, which initializes the stats cookie.
However sometimes it is not the case, e.g. some error paths might
report the same cookie twice because it is hard to accurately track if
the cookie was reported yet or not.

This patch cleans the cookie after report.
(Note: block_acct_failed/done without a previous block_acct_start at
all should be avoided. Uninitialized cookie might hold a garbage value
and there is still "< BLOCK_MAX_IOTYPE" assertion for that)

It will be particularly useful in ide code where it's hard to
keep track whether the request done its accounting or not: in the
following patch of the series, trim requests will do the accounting
separately.

Signed-off-by: Anton Nefedov 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20190923121737.83281-4-anton.nefe...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 include/block/accounting.h | 1 +
 block/accounting.c | 6 ++
 2 files changed, 7 insertions(+)

diff --git a/include/block/accounting.h b/include/block/accounting.h
index ba8b04d572..878b4c3581 100644
--- a/include/block/accounting.h
+++ b/include/block/accounting.h
@@ -33,6 +33,7 @@ typedef struct BlockAcctTimedStats BlockAcctTimedStats;
 typedef struct BlockAcctStats BlockAcctStats;
 
 enum BlockAcctType {
+BLOCK_ACCT_NONE = 0,
 BLOCK_ACCT_READ,
 BLOCK_ACCT_WRITE,
 BLOCK_ACCT_FLUSH,
diff --git a/block/accounting.c b/block/accounting.c
index 70a3d9a426..8d41c8a83a 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -195,6 +195,10 @@ static void block_account_one_io(BlockAcctStats *stats, 
BlockAcctCookie *cookie,
 
 assert(cookie->type < BLOCK_MAX_IOTYPE);
 
+if (cookie->type == BLOCK_ACCT_NONE) {
+return;
+}
+
 qemu_mutex_lock(>lock);
 
 if (failed) {
@@ -217,6 +221,8 @@ static void block_account_one_io(BlockAcctStats *stats, 
BlockAcctCookie *cookie,
 }
 
 qemu_mutex_unlock(>lock);
+
+cookie->type = BLOCK_ACCT_NONE;
 }
 
 void block_acct_done(BlockAcctStats *stats, BlockAcctCookie *cookie)
-- 
2.21.0

[PULL 12/36] block: move block_copy from block/backup.c to separate file

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Split block_copy to separate file, to be cleanly shared with backup-top
filter driver in further commits.

It's a clean movement, the only change is drop "static" from interface
functions.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Message-id: 20190920142056.12778-8-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block/Makefile.objs|   1 +
 include/block/block-copy.h |  76 
 block/backup.c | 355 +
 block/block-copy.c | 333 ++
 block/trace-events |   2 +
 5 files changed, 413 insertions(+), 354 deletions(-)
 create mode 100644 include/block/block-copy.h
 create mode 100644 block/block-copy.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index c2eb8c8769..f06f1fa1ac 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -37,6 +37,7 @@ block-obj-y += write-threshold.o
 block-obj-y += backup.o
 block-obj-$(CONFIG_REPLICATION) += replication.o
 block-obj-y += throttle.o copy-on-read.o
+block-obj-y += block-copy.o
 
 block-obj-y += crypto.o
 
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
new file mode 100644
index 00..54f90d0c9a
--- /dev/null
+++ b/include/block/block-copy.h
@@ -0,0 +1,76 @@
+/*
+ * block_copy API
+ *
+ * Copyright (C) 2013 Proxmox Server Solutions
+ * Copyright (c) 2019 Virtuozzo International GmbH.
+ *
+ * Authors:
+ *  Dietmar Maurer (diet...@proxmox.com)
+ *  Vladimir Sementsov-Ogievskiy 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef BLOCK_COPY_H
+#define BLOCK_COPY_H
+
+#include "block/block.h"
+
+typedef void (*ProgressBytesCallbackFunc)(int64_t bytes, void *opaque);
+typedef void (*ProgressResetCallbackFunc)(void *opaque);
+typedef struct BlockCopyState {
+BlockBackend *source;
+BlockBackend *target;
+BdrvDirtyBitmap *copy_bitmap;
+int64_t cluster_size;
+bool use_copy_range;
+int64_t copy_range_size;
+uint64_t len;
+
+BdrvRequestFlags write_flags;
+
+/*
+ * skip_unallocated:
+ *
+ * Used by sync=top jobs, which first scan the source node for unallocated
+ * areas and clear them in the copy_bitmap.  During this process, the 
bitmap
+ * is thus not fully initialized: It may still have bits set for areas that
+ * are unallocated and should actually not be copied.
+ *
+ * This is indicated by skip_unallocated.
+ *
+ * In this case, block_copy() will query the source’s allocation status,
+ * skip unallocated regions, clear them in the copy_bitmap, and invoke
+ * block_copy_reset_unallocated() every time it does.
+ */
+bool skip_unallocated;
+
+/* progress_bytes_callback: called when some copying progress is done. */
+ProgressBytesCallbackFunc progress_bytes_callback;
+
+/*
+ * progress_reset_callback: called when some bytes reset from copy_bitmap
+ * (see @skip_unallocated above). The callee is assumed to recalculate how
+ * many bytes remain based on the dirty bit count of copy_bitmap.
+ */
+ProgressResetCallbackFunc progress_reset_callback;
+void *progress_opaque;
+} BlockCopyState;
+
+BlockCopyState *block_copy_state_new(
+BlockDriverState *source, BlockDriverState *target,
+int64_t cluster_size, BdrvRequestFlags write_flags,
+ProgressBytesCallbackFunc progress_bytes_callback,
+ProgressResetCallbackFunc progress_reset_callback,
+void *progress_opaque, Error **errp);
+
+void block_copy_state_free(BlockCopyState *s);
+
+int64_t block_copy_reset_unallocated(BlockCopyState *s,
+ int64_t offset, int64_t *count);
+
+int coroutine_fn block_copy(BlockCopyState *s, int64_t start, uint64_t bytes,
+bool *error_is_read, bool is_write_notifier);
+
+#endif /* BLOCK_COPY_H */
diff --git a/block/backup.c b/block/backup.c
index f5125984db..4613b8c88d 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -18,6 +18,7 @@
 #include "block/block_int.h"
 #include "block/blockjob_int.h"
 #include "block/block_backup.h"
+#include "block/block-copy.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
 #include "qemu/ratelimit.h"
@@ -35,47 +36,6 @@ typedef struct CowRequest {
 CoQueue wait_queue; /* coroutines blocked on this request */
 } CowRequest;
 
-typedef void (*ProgressBytesCallbackFunc)(int64_t bytes, void *opaque);
-typedef void (*ProgressResetCallbackFunc)(void *opaque);
-typedef struct BlockCopyState {
-BlockBackend *source;
-BlockBackend *target;
-BdrvDirtyBitmap *copy_bitmap;
-int64_t cluster_size;
-bool use_copy_range;
-int64_t copy_range_size;
-uint64_t len;
-
-BdrvRequestFlags write_flags;
-
-/*
- * skip_unallocated:
- *
- * Used by sync=top jobs, which first scan the source node for 

[PULL 20/36] ide: account UNMAP (TRIM) operations

[Max Reitz]

From: Anton Nefedov 

Signed-off-by: Anton Nefedov 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20190923121737.83281-5-anton.nefe...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 hw/ide/core.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index e6e54c6c9a..754ff4dc34 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -442,6 +442,14 @@ static void ide_issue_trim_cb(void *opaque, int ret)
 TrimAIOCB *iocb = opaque;
 IDEState *s = iocb->s;
 
+if (iocb->i >= 0) {
+if (ret >= 0) {
+block_acct_done(blk_get_stats(s->blk), >acct);
+} else {
+block_acct_failed(blk_get_stats(s->blk), >acct);
+}
+}
+
 if (ret >= 0) {
 while (iocb->j < iocb->qiov->niov) {
 int j = iocb->j;
@@ -459,10 +467,14 @@ static void ide_issue_trim_cb(void *opaque, int ret)
 }
 
 if (!ide_sect_range_ok(s, sector, count)) {
+block_acct_invalid(blk_get_stats(s->blk), 
BLOCK_ACCT_UNMAP);
 iocb->ret = -EINVAL;
 goto done;
 }
 
+block_acct_start(blk_get_stats(s->blk), >acct,
+ count << BDRV_SECTOR_BITS, BLOCK_ACCT_UNMAP);
+
 /* Got an entry! Submit and exit.  */
 iocb->aiocb = blk_aio_pdiscard(s->blk,
sector << BDRV_SECTOR_BITS,
-- 
2.21.0

Re: [PATCH v2 02/11] qcrypto-luks: extend the create options for upcoming encryption key management

[Kevin Wolf]

Am 13.09.2019 um 00:30 hat Maxim Levitsky geschrieben:
> Now you can specify which slot to put the encryption key to
> Plus add 'active' option which will let  user erase the key secret
> instead of adding it.
> Check that active=true it when creating.
> 
> Signed-off-by: Maxim Levitsky 

> diff --git a/qapi/crypto.json b/qapi/crypto.json
> index b2a4cff683..9b83a70634 100644
> --- a/qapi/crypto.json
> +++ b/qapi/crypto.json
> @@ -190,6 +190,20 @@
>  #  Currently defaults to 'sha256'
>  # @hash-alg: the master key hash algorithm
>  #Currently defaults to 'sha256'
> +#
> +# @active: Should the new secret be added (true) or erased (false)
> +#  (amend only, since 4.2)
> +#
> +# @slot: The slot in which to put/erase the secret
> +#if not given, will select first free slot for secret addtion
> +#and erase all matching keyslots for erase. except last one
> +#(optional, since 4.2)
> +#
> +# @unlock-secret: The secret to use to unlock the image
> +#If not given, will use the secret that was used
> +#when opening the image.
> +#(optional, for amend only, since 4.2)
> +#
>  # @iter-time: number of milliseconds to spend in
>  # PBKDF passphrase processing. Currently defaults
>  # to 2000. (since 2.8)

This approach doesn't look right to me. BlockdevCreateOptions should
describe the state of the image after the operation. You're describing
an update instead (and in a way that doesn't allow you to change
everything that you may want to change, so that you need to call the
operation multiple times).

I imagined the syntax of a blockdev-amend QMP command similar to
x-blockdev-reopen: Describe the full set of options that you want to
have in effect after the operation; if you don't want to change some
option, you just specify it again with its old value.

Specifically for luks, this probably means that you have a @slots, which
is a list that contains at least the secret for each slot, or JSON null
for a slot that should be left empty.

With the same approach, you don't have to make 'size' optional in later
patches, you can just require that the current size is re-specified. And
later, blockdev-amend could actually allow changing the size of images
if you provide a different value.

Kevin

Re: [PATCH v2 3/4] qemu-iotests: 044: pass is actually a noop, so remove it

[Philippe Mathieu-Daudé]

On 10/9/19 9:47 PM, Cleber Rosa wrote:

Reviewed-by: Eric Blake 
Signed-off-by: Cleber Rosa 
---
  tests/qemu-iotests/044 | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/tests/qemu-iotests/044 b/tests/qemu-iotests/044
index 8b2afa2a11..aa2a00ceed 100755
--- a/tests/qemu-iotests/044
+++ b/tests/qemu-iotests/044
@@ -102,17 +102,14 @@ class TestRefcountTableGrowth(iotests.QMPTestCase):
  def setUp(self):
  qemu_img('create', '-f', iotests.imgfmt, '-o', 'cluster_size=512', 
test_img, '16G')
  self.preallocate(test_img)
-pass
  
  
  def tearDown(self):

  os.remove(test_img)
-pass
  
  def test_grow_refcount_table(self):

  qemu_io('-c', 'write 3800M 1M', test_img)
  qemu_img_verbose('check' , test_img)
-pass
  
  if __name__ == '__main__':

  iotests.main(supported_fmts=['qcow2'],



Reviewed-by: Philippe Mathieu-Daudé 

[PULL 24/36] file-posix: account discard operations

[Max Reitz]

From: Anton Nefedov 

This will help to identify how many of the user-issued discard operations
(accounted on a device level) have actually suceeded down on the host file
(even though the numbers will not be exactly the same if non-raw format
driver is used (e.g. qcow2 sending metadata discards)).

Note that these numbers will not include discards triggered by
write-zeroes + MAY_UNMAP calls.

Signed-off-by: Anton Nefedov 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20190923121737.83281-9-anton.nefe...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block/file-posix.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index f12c06de2d..f3934c4e10 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -161,6 +161,11 @@ typedef struct BDRVRawState {
 bool needs_alignment;
 bool drop_cache;
 bool check_cache_dropped;
+struct {
+uint64_t discard_nb_ok;
+uint64_t discard_nb_failed;
+uint64_t discard_bytes_ok;
+} stats;
 
 PRManager *pr_mgr;
 } BDRVRawState;
@@ -2660,11 +2665,22 @@ static void coroutine_fn 
raw_co_invalidate_cache(BlockDriverState *bs,
 #endif /* !__linux__ */
 }
 
+static void raw_account_discard(BDRVRawState *s, uint64_t nbytes, int ret)
+{
+if (ret) {
+s->stats.discard_nb_failed++;
+} else {
+s->stats.discard_nb_ok++;
+s->stats.discard_bytes_ok += nbytes;
+}
+}
+
 static coroutine_fn int
 raw_do_pdiscard(BlockDriverState *bs, int64_t offset, int bytes, bool blkdev)
 {
 BDRVRawState *s = bs->opaque;
 RawPosixAIOData acb;
+int ret;
 
 acb = (RawPosixAIOData) {
 .bs = bs,
@@ -2678,7 +2694,9 @@ raw_do_pdiscard(BlockDriverState *bs, int64_t offset, int 
bytes, bool blkdev)
 acb.aio_type |= QEMU_AIO_BLKDEV;
 }
 
-return raw_thread_pool_submit(bs, handle_aiocb_discard, );
+ret = raw_thread_pool_submit(bs, handle_aiocb_discard, );
+raw_account_discard(s, bytes, ret);
+return ret;
 }
 
 static coroutine_fn int
@@ -3301,10 +3319,12 @@ static int fd_open(BlockDriverState *bs)
 static coroutine_fn int
 hdev_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
 {
+BDRVRawState *s = bs->opaque;
 int ret;
 
 ret = fd_open(bs);
 if (ret < 0) {
+raw_account_discard(s, bytes, ret);
 return ret;
 }
 return raw_do_pdiscard(bs, offset, bytes, true);
-- 
2.21.0

[PULL 05/36] block/qcow2: introduce parallel subrequest handling in read and write

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

It improves performance for fragmented qcow2 images.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20190916175324.18478-6-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block/qcow2.h  |   3 ++
 block/qcow2.c  | 125 -
 block/trace-events |   1 +
 3 files changed, 117 insertions(+), 12 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index a488d761ff..f51f478e34 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -65,6 +65,9 @@
 #define QCOW2_MAX_BITMAPS 65535
 #define QCOW2_MAX_BITMAP_DIRECTORY_SIZE (1024 * QCOW2_MAX_BITMAPS)
 
+/* Maximum of parallel sub-request per guest request */
+#define QCOW2_MAX_WORKERS 8
+
 /* indicate that the refcount of the referenced cluster is exactly one. */
 #define QCOW_OFLAG_COPIED (1ULL << 63)
 /* indicate that the cluster is compressed (they never have the copied flag) */
diff --git a/block/qcow2.c b/block/qcow2.c
index 164b22e4a2..7961c05783 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -41,6 +41,7 @@
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/qapi-visit-block-core.h"
 #include "crypto.h"
+#include "block/aio_task.h"
 
 /*
   Differences with QCOW:
@@ -2025,6 +2026,60 @@ fail:
 return ret;
 }
 
+typedef struct Qcow2AioTask {
+AioTask task;
+
+BlockDriverState *bs;
+QCow2ClusterType cluster_type; /* only for read */
+uint64_t file_cluster_offset;
+uint64_t offset;
+uint64_t bytes;
+QEMUIOVector *qiov;
+uint64_t qiov_offset;
+QCowL2Meta *l2meta; /* only for write */
+} Qcow2AioTask;
+
+static coroutine_fn int qcow2_co_preadv_task_entry(AioTask *task);
+static coroutine_fn int qcow2_add_task(BlockDriverState *bs,
+   AioTaskPool *pool,
+   AioTaskFunc func,
+   QCow2ClusterType cluster_type,
+   uint64_t file_cluster_offset,
+   uint64_t offset,
+   uint64_t bytes,
+   QEMUIOVector *qiov,
+   size_t qiov_offset,
+   QCowL2Meta *l2meta)
+{
+Qcow2AioTask local_task;
+Qcow2AioTask *task = pool ? g_new(Qcow2AioTask, 1) : _task;
+
+*task = (Qcow2AioTask) {
+.task.func = func,
+.bs = bs,
+.cluster_type = cluster_type,
+.qiov = qiov,
+.file_cluster_offset = file_cluster_offset,
+.offset = offset,
+.bytes = bytes,
+.qiov_offset = qiov_offset,
+.l2meta = l2meta,
+};
+
+trace_qcow2_add_task(qemu_coroutine_self(), bs, pool,
+ func == qcow2_co_preadv_task_entry ? "read" : "write",
+ cluster_type, file_cluster_offset, offset, bytes,
+ qiov, qiov_offset);
+
+if (!pool) {
+return func(>task);
+}
+
+aio_task_pool_start_task(pool, >task);
+
+return 0;
+}
+
 static coroutine_fn int qcow2_co_preadv_task(BlockDriverState *bs,
  QCow2ClusterType cluster_type,
  uint64_t file_cluster_offset,
@@ -2074,18 +2129,28 @@ static coroutine_fn int 
qcow2_co_preadv_task(BlockDriverState *bs,
 g_assert_not_reached();
 }
 
+static coroutine_fn int qcow2_co_preadv_task_entry(AioTask *task)
+{
+Qcow2AioTask *t = container_of(task, Qcow2AioTask, task);
+
+assert(!t->l2meta);
+
+return qcow2_co_preadv_task(t->bs, t->cluster_type, t->file_cluster_offset,
+t->offset, t->bytes, t->qiov, t->qiov_offset);
+}
+
 static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,
  uint64_t offset, uint64_t bytes,
  QEMUIOVector *qiov,
  size_t qiov_offset, int flags)
 {
 BDRVQcow2State *s = bs->opaque;
-int ret;
+int ret = 0;
 unsigned int cur_bytes; /* number of bytes in current iteration */
 uint64_t cluster_offset = 0;
+AioTaskPool *aio = NULL;
 
-while (bytes != 0) {
-
+while (bytes != 0 && aio_task_pool_status(aio) == 0) {
 /* prepare next request */
 cur_bytes = MIN(bytes, INT_MAX);
 if (s->crypto) {
@@ -2097,7 +2162,7 @@ static coroutine_fn int 
qcow2_co_preadv_part(BlockDriverState *bs,
 ret = qcow2_get_cluster_offset(bs, offset, _bytes, 
_offset);
 qemu_co_mutex_unlock(>lock);
 if (ret < 0) {
-return ret;
+goto out;
 }
 
 if (ret == QCOW2_CLUSTER_ZERO_PLAIN ||
@@ -2106,11 +2171,14 @@ static coroutine_fn int 
qcow2_co_preadv_part(BlockDriverState *bs,
 {
 qemu_iovec_memset(qiov, qiov_offset, 0, cur_bytes);
 } else {
-

[PULL 33/36] block/backup: use backup-top instead of write notifiers

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Drop write notifiers and use filter node instead.

= Changes =

1. Add filter-node-name argument for backup qmp api. We have to do it
in this commit, as 257 needs to be fixed.

2. There are no more write notifiers here, so is_write_notifier
parameter is dropped from block-copy paths.

3. To sync with in-flight requests at job finish we now have drained
removing of the filter, we don't need rw-lock.

4. Block-copy is now using BdrvChildren instead of BlockBackends

5. As backup-top owns these children, we also move block-copy state
into backup-top's ownership.

= Iotest changes =

56: op-blocker doesn't shoot now, as we set it on source, but then
check on filter, when trying to start second backup.
To keep the test we instead can catch another collision: both jobs will
get 'drive0' job-id, as job-id parameter is unspecified. To prevent
interleaving with file-posix locks (as they are dependent on config)
let's use another target for second backup.

Also, it's obvious now that we'd like to drop this op-blocker at all
and add a test-case for two backups from one node (to different
destinations) actually works. But not in these series.

141: Output changed: prepatch, "Node is in use" comes from bdrv_has_blk
check inside qmp_blockdev_del. But we've dropped block-copy blk
objects, so no more blk objects on source bs (job blk is on backup-top
filter bs). New message is from op-blocker, which is the next check in
qmp_blockdev_add.

257: The test wants to emulate guest write during backup. They should
go to filter node, not to original source node, of course. Therefore we
need to specify filter node name and use it.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20191001131409.14202-6-vsement...@virtuozzo.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 qapi/block-core.json   |   8 +-
 include/block/block-copy.h |  14 +-
 include/block/block_int.h  |   1 +
 block/backup-top.c |  21 +--
 block/backup.c |  73 +++--
 block/block-copy.c |  81 +++---
 block/replication.c|   2 +-
 blockdev.c |   1 +
 tests/qemu-iotests/056 |   8 +-
 tests/qemu-iotests/141.out |   2 +-
 tests/qemu-iotests/257 |   7 +-
 tests/qemu-iotests/257.out | 306 ++---
 12 files changed, 237 insertions(+), 287 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 859acea014..f66553aac7 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1450,6 +1450,11 @@
 #list without user intervention.
 #Defaults to true. (Since 2.12)
 #
+# @filter-node-name: the node name that should be assigned to the
+#filter driver that the backup job inserts into the graph
+#above node specified by @drive. If this option is not 
given,
+#a node name is autogenerated. (Since: 4.2)
+#
 # Note: @on-source-error and @on-target-error only affect background
 # I/O.  If an error occurs during a guest write request, the device's
 # rerror/werror actions will be used.
@@ -1463,7 +1468,8 @@
 '*compress': 'bool',
 '*on-source-error': 'BlockdevOnError',
 '*on-target-error': 'BlockdevOnError',
-'*auto-finalize': 'bool', '*auto-dismiss': 'bool' } }
+'*auto-finalize': 'bool', '*auto-dismiss': 'bool',
+'*filter-node-name': 'str' } }
 
 ##
 # @DriveBackup:
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index 340d856246..e2e135ff1b 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -27,8 +27,13 @@ typedef struct BlockCopyInFlightReq {
 typedef void (*ProgressBytesCallbackFunc)(int64_t bytes, void *opaque);
 typedef void (*ProgressResetCallbackFunc)(void *opaque);
 typedef struct BlockCopyState {
-BlockBackend *source;
-BlockBackend *target;
+/*
+ * BdrvChild objects are not owned or managed by block-copy. They are
+ * provided by block-copy user and user is responsible for appropriate
+ * permissions on these children.
+ */
+BdrvChild *source;
+BdrvChild *target;
 BdrvDirtyBitmap *copy_bitmap;
 int64_t cluster_size;
 bool use_copy_range;
@@ -66,8 +71,7 @@ typedef struct BlockCopyState {
 void *progress_opaque;
 } BlockCopyState;
 
-BlockCopyState *block_copy_state_new(BlockDriverState *source,
- BlockDriverState *target,
+BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
  int64_t cluster_size,
  BdrvRequestFlags write_flags,
  Error **errp);
@@ -84,6 +88,6 @@ int64_t block_copy_reset_unallocated(BlockCopyState *s,
  int64_t offset, int64_t *count);
 
 int coroutine_fn block_copy(BlockCopyState *s, int64_t start, uint64_t bytes,
-   

[PULL 18/36] qapi: add unmap to BlockDeviceStats

[Max Reitz]

From: Anton Nefedov 

Signed-off-by: Anton Nefedov 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Message-id: 20190923121737.83281-3-anton.nefe...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 qapi/block-core.json   | 29 +++--
 include/block/accounting.h |  1 +
 block/qapi.c   |  6 ++
 tests/qemu-iotests/227.out | 18 ++
 4 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5ab554b54a..7d3e05891c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -860,6 +860,8 @@
 #
 # @wr_bytes:  The number of bytes written by the device.
 #
+# @unmap_bytes: The number of bytes unmapped by the device (Since 4.2)
+#
 # @rd_operations: The number of read operations performed by the device.
 #
 # @wr_operations: The number of write operations performed by the device.
@@ -867,6 +869,9 @@
 # @flush_operations: The number of cache flush operations performed by the
 #device (since 0.15.0)
 #
+# @unmap_operations: The number of unmap operations performed by the device
+#(Since 4.2)
+#
 # @rd_total_time_ns: Total time spent on reads in nanoseconds (since 0.15.0).
 #
 # @wr_total_time_ns: Total time spent on writes in nanoseconds (since 0.15.0).
@@ -874,6 +879,9 @@
 # @flush_total_time_ns: Total time spent on cache flushes in nanoseconds
 #   (since 0.15.0).
 #
+# @unmap_total_time_ns: Total time spent on unmap operations in nanoseconds
+#   (Since 4.2)
+#
 # @wr_highest_offset: The offset after the greatest byte written to the
 # device.  The intended use of this information is for
 # growable sparse files (like qcow2) that are used on top
@@ -885,6 +893,9 @@
 # @wr_merged: Number of write requests that have been merged into another
 # request (Since 2.3).
 #
+# @unmap_merged: Number of unmap requests that have been merged into another
+#request (Since 4.2)
+#
 # @idle_time_ns: Time since the last I/O operation, in
 #nanoseconds. If the field is absent it means that
 #there haven't been any operations yet (Since 2.5).
@@ -898,6 +909,9 @@
 # @failed_flush_operations: The number of failed flush operations
 #   performed by the device (Since 2.5)
 #
+# @failed_unmap_operations: The number of failed unmap operations performed
+#   by the device (Since 4.2)
+#
 # @invalid_rd_operations: The number of invalid read operations
 #  performed by the device (Since 2.5)
 #
@@ -907,6 +921,9 @@
 # @invalid_flush_operations: The number of invalid flush operations
 #performed by the device (Since 2.5)
 #
+# @invalid_unmap_operations: The number of invalid unmap operations performed
+#by the device (Since 4.2)
+#
 # @account_invalid: Whether invalid operations are included in the
 #   last access statistics (Since 2.5)
 #
@@ -925,18 +942,18 @@
 # Since: 0.14.0
 ##
 { 'struct': 'BlockDeviceStats',
-  'data': {'rd_bytes': 'int', 'wr_bytes': 'int',
+  'data': {'rd_bytes': 'int', 'wr_bytes': 'int', 'unmap_bytes' : 'int',
'rd_operations': 'int', 'wr_operations': 'int',
-   'flush_operations': 'int',
+   'flush_operations': 'int', 'unmap_operations': 'int',
'rd_total_time_ns': 'int', 'wr_total_time_ns': 'int',
-   'flush_total_time_ns': 'int',
+   'flush_total_time_ns': 'int', 'unmap_total_time_ns': 'int',
'wr_highest_offset': 'int',
-   'rd_merged': 'int', 'wr_merged': 'int',
+   'rd_merged': 'int', 'wr_merged': 'int', 'unmap_merged': 'int',
'*idle_time_ns': 'int',
'failed_rd_operations': 'int', 'failed_wr_operations': 'int',
-   'failed_flush_operations': 'int',
+   'failed_flush_operations': 'int', 'failed_unmap_operations': 'int',
'invalid_rd_operations': 'int', 'invalid_wr_operations': 'int',
-   'invalid_flush_operations': 'int',
+   'invalid_flush_operations': 'int', 'invalid_unmap_operations': 
'int',
'account_invalid': 'bool', 'account_failed': 'bool',
'timed_stats': ['BlockDeviceTimedStats'],
'*rd_latency_histogram': 'BlockLatencyHistogramInfo',
diff --git a/include/block/accounting.h b/include/block/accounting.h
index d1f67b10dd..ba8b04d572 100644
--- a/include/block/accounting.h
+++ b/include/block/accounting.h
@@ -36,6 +36,7 @@ enum BlockAcctType {
 BLOCK_ACCT_READ,
 BLOCK_ACCT_WRITE,
 BLOCK_ACCT_FLUSH,
+BLOCK_ACCT_UNMAP,
 BLOCK_MAX_IOTYPE,
 };
 
diff --git a/block/qapi.c b/block/qapi.c
index 7ee2ee065d..69c35c4196 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -440,24 +440,30 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, 

[PATCH v3 1/8] hw/acpi/piix4: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

The PIIX4/PM is a PCI device within the PIIX4 chipset, it will be reset
when the PCI bus it stands on is reset.

Convert its reset handler into a proper Device reset method.

Reviewed-by: Igor Mammedov 
Reviewed-by: Li Qiang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/acpi/piix4.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 5742c3df87..4e079b39bd 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -27,7 +27,6 @@
 #include "hw/pci/pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/acpi/acpi.h"
-#include "sysemu/reset.h"
 #include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
 #include "qapi/error.h"
@@ -344,9 +343,9 @@ static const VMStateDescription vmstate_acpi = {
 }
 };
 
-static void piix4_reset(void *opaque)
+static void piix4_pm_reset(DeviceState *dev)
 {
-PIIX4PMState *s = opaque;
+PIIX4PMState *s = PIIX4_PM(dev);
 PCIDevice *d = PCI_DEVICE(s);
 uint8_t *pci_conf = d->config;
 
@@ -542,7 +541,6 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp)
 
 s->machine_ready.notify = piix4_pm_machine_ready;
 qemu_add_machine_init_done_notifier(>machine_ready);
-qemu_register_reset(piix4_reset, s);
 
 piix4_acpi_system_hot_add_init(pci_address_space_io(dev),
pci_get_bus(dev), s);
@@ -692,6 +690,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void 
*data)
 k->device_id = PCI_DEVICE_ID_INTEL_82371AB_3;
 k->revision = 0x03;
 k->class_id = PCI_CLASS_BRIDGE_OTHER;
+dc->reset = piix4_pm_reset;
 dc->desc = "PM";
 dc->vmsd = _acpi;
 dc->props = piix4_pm_properties;
-- 
2.21.0

[PULL 04/36] block/qcow2: refactor qcow2_co_pwritev_part

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Similarly to previous commit, prepare for parallelizing write-loop
iterations.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Message-id: 20190916175324.18478-5-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block/qcow2.c | 154 +-
 1 file changed, 90 insertions(+), 64 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 6feb169f7c..164b22e4a2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2242,6 +2242,88 @@ static int handle_alloc_space(BlockDriverState *bs, 
QCowL2Meta *l2meta)
 return 0;
 }
 
+/*
+ * qcow2_co_pwritev_task
+ * Called with s->lock unlocked
+ * l2meta  - if not NULL, qcow2_co_pwritev_task() will consume it. Caller must
+ *   not use it somehow after qcow2_co_pwritev_task() call
+ */
+static coroutine_fn int qcow2_co_pwritev_task(BlockDriverState *bs,
+  uint64_t file_cluster_offset,
+  uint64_t offset, uint64_t bytes,
+  QEMUIOVector *qiov,
+  uint64_t qiov_offset,
+  QCowL2Meta *l2meta)
+{
+int ret;
+BDRVQcow2State *s = bs->opaque;
+void *crypt_buf = NULL;
+int offset_in_cluster = offset_into_cluster(s, offset);
+QEMUIOVector encrypted_qiov;
+
+if (bs->encrypted) {
+assert(s->crypto);
+assert(bytes <= QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
+crypt_buf = qemu_try_blockalign(bs->file->bs, bytes);
+if (crypt_buf == NULL) {
+ret = -ENOMEM;
+goto out_unlocked;
+}
+qemu_iovec_to_buf(qiov, qiov_offset, crypt_buf, bytes);
+
+if (qcow2_co_encrypt(bs, file_cluster_offset + offset_in_cluster,
+ offset, crypt_buf, bytes) < 0)
+{
+ret = -EIO;
+goto out_unlocked;
+}
+
+qemu_iovec_init_buf(_qiov, crypt_buf, bytes);
+qiov = _qiov;
+qiov_offset = 0;
+}
+
+/* Try to efficiently initialize the physical space with zeroes */
+ret = handle_alloc_space(bs, l2meta);
+if (ret < 0) {
+goto out_unlocked;
+}
+
+/*
+ * If we need to do COW, check if it's possible to merge the
+ * writing of the guest data together with that of the COW regions.
+ * If it's not possible (or not necessary) then write the
+ * guest data now.
+ */
+if (!merge_cow(offset, bytes, qiov, qiov_offset, l2meta)) {
+BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
+trace_qcow2_writev_data(qemu_coroutine_self(),
+file_cluster_offset + offset_in_cluster);
+ret = bdrv_co_pwritev_part(s->data_file,
+   file_cluster_offset + offset_in_cluster,
+   bytes, qiov, qiov_offset, 0);
+if (ret < 0) {
+goto out_unlocked;
+}
+}
+
+qemu_co_mutex_lock(>lock);
+
+ret = qcow2_handle_l2meta(bs, , true);
+goto out_locked;
+
+out_unlocked:
+qemu_co_mutex_lock(>lock);
+
+out_locked:
+qcow2_handle_l2meta(bs, , false);
+qemu_co_mutex_unlock(>lock);
+
+qemu_vfree(crypt_buf);
+
+return ret;
+}
+
 static coroutine_fn int qcow2_co_pwritev_part(
 BlockDriverState *bs, uint64_t offset, uint64_t bytes,
 QEMUIOVector *qiov, size_t qiov_offset, int flags)
@@ -2251,15 +2333,10 @@ static coroutine_fn int qcow2_co_pwritev_part(
 int ret;
 unsigned int cur_bytes; /* number of sectors in current iteration */
 uint64_t cluster_offset;
-QEMUIOVector encrypted_qiov;
-uint64_t bytes_done = 0;
-uint8_t *cluster_data = NULL;
 QCowL2Meta *l2meta = NULL;
 
 trace_qcow2_writev_start_req(qemu_coroutine_self(), offset, bytes);
 
-qemu_co_mutex_lock(>lock);
-
 while (bytes != 0) {
 
 l2meta = NULL;
@@ -2273,6 +2350,8 @@ static coroutine_fn int qcow2_co_pwritev_part(
 - offset_in_cluster);
 }
 
+qemu_co_mutex_lock(>lock);
+
 ret = qcow2_alloc_cluster_offset(bs, offset, _bytes,
  _offset, );
 if (ret < 0) {
@@ -2290,73 +2369,20 @@ static coroutine_fn int qcow2_co_pwritev_part(
 
 qemu_co_mutex_unlock(>lock);
 
-if (bs->encrypted) {
-assert(s->crypto);
-if (!cluster_data) {
-cluster_data = qemu_try_blockalign(bs->file->bs,
-   QCOW_MAX_CRYPT_CLUSTERS
-   * s->cluster_size);
-if (cluster_data == NULL) {
-ret = -ENOMEM;
-goto out_unlocked;
-}
-}
-
-assert(cur_bytes <= QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
- 

[PULL 10/36] block/backup: introduce BlockCopyState

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Split copying code part from backup to "block-copy", including separate
state structure and function renaming. This is needed to share it with
backup-top filter driver in further commits.

Notes:

1. As BlockCopyState keeps own BlockBackend objects, remaining
job->common.blk users only use it to get bs by blk_bs() call, so clear
job->commen.blk permissions set in block_job_create and add
job->source_bs to be used instead of blk_bs(job->common.blk), to keep
it more clear which bs we use when introduce backup-top filter in
further commit.

2. Rename s/initializing_bitmap/skip_unallocated/ to sound a bit better
as interface to BlockCopyState

3. Split is not very clean: there left some duplicated fields, backup
code uses some BlockCopyState fields directly, let's postpone it for
further improvements and keep this comment simpler for review.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20190920142056.12778-6-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block/backup.c | 370 -
 block/trace-events |  12 +-
 2 files changed, 239 insertions(+), 143 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index ae28849660..5dda1673ca 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -35,12 +35,52 @@ typedef struct CowRequest {
 CoQueue wait_queue; /* coroutines blocked on this request */
 } CowRequest;
 
+typedef void (*ProgressBytesCallbackFunc)(int64_t bytes, void *opaque);
+typedef void (*ProgressResetCallbackFunc)(void *opaque);
+typedef struct BlockCopyState {
+BlockBackend *source;
+BlockBackend *target;
+BdrvDirtyBitmap *copy_bitmap;
+int64_t cluster_size;
+bool use_copy_range;
+int64_t copy_range_size;
+uint64_t len;
+
+BdrvRequestFlags write_flags;
+
+/*
+ * skip_unallocated:
+ *
+ * Used by sync=top jobs, which first scan the source node for unallocated
+ * areas and clear them in the copy_bitmap.  During this process, the 
bitmap
+ * is thus not fully initialized: It may still have bits set for areas that
+ * are unallocated and should actually not be copied.
+ *
+ * This is indicated by skip_unallocated.
+ *
+ * In this case, block_copy() will query the source’s allocation status,
+ * skip unallocated regions, clear them in the copy_bitmap, and invoke
+ * block_copy_reset_unallocated() every time it does.
+ */
+bool skip_unallocated;
+
+/* progress_bytes_callback: called when some copying progress is done. */
+ProgressBytesCallbackFunc progress_bytes_callback;
+
+/*
+ * progress_reset_callback: called when some bytes reset from copy_bitmap
+ * (see @skip_unallocated above). The callee is assumed to recalculate how
+ * many bytes remain based on the dirty bit count of copy_bitmap.
+ */
+ProgressResetCallbackFunc progress_reset_callback;
+void *progress_opaque;
+} BlockCopyState;
+
 typedef struct BackupBlockJob {
 BlockJob common;
-BlockBackend *target;
+BlockDriverState *source_bs;
 
 BdrvDirtyBitmap *sync_bitmap;
-BdrvDirtyBitmap *copy_bitmap;
 
 MirrorSyncMode sync_mode;
 BitmapSyncMode bitmap_mode;
@@ -53,11 +93,7 @@ typedef struct BackupBlockJob {
 NotifierWithReturn before_write;
 QLIST_HEAD(, CowRequest) inflight_reqs;
 
-bool use_copy_range;
-int64_t copy_range_size;
-
-BdrvRequestFlags write_flags;
-bool initializing_bitmap;
+BlockCopyState *bcs;
 } BackupBlockJob;
 
 static const BlockJobDriver backup_job_driver;
@@ -99,9 +135,97 @@ static void cow_request_end(CowRequest *req)
 qemu_co_queue_restart_all(>wait_queue);
 }
 
+static void block_copy_state_free(BlockCopyState *s)
+{
+if (!s) {
+return;
+}
+
+bdrv_release_dirty_bitmap(blk_bs(s->source), s->copy_bitmap);
+blk_unref(s->source);
+blk_unref(s->target);
+g_free(s);
+}
+
+static BlockCopyState *block_copy_state_new(
+BlockDriverState *source, BlockDriverState *target,
+int64_t cluster_size, BdrvRequestFlags write_flags,
+ProgressBytesCallbackFunc progress_bytes_callback,
+ProgressResetCallbackFunc progress_reset_callback,
+void *progress_opaque, Error **errp)
+{
+BlockCopyState *s;
+int ret;
+uint64_t no_resize = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE |
+ BLK_PERM_WRITE_UNCHANGED | BLK_PERM_GRAPH_MOD;
+BdrvDirtyBitmap *copy_bitmap;
+
+copy_bitmap = bdrv_create_dirty_bitmap(source, cluster_size, NULL, errp);
+if (!copy_bitmap) {
+return NULL;
+}
+bdrv_disable_dirty_bitmap(copy_bitmap);
+
+s = g_new(BlockCopyState, 1);
+*s = (BlockCopyState) {
+.source = blk_new(bdrv_get_aio_context(source),
+  BLK_PERM_CONSISTENT_READ, no_resize),
+.target = blk_new(bdrv_get_aio_context(target),
+  BLK_PERM_WRITE, no_resize),
+.copy_bitmap = 

Re: [PATCH v2 1/1] IDE: deprecate ide-drive

[Kevin Wolf]

Am 10.10.2019 um 13:54 hat Peter Krempa geschrieben:
> On Thu, Oct 10, 2019 at 13:42:26 +0200, Philippe Mathieu-Daudé wrote:
> > On 10/10/19 1:26 PM, Peter Krempa wrote:
> > > On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote:
> > > > On 10/10/19 12:43 AM, John Snow wrote:
> > > > > It's an old compatibility shim that just delegates to ide-cd or 
> > > > > ide-hd.
> > > > > I'd like to refactor these some day, and getting rid of the 
> > > > > super-object
> > > > > will make that easier.
> > > > > 
> > > > > Either way, we don't need this.
> > > > > 
> > > > > Libvirt-checked-by: Peter Krempa 
> > > > 
> > > > Peter made a comment regarding Laszlo's Regression-tested-by tag:
> > > > 
> > > >[...] nobody else is using
> > > >this convention (there are exactly 0 instances of
> > > >"Regression-tested-by" in the project git log as far as
> > > >I can see), and so in practice people reading the commits
> > > >won't really know what you meant by it. Everybody else
> > > >on the project uses "Tested-by" to mean either of the
> > > >two cases you describe above, without distinction...
> > > > 
> > > > It probably applies to 'Libvirt-checked-by' too.
> > > 
> > > I certainly didn't test it. So feel free to drop that line altogether.
> > 
> > But you reviewed it, can we use your 'Reviewed-by' instead?
> 
> To be honest, I didn't really review the code nor the documentation.
> I actually reviewed only the idea itself in the context of integration
> with libvirt and that's why I didn't go for 'Reviewed-by:'.
> 
> The gist of the citation above is that we should stick to well known
> tags with their well known meanings and I think that considering this a
> 'review' would be a stretch of the definiton.

I think Acked-by works well for instances like this (i.e. you're just
saying that you agree and the intended change is fine from libvirt's
POV, not that you made any effort to check that the patches are correct
or anything).

Kevin

[PATCH v3 3/8] hw/ide/piix: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

The PIIX/IDE is a PCI device within a PIIX chipset, it will be reset
when the PCI bus it stands on is reset.

Convert its reset handler into a proper Device reset method.

Signed-off-by: Philippe Mathieu-Daudé 
---
v3: Also convert PIIX4 (Li Qiang)
---
 hw/ide/piix.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index fba6bc8bff..db313dd3b1 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -30,7 +30,6 @@
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
 #include "sysemu/dma.h"
-#include "sysemu/reset.h"
 
 #include "hw/ide/pci.h"
 #include "trace.h"
@@ -103,9 +102,9 @@ static void bmdma_setup_bar(PCIIDEState *d)
 }
 }
 
-static void piix3_reset(void *opaque)
+static void piix_ide_reset(DeviceState *dev)
 {
-PCIIDEState *d = opaque;
+PCIIDEState *d = PCI_IDE(dev);
 PCIDevice *pd = PCI_DEVICE(d);
 uint8_t *pci_conf = pd->config;
 int i;
@@ -154,8 +153,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error 
**errp)
 
 pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
 
-qemu_register_reset(piix3_reset, d);
-
 bmdma_setup_bar(d);
 pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, >bmdma_bar);
 
@@ -247,6 +244,7 @@ static void piix3_ide_class_init(ObjectClass *klass, void 
*data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
+dc->reset = piix_ide_reset;
 k->realize = pci_piix_ide_realize;
 k->exit = pci_piix_ide_exitfn;
 k->vendor_id = PCI_VENDOR_ID_INTEL;
@@ -273,6 +271,7 @@ static void piix4_ide_class_init(ObjectClass *klass, void 
*data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
+dc->reset = piix_ide_reset;
 k->realize = pci_piix_ide_realize;
 k->exit = pci_piix_ide_exitfn;
 k->vendor_id = PCI_VENDOR_ID_INTEL;
-- 
2.21.0

[PATCH v3 4/8] hw/ide/sii3112: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

The SiI3112A SATA controller is a PCI device, it will be reset
when the PCI bus it stands on is reset.

Convert its reset handler into a proper Device reset method.

Reviewed-by: Li Qiang 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ide/sii3112.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index 2181260531..06605d7af2 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -15,7 +15,6 @@
 #include "qemu/osdep.h"
 #include "hw/ide/pci.h"
 #include "qemu/module.h"
-#include "sysemu/reset.h"
 #include "trace.h"
 
 #define TYPE_SII3112_PCI "sii3112"
@@ -237,9 +236,9 @@ static void sii3112_set_irq(void *opaque, int channel, int 
level)
 sii3112_update_irq(s);
 }
 
-static void sii3112_reset(void *opaque)
+static void sii3112_reset(DeviceState *dev)
 {
-SiI3112PCIState *s = opaque;
+SiI3112PCIState *s = SII3112_PCI(dev);
 int i;
 
 for (i = 0; i < 2; i++) {
@@ -290,7 +289,6 @@ static void sii3112_pci_realize(PCIDevice *dev, Error 
**errp)
 s->bmdma[i].bus = >bus[i];
 ide_register_restart_cb(>bus[i]);
 }
-qemu_register_reset(sii3112_reset, s);
 }
 
 static void sii3112_pci_class_init(ObjectClass *klass, void *data)
@@ -303,6 +301,7 @@ static void sii3112_pci_class_init(ObjectClass *klass, void 
*data)
 pd->class_id = PCI_CLASS_STORAGE_RAID;
 pd->revision = 1;
 pd->realize = sii3112_pci_realize;
+dc->reset = sii3112_reset;
 dc->desc = "SiI3112A SATA controller";
 set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 }
-- 
2.21.0

[PATCH v3 2/8] hw/isa/piix4: Convert reset handler to DeviceReset

[Philippe Mathieu-Daudé]

The PIIX4/ISA is a PCI device within the PIIX4 chipset, it will be reset
when the PCI bus it stands on is reset.

Convert its reset handler into a proper Device reset method.

Reviewed-by: Li Qiang 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/isa/piix4.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 3294056cd5..890d999abf 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -28,7 +28,6 @@
 #include "hw/isa/isa.h"
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
-#include "sysemu/reset.h"
 
 PCIDevice *piix4_dev;
 
@@ -40,9 +39,9 @@ typedef struct PIIX4State {
 #define PIIX4_PCI_DEVICE(obj) \
 OBJECT_CHECK(PIIX4State, (obj), TYPE_PIIX4_PCI_DEVICE)
 
-static void piix4_reset(void *opaque)
+static void piix4_isa_reset(DeviceState *dev)
 {
-PIIX4State *d = opaque;
+PIIX4State *d = PIIX4_PCI_DEVICE(dev);
 uint8_t *pci_conf = d->dev.config;
 
 pci_conf[0x04] = 0x07; // master, memory and I/O
@@ -97,7 +96,6 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
 return;
 }
 piix4_dev = >dev;
-qemu_register_reset(piix4_reset, d);
 }
 
 int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn)
@@ -118,6 +116,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
 k->vendor_id = PCI_VENDOR_ID_INTEL;
 k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
 k->class_id = PCI_CLASS_BRIDGE_ISA;
+dc->reset = piix4_isa_reset;
 dc->desc = "ISA bridge";
 dc->vmsd = _piix4;
 /*
-- 
2.21.0

[PATCH v3 8/8] hw/misc/vmcoreinfo: Add comment about reset handler

[Philippe Mathieu-Daudé]

The VM coreinfo device does not sit on a bus, so it won't be
reset automatically. This is why it calls qemu_register_reset().

Add a comment about it, so we don't convert its reset handler
to a DeviceReset method.

Reviewed-by: Marc-André Lureau 
Reviewed-by: Li Qiang 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/misc/vmcoreinfo.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/misc/vmcoreinfo.c b/hw/misc/vmcoreinfo.c
index 326a3ce8f4..a9d718fc23 100644
--- a/hw/misc/vmcoreinfo.c
+++ b/hw/misc/vmcoreinfo.c
@@ -61,6 +61,10 @@ static void vmcoreinfo_realize(DeviceState *dev, Error 
**errp)
  NULL, fw_cfg_vmci_write, s,
  >vmcoreinfo, sizeof(s->vmcoreinfo), false);
 
+/*
+ * This device requires to register a global reset because it is
+ * not plugged to a bus (which, as its QOM parent, would reset it).
+ */
 qemu_register_reset(vmcoreinfo_reset, dev);
 vmcoreinfo_state = s;
 }
-- 
2.21.0

[PATCH 22/23] iotests/267: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/267 | 4 ++--
 tests/qemu-iotests/267.out | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/267 b/tests/qemu-iotests/267
index d37a67c012..170e173c0a 100755
--- a/tests/qemu-iotests/267
+++ b/tests/qemu-iotests/267
@@ -29,7 +29,7 @@ status=1  # failure is the default!
 _cleanup()
 {
 _cleanup_test_img
-rm -f "$TEST_DIR/nbd"
+rm -f "$SOCK_DIR/nbd"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -143,7 +143,7 @@ echo
 
 IMGOPTS="backing_file=$TEST_IMG.base" _make_test_img $size
 cat <

[PATCH 14/23] iotests/194: Create sockets in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/194 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/194 b/tests/qemu-iotests/194
index d746ab1e21..72e47e8833 100755
--- a/tests/qemu-iotests/194
+++ b/tests/qemu-iotests/194
@@ -26,8 +26,8 @@ iotests.verify_platform(['linux'])
 
 with iotests.FilePath('source.img') as source_img_path, \
  iotests.FilePath('dest.img') as dest_img_path, \
- iotests.FilePath('migration.sock') as migration_sock_path, \
- iotests.FilePath('nbd.sock') as nbd_sock_path, \
+ iotests.FilePaths(['migration.sock', 'nbd.sock'], iotests.sock_dir) as \
+ [migration_sock_path, nbd_sock_path], \
  iotests.VM('source') as source_vm, \
  iotests.VM('dest') as dest_vm:
 
-- 
2.21.0

[PATCH 16/23] iotests/205: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/205 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/205 b/tests/qemu-iotests/205
index 76f6c5fa2b..4bb2c21e8b 100755
--- a/tests/qemu-iotests/205
+++ b/tests/qemu-iotests/205
@@ -24,7 +24,7 @@ import iotests
 import time
 from iotests import qemu_img_create, qemu_io, filter_qemu_io, QemuIoInteractive
 
-nbd_sock = os.path.join(iotests.test_dir, 'nbd_sock')
+nbd_sock = os.path.join(iotests.sock_dir, 'nbd_sock')
 nbd_uri = 'nbd+unix:///exp?socket=' + nbd_sock
 disk = os.path.join(iotests.test_dir, 'disk')
 
-- 
2.21.0

[PATCH 13/23] iotests/192: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/192 | 4 ++--
 tests/qemu-iotests/192.out | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/192 b/tests/qemu-iotests/192
index 034432272f..d2ba55dd90 100755
--- a/tests/qemu-iotests/192
+++ b/tests/qemu-iotests/192
@@ -31,7 +31,7 @@ _cleanup()
 {
 _cleanup_qemu
 _cleanup_test_img
-rm -f "$TEST_DIR/nbd"
+rm -f "$SOCK_DIR/nbd"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -66,7 +66,7 @@ else
 QEMU_COMM_TIMEOUT=1
 fi
 
-_send_qemu_cmd $h "nbd_server_start unix:$TEST_DIR/nbd" "(qemu)"
+_send_qemu_cmd $h "nbd_server_start unix:$SOCK_DIR/nbd" "(qemu)"
 _send_qemu_cmd $h "nbd_server_add -w drive0" "(qemu)"
 _send_qemu_cmd $h "q" "(qemu)"
 
diff --git a/tests/qemu-iotests/192.out b/tests/qemu-iotests/192.out
index 1e0be4c4d7..b9429dbe36 100644
--- a/tests/qemu-iotests/192.out
+++ b/tests/qemu-iotests/192.out
@@ -1,7 +1,7 @@
 QA output created by 192
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) nbd_server_start unix:TEST_DIR/nbd
+(qemu) nbd_server_start unix:SOCK_DIR/nbd
 (qemu) nbd_server_add -w drive0
 (qemu) q
 *** done
-- 
2.21.0

[PATCH 15/23] iotests/201: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/201 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/201 b/tests/qemu-iotests/201
index 7abf740fe4..86fa37e714 100755
--- a/tests/qemu-iotests/201
+++ b/tests/qemu-iotests/201
@@ -24,7 +24,7 @@ echo "QA output created by $seq"
 
 status=1   # failure is the default!
 
-MIG_SOCKET="${TEST_DIR}/migrate"
+MIG_SOCKET="${SOCK_DIR}/migrate"
 
 # get standard environment, filters and checks
 . ./common.rc
-- 
2.21.0

[PATCH 06/23] iotests/083: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/083 |  6 +++---
 tests/qemu-iotests/083.out | 34 +-
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083
index b270550d3e..10fdfc8ebb 100755
--- a/tests/qemu-iotests/083
+++ b/tests/qemu-iotests/083
@@ -28,7 +28,7 @@ status=1  # failure is the default!
 
 _cleanup()
 {
-   rm -f nbd.sock
+   rm -f "$SOCK_DIR/nbd.sock"
rm -f nbd-fault-injector.out
rm -f nbd-fault-injector.conf
 }
@@ -80,10 +80,10 @@ EOF
if [ "$proto" = "tcp" ]; then
nbd_addr="127.0.0.1:0"
else
-   nbd_addr="$TEST_DIR/nbd.sock"
+   nbd_addr="$SOCK_DIR/nbd.sock"
fi
 
-   rm -f "$TEST_DIR/nbd.sock"
+   rm -f "$SOCK_DIR/nbd.sock"
 
 echo > "$TEST_DIR/nbd-fault-injector.out"
$PYTHON nbd-fault-injector.py $extra_args "$nbd_addr" 
"$TEST_DIR/nbd-fault-injector.conf" >"$TEST_DIR/nbd-fault-injector.out" 2>&1 &
diff --git a/tests/qemu-iotests/083.out b/tests/qemu-iotests/083.out
index eee6dd1379..2090ee693c 100644
--- a/tests/qemu-iotests/083.out
+++ b/tests/qemu-iotests/083.out
@@ -110,43 +110,43 @@ read failed: Input/output error
 
 === Check disconnect before neg1 ===
 
-qemu-io: can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///foo?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect after neg1 ===
 
-qemu-io: can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///foo?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect 8 neg1 ===
 
-qemu-io: can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///foo?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect 16 neg1 ===
 
-qemu-io: can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///foo?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect before export ===
 
-qemu-io: can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///foo?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect after export ===
 
-qemu-io: can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///foo?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect 4 export ===
 
-qemu-io: can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///foo?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect 12 export ===
 
-qemu-io: can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///foo?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect 16 export ===
 
-qemu-io: can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///foo?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect before neg2 ===
 
-qemu-io: can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///foo?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect after neg2 ===
 
@@ -154,11 +154,11 @@ read failed: Input/output error
 
 === Check disconnect 8 neg2 ===
 
-qemu-io: can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///foo?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect 10 neg2 ===
 
-qemu-io: can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///foo?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect before request ===
 
@@ -195,23 +195,23 @@ read 512/512 bytes at offset 0
 
 === Check disconnect before neg-classic ===
 
-qemu-io: can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect 8 neg-classic ===
 
-qemu-io: can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect 16 neg-classic ===
 
-qemu-io: can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect 24 neg-classic ===
 
-qemu-io: can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect 28 neg-classic ===
 
-qemu-io: can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
+qemu-io: can't open device nbd+unix:///?socket=SOCK_DIR/nbd.sock
 
 === Check disconnect after neg-classic ===
 
-- 
2.21.0

[PATCH 07/23] iotests/140: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/140 | 8 
 tests/qemu-iotests/140.out | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/140 b/tests/qemu-iotests/140
index b965b1dd5d..8d2ce5d9e3 100755
--- a/tests/qemu-iotests/140
+++ b/tests/qemu-iotests/140
@@ -34,7 +34,7 @@ _cleanup()
 {
 _cleanup_qemu
 _cleanup_test_img
-rm -f "$TEST_DIR/nbd"
+rm -f "$SOCK_DIR/nbd"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -69,7 +69,7 @@ _send_qemu_cmd $QEMU_HANDLE \
 _send_qemu_cmd $QEMU_HANDLE \
 "{ 'execute': 'nbd-server-start',
'arguments': { 'addr': { 'type': 'unix',
-'data': { 'path': '$TEST_DIR/nbd' " \
+'data': { 'path': '$SOCK_DIR/nbd' " \
 'return'
 
 _send_qemu_cmd $QEMU_HANDLE \
@@ -78,7 +78,7 @@ _send_qemu_cmd $QEMU_HANDLE \
 'return'
 
 $QEMU_IO_PROG -f raw -r -c 'read -P 42 0 64k' \
-"nbd+unix:///drv?socket=$TEST_DIR/nbd" 2>&1 \
+"nbd+unix:///drv?socket=$SOCK_DIR/nbd" 2>&1 \
 | _filter_qemu_io | _filter_nbd
 
 _send_qemu_cmd $QEMU_HANDLE \
@@ -87,7 +87,7 @@ _send_qemu_cmd $QEMU_HANDLE \
 'return'
 
 $QEMU_IO_PROG -f raw -r -c close \
-"nbd+unix:///drv?socket=$TEST_DIR/nbd" 2>&1 \
+"nbd+unix:///drv?socket=$SOCK_DIR/nbd" 2>&1 \
 | _filter_qemu_io | _filter_nbd
 
 _send_qemu_cmd $QEMU_HANDLE \
diff --git a/tests/qemu-iotests/140.out b/tests/qemu-iotests/140.out
index 67fe44a3e3..2511eb7369 100644
--- a/tests/qemu-iotests/140.out
+++ b/tests/qemu-iotests/140.out
@@ -8,7 +8,7 @@ wrote 65536/65536 bytes at offset 0
 read 65536/65536 bytes at offset 0
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 {"return": {}}
-qemu-io: can't open device nbd+unix:///drv?socket=TEST_DIR/nbd: Requested 
export not available
+qemu-io: can't open device nbd+unix:///drv?socket=SOCK_DIR/nbd: Requested 
export not available
 server reported: export 'drv' not present
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
-- 
2.21.0

[RESEND PATCH V4] block/vhdx: add check for truncated image files

[Peter Lieven]

qemu is currently not able to detect truncated vhdx image files.
Add a basic check if all allocated blocks are reachable at open and
report all errors during bdrv_co_check.

Signed-off-by: Peter Lieven 
---
V4: - allow partial last blocks [Kevin]
- report offsets in error messages [Kevin]
- check for start and end offset after eof

V3: - check for bdrv_getlength failure [Kevin]
- use uint32_t for i [Kevin]
- check for BAT entry overflow [Kevin]
- break on !errcnt in second check

V2: - add error reporting [Kevin]
- use bdrv_getlength instead of bdrv_get_allocated_file_size [Kevin]
- factor out BAT entry check and add error reporting for region
  overlaps
- already check on vhdx_open

 block/vhdx.c | 120 +++
 1 file changed, 103 insertions(+), 17 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index 6a09d0a55c..371f226286 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -24,6 +24,7 @@
 #include "qemu/option.h"
 #include "qemu/crc32c.h"
 #include "qemu/bswap.h"
+#include "qemu/error-report.h"
 #include "vhdx.h"
 #include "migration/blocker.h"
 #include "qemu/uuid.h"
@@ -235,6 +236,9 @@ static int vhdx_region_check(BDRVVHDXState *s, uint64_t 
start, uint64_t length)
 end = start + length;
 QLIST_FOREACH(r, >regions, entries) {
 if (!((start >= r->end) || (end <= r->start))) {
+error_report("VHDX region %" PRIu64 "-%" PRIu64 " overlaps with "
+ "region %" PRIu64 "-%." PRIu64, start, end, r->start,
+ r->end);
 ret = -EINVAL;
 goto exit;
 }
@@ -877,6 +881,95 @@ static void vhdx_calc_bat_entries(BDRVVHDXState *s)
 
 }
 
+static int vhdx_check_bat_entries(BlockDriverState *bs, int *errcnt)
+{
+BDRVVHDXState *s = bs->opaque;
+int64_t image_file_size = bdrv_getlength(bs->file->bs);
+uint64_t payblocks = s->chunk_ratio;
+uint64_t i;
+int ret = 0;
+
+if (image_file_size < 0) {
+error_report("Could not determinate VHDX image file size.");
+return image_file_size;
+}
+
+for (i = 0; i < s->bat_entries; i++) {
+if ((s->bat[i] & VHDX_BAT_STATE_BIT_MASK) ==
+PAYLOAD_BLOCK_FULLY_PRESENT) {
+uint64_t offset = s->bat[i] & VHDX_BAT_FILE_OFF_MASK;
+/*
+ * Allow that the last block exists only partially. The VHDX spec
+ * states that the image file can only grow in blocksize 
increments,
+ * but QEMU created images with partial last blocks in the past.
+ */
+uint32_t block_length = MIN(s->block_size,
+bs->total_sectors * BDRV_SECTOR_SIZE - i * s->block_size);
+/*
+ * Check for BAT entry overflow.
+ */
+if (offset > INT64_MAX - s->block_size) {
+error_report("VHDX BAT entry %" PRIu64 " offset overflow.", i);
+ret = -EINVAL;
+if (!errcnt) {
+break;
+}
+(*errcnt)++;
+}
+/*
+ * Check if fully allocated BAT entries do not reside after
+ * end of the image file.
+ */
+if (offset >= image_file_size) {
+error_report("VHDX BAT entry %" PRIu64 " start offset %" PRIu64
+ " points after end of file (%" PRIi64 "). Image"
+ " has probably been truncated.",
+ i, offset, image_file_size);
+ret = -EINVAL;
+if (!errcnt) {
+break;
+}
+(*errcnt)++;
+} else if (offset + block_length > image_file_size) {
+error_report("VHDX BAT entry %" PRIu64 " end offset %" PRIu64
+ " points after end of file (%" PRIi64 "). Image"
+ " has probably been truncated.",
+ i, offset + block_length - 1, image_file_size);
+ret = -EINVAL;
+if (!errcnt) {
+break;
+}
+(*errcnt)++;
+}
+
+/*
+ * verify populated BAT field file offsets against
+ * region table and log entries
+ */
+if (payblocks--) {
+/* payload bat entries */
+int ret2;
+ret2 = vhdx_region_check(s, offset, s->block_size);
+if (ret2 < 0) {
+ret = -EINVAL;
+if (!errcnt) {
+break;
+}
+(*errcnt)++;
+}
+} else {
+payblocks = s->chunk_ratio;
+/*
+ * Once differencing files are supported, verify sector bitmap
+ * blocks here
+ */

Re: [Qemu-block] [PATCH v2 1/2] blockdev: release the AioContext at drive_backup_prepare

[Kevin Wolf]

Am 03.10.2019 um 11:33 hat Sergio Lopez geschrieben:
> 
> Sergio Lopez  writes:
> 
> > Kevin Wolf  writes:
> >
> >> Am 13.09.2019 um 21:54 hat John Snow geschrieben:
> >>> 
> >>> 
> >>> On 9/13/19 11:25 AM, Sergio Lopez wrote:
> >>> > do_drive_backup() already acquires the AioContext, so release it
> >>> > before the call.
> >>> > 
> >>> > Signed-off-by: Sergio Lopez 
> >>> > ---
> >>> >  blockdev.c | 6 +-
> >>> >  1 file changed, 1 insertion(+), 5 deletions(-)
> >>> > 
> >>> > diff --git a/blockdev.c b/blockdev.c
> >>> > index fbef6845c8..3927fdab80 100644
> >>> > --- a/blockdev.c
> >>> > +++ b/blockdev.c
> >>> > @@ -1783,20 +1783,16 @@ static void drive_backup_prepare(BlkActionState 
> >>> > *common, Error **errp)
> >>> >  
> >>> >  aio_context = bdrv_get_aio_context(bs);
> >>> >  aio_context_acquire(aio_context);
> >>> > -
> >>
> >> Are you removing this unrelated empty line intentionally?
> >
> > Yes. In the sense of that whole set of lines being a "open drained
> > section" block.
> >
> >>> >  /* Paired with .clean() */
> >>> >  bdrv_drained_begin(bs);
> >>> 
> >>> Do we need to make this change to blockdev_backup_prepare as well?
> >>
> >> Actually, the whole structure feels a bit wrong. We get the bs here and
> >> take its lock, then release the lock again and forget the reference,
> >> only to do both things again inside do_drive_backup().
> >>
> >> The way snapshots work is that the "normal" snapshot commands are
> >> wrappers that turn it into a single-entry transaction. Then you have
> >> only one code path where you can resolve the ID and take the lock just
> >> once. So maybe backup should work like this, too?
> >
> > I'm neither opposed nor in favor, but I think this is outside the scope
> > of this patch series.
> 
> Kevin, do you think we should attempt to just fix this issue (which
> would make a possible backport easier) or try to move all blockdev
> actions to be transaction-based?

Maybe fix it and then do the cleanup on top, though possibly in the same
series?

Kevin


signature.asc
Description: PGP signature

Re: [PATCH 1/2] qcow2: Limit total allocation range to INT_MAX

[Eric Blake]

On 10/10/19 5:08 AM, Max Reitz wrote:

When the COW areas are included, the size of an allocation can exceed
INT_MAX.  This is kind of limited by handle_alloc() in that it already
caps avail_bytes at INT_MAX, but the number of clusters still reflects
the original length.

This can have all sorts of effects, ranging from the storage layer write
call failing to image corruption.  (If there were no image corruption,
then I suppose there would be data loss because the .cow_end area is
forced to be empty, even though there might be something we need to
COW.)

Fix all of it by limiting nb_clusters so the equivalent number of bytes
will not exceed INT_MAX.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Max Reitz 
---
  block/qcow2-cluster.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: Problems with c8bb23cbdbe3 on ppc64le

[Anton Nefedov]

On 10/10/2019 6:17 PM, Max Reitz wrote:
> Hi everyone,
> 
> (CCs just based on tags in the commit in question)
> 
> I have two bug reports which claim problems of qcow2 on XFS on ppc64le
> machines since qemu 4.1.0.  One of those is about bad performance
> (sorry, is isn’t public :-/), the other about data corruption
> (https://bugzilla.redhat.com/show_bug.cgi?id=1751934).
> 
> It looks like in both cases reverting c8bb23cbdbe3 solves the problem
> (which optimized COW of unallocated areas).
> 
> I think I’ve looked at every angle but can‘t find what could be wrong
> with it.  Do any of you have any idea? :-/
> 

hi,

oh, that patch strikes again..

I don't quite follow, was this bug confirmed to happen on x86? Comment 8
(https://bugzilla.redhat.com/show_bug.cgi?id=1751934#c8) mentioned that
(or was that mixed up with the old xfsctl bug?)

Regardless of the platform, does it reproduce? That's comforting
already; worst case we can trace each and every request then (unless it
will stop to reproduce this way).

Also, perhaps it's worth to try to replace fallocate with write(0)?
Either in qcow2 (in the patch, bdrv_co_pwrite_zeroes -> bdrv_co_pwritev)
or in the file driver. It might hint whether it's misbehaving fallocate
(in qemu or in kernel) or something else.

/Anton

Re: [PATCH V4] block/vhdx: add check for truncated image files

[Kevin Wolf]

Am 10.09.2019 um 17:26 hat Peter Lieven geschrieben:
> qemu is currently not able to detect truncated vhdx image files.
> Add a basic check if all allocated blocks are reachable at open and
> report all errors during bdrv_co_check.
> 
> Signed-off-by: Peter Lieven 

Thanks, applied to the block branch.

Kevin

Re: [PATCH 2/2] iotests: Test large write request to qcow2 file

[Eric Blake]

On 10/10/19 5:08 AM, Max Reitz wrote:

Without HEAD^, the following happens when you attempt a large write
request to a qcow2 file such that the number of bytes covered by all
clusters involved in a single allocation will exceed INT_MAX:

(A) handle_alloc_space() decides to fill the whole area with zeroes and
 fails because bdrv_co_pwrite_zeroes() fails (the request is too
 large).

(B) If handle_alloc_space() does not do anything, but merge_cow()
 decides that the requests can be merged, it will create a too long
 IOV that later cannot be written.

(C) Otherwise, all parts will be written separately, so those requests
 will work.

In either B or C, though, qcow2_alloc_cluster_link_l2() will have an
overflow: We use an int (i) to iterate over nb_clusters, and then
calculate the L2 entry based on "i << s->cluster_bits" -- which will
overflow if the range covers more than INT_MAX bytes.  This then leads
to image corruption because the L2 entry will be wrong (it will be
recognized as a compressed cluster).

Even if that were not the case, the .cow_end area would be empty
(because handle_alloc() will cap avail_bytes and nb_bytes at INT_MAX, so
their difference (which is the .cow_end size) will be 0).

So this test checks that on such large requests, the image will not be
corrupted.  Unfortunately, we cannot check whether COW will be handled
correctly, because that data is discarded when it is written to null-co
(but we have to use null-co, because writing 2 GB of data in a test is
not quite reasonable).

Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/268 | 83 ++
  tests/qemu-iotests/268.out |  9 +
  tests/qemu-iotests/group   |  1 +
  3 files changed, 93 insertions(+)
  create mode 100755 tests/qemu-iotests/268
  create mode 100644 tests/qemu-iotests/268.out

diff --git a/tests/qemu-iotests/268 b/tests/qemu-iotests/268
new file mode 100755
index 00..b9a12b908c
--- /dev/null
+++ b/tests/qemu-iotests/268



+# We want a null-co as the data file, because it allows us to quickly
+# "write" 2G of data without using any space.
+# (qemu-img create does not like it, though, because null-co does not
+# support image creation.)
+$QEMU_IMG amend -o data_file="json:{'driver':'null-co',,'size':'4294967296'}" \
+"$TEST_IMG"
+


A bit awkward, but works.


+# This gives us a range of:
+#   2^31 - 512 + 768 - 1 = 2^31 + 255 > 2^31
+# until the beginning of the end COW block.  (The total allocation
+# size depends on the cluster size, but all that is important is that
+# it exceeds INT_MAX.)
+#
+# 2^31 - 512 is the maximum request size.  We want this to result in a
+# single allocation, and because the qcow2 driver splits allocations
+# on L2 boundaries, we need large L2 tables; hence the cluster size of
+# 2 MB.  (Anything from 256 kB should work, though, because then one L2
+# table covers 8 GB.)
+$QEMU_IO -c "write 768 $((2 ** 31 - 512))" "$TEST_IMG" | _filter_qemu_io


Yep, that causes the rounding issue that requires being able to handle > 
2G gracefully.


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Problems with c8bb23cbdbe3 on ppc64le

[Max Reitz]

Hi everyone,

(CCs just based on tags in the commit in question)

I have two bug reports which claim problems of qcow2 on XFS on ppc64le
machines since qemu 4.1.0.  One of those is about bad performance
(sorry, is isn’t public :-/), the other about data corruption
(https://bugzilla.redhat.com/show_bug.cgi?id=1751934).

It looks like in both cases reverting c8bb23cbdbe3 solves the problem
(which optimized COW of unallocated areas).

I think I’ve looked at every angle but can‘t find what could be wrong
with it.  Do any of you have any idea? :-/


Thanks,

Max



signature.asc
Description: OpenPGP digital signature

[PATCH 02/23] iotests.py: Store socket files in $SOCK_DIR

[Max Reitz]

iotests.py itself does not store socket files, but it machine.py and
qtest.py do.  iotests.py needs to pass the respective path to them, and
they need to adhere to it.

Signed-off-by: Max Reitz 
---
 python/qemu/machine.py| 15 ---
 python/qemu/qtest.py  |  9 ++---
 tests/qemu-iotests/iotests.py |  4 +++-
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 128a3d1dc2..2024e8b1b1 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -71,7 +71,7 @@ class QEMUMachine(object):
 
 def __init__(self, binary, args=None, wrapper=None, name=None,
  test_dir="/var/tmp", monitor_address=None,
- socket_scm_helper=None):
+ socket_scm_helper=None, sock_dir=None):
 '''
 Initialize a QEMUMachine
 
@@ -90,6 +90,8 @@ class QEMUMachine(object):
 wrapper = []
 if name is None:
 name = "qemu-%d" % os.getpid()
+if sock_dir is None:
+sock_dir = test_dir
 self._name = name
 self._monitor_address = monitor_address
 self._vm_monitor = None
@@ -106,12 +108,14 @@ class QEMUMachine(object):
 self._qemu_full_args = None
 self._test_dir = test_dir
 self._temp_dir = None
+self._sock_dir = sock_dir
 self._launched = False
 self._machine = None
 self._console_set = False
 self._console_device_type = None
 self._console_address = None
 self._console_socket = None
+self._remove_files = []
 
 # just in case logging wasn't configured by the main script:
 logging.basicConfig()
@@ -236,8 +240,9 @@ class QEMUMachine(object):
 if self._machine is not None:
 args.extend(['-machine', self._machine])
 if self._console_set:
-self._console_address = os.path.join(self._temp_dir,
+self._console_address = os.path.join(self._sock_dir,
  self._name + "-console.sock")
+self._remove_files.append(self._console_address)
 chardev = ('socket,id=console,path=%s,server,nowait' %
self._console_address)
 args.extend(['-chardev', chardev])
@@ -253,8 +258,9 @@ class QEMUMachine(object):
 if self._monitor_address is not None:
 self._vm_monitor = self._monitor_address
 else:
-self._vm_monitor = os.path.join(self._temp_dir,
+self._vm_monitor = os.path.join(self._sock_dir,
 self._name + "-monitor.sock")
+self._remove_files.append(self._vm_monitor)
 self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log")
 self._qemu_log_file = open(self._qemu_log_path, 'wb')
 
@@ -279,6 +285,9 @@ class QEMUMachine(object):
 shutil.rmtree(self._temp_dir)
 self._temp_dir = None
 
+while len(self._remove_files) > 0:
+self._remove_if_exists(self._remove_files.pop())
+
 def launch(self):
 """
 Launch the VM and make sure we cleanup and expose the
diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
index 3f1d2cb325..d24ad04256 100644
--- a/python/qemu/qtest.py
+++ b/python/qemu/qtest.py
@@ -84,14 +84,17 @@ class QEMUQtestMachine(QEMUMachine):
 '''A QEMU VM'''
 
 def __init__(self, binary, args=None, name=None, test_dir="/var/tmp",
- socket_scm_helper=None):
+ socket_scm_helper=None, sock_dir=None):
 if name is None:
 name = "qemu-%d" % os.getpid()
+if sock_dir is None:
+sock_dir = test_dir
 super(QEMUQtestMachine,
   self).__init__(binary, args, name=name, test_dir=test_dir,
- socket_scm_helper=socket_scm_helper)
+ socket_scm_helper=socket_scm_helper,
+ sock_dir=sock_dir)
 self._qtest = None
-self._qtest_path = os.path.join(test_dir, name + "-qtest.sock")
+self._qtest_path = os.path.join(sock_dir, name + "-qtest.sock")
 
 def _base_args(self):
 args = super(QEMUQtestMachine, self)._base_args()
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 3a8f378f90..49cd205a97 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -57,6 +57,7 @@ qemu_opts = os.environ.get('QEMU_OPTIONS', 
'').strip().split(' ')
 imgfmt = os.environ.get('IMGFMT', 'raw')
 imgproto = os.environ.get('IMGPROTO', 'file')
 test_dir = os.environ.get('TEST_DIR')
+sock_dir = os.environ.get('SOCK_DIR')
 output_dir = os.environ.get('OUTPUT_DIR', '.')
 cachemode = os.environ.get('CACHEMODE')
 qemu_default_machine = os.environ.get('QEMU_DEFAULT_MACHINE')
@@ -445,7 +446,8 @@ class VM(qtest.QEMUQtestMachine):
 name = "qemu%s-%d" % (path_suffix, 

[PATCH 01/23] iotests: Introduce $SOCK_DIR

[Max Reitz]

Unix sockets generally have a maximum path length.  Depending on your
$TEST_DIR, it may be exceeded and then all tests that create and use
Unix sockets there may fail.

Circumvent this by adding a new scratch directory specifically for
Unix socket files.  It defaults to a temporary directory (mktemp -d)
that is completely removed after the iotests are done.

(By default, mktemp -d creates a /tmp/tmp.XX directory, which
should be short enough for our use cases.)

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/check | 17 +
 1 file changed, 17 insertions(+)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 588c453a94..a257215448 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -97,6 +97,7 @@ IMGFMT-- $FULL_IMGFMT_DETAILS
 IMGPROTO  -- $IMGPROTO
 PLATFORM  -- $FULL_HOST_DETAILS
 TEST_DIR  -- $TEST_DIR
+SOCK_DIR  -- $SOCK_DIR
 SOCKET_SCM_HELPER -- $SOCKET_SCM_HELPER
 
 EOF
@@ -121,6 +122,16 @@ if [ ! -e "$TEST_DIR" ]; then
 mkdir "$TEST_DIR"
 fi
 
+tmp_sock_dir=false
+if [ -z "$SOCK_DIR" ]; then
+SOCK_DIR=$(mktemp -d)
+tmp_sock_dir=true
+fi
+
+if [ ! -d "$SOCK_DIR" ]; then
+mkdir "$SOCK_DIR"
+fi
+
 diff="diff -u"
 verbose=false
 debug=false
@@ -534,6 +545,7 @@ if [ -z "$SAMPLE_IMG_DIR" ]; then
 fi
 
 export TEST_DIR
+export SOCK_DIR
 export SAMPLE_IMG_DIR
 
 if [ -s $tmp.list ]
@@ -716,6 +728,11 @@ END{ if (NR > 0) {
 rm -f "${TEST_DIR}"/*.out "${TEST_DIR}"/*.err "${TEST_DIR}"/*.time
 rm -f "${TEST_DIR}"/check.pid "${TEST_DIR}"/check.sts
 rm -f $tmp.*
+
+if $tmp_sock_dir
+then
+rm -rf "$SOCK_DIR"
+fi
 }
 
 trap "_wrapup; exit \$status" 0 1 2 3 15
-- 
2.21.0

[PATCH 00/23] iotests: Add and use $SOCK_DIR

[Max Reitz]

Hi,

Perhaps the main reason we cannot run important tests such as 041 in CI
is that when they care Unix sockets in $TEST_DIR, the path may become
too long to connect to them.

To get by this problem, this series lets the check script create a new
temporary directory (mktemp -d) and then makes the iotests use it for
all Unix sockets.


Max Reitz (23):
  iotests: Introduce $SOCK_DIR
  iotests.py: Store socket files in $SOCK_DIR
  iotests.py: Add @base_dir to FilePaths etc.
  iotests: Filter $SOCK_DIR
  iotests: Let common.nbd create socket in $SOCK_DIR
  iotests/083: Create socket in $SOCK_DIR
  iotests/140: Create socket in $SOCK_DIR
  iotests/143: Create socket in $SOCK_DIR
  iotests/147: Create socket in $SOCK_DIR
  iotests/181: Create socket in $SOCK_DIR
  iotests/182: Create socket in $SOCK_DIR
  iotests/183: Create socket in $SOCK_DIR
  iotests/192: Create socket in $SOCK_DIR
  iotests/194: Create sockets in $SOCK_DIR
  iotests/201: Create socket in $SOCK_DIR
  iotests/205: Create socket in $SOCK_DIR
  iotests/208: Create socket in $SOCK_DIR
  iotests/209: Create socket in $SOCK_DIR
  iotests/222: Create socket in $SOCK_DIR
  iotests/223: Create socket in $SOCK_DIR
  iotests/240: Create socket in $SOCK_DIR
  iotests/267: Create socket in $SOCK_DIR
  iotests: Drop TEST_DIR filter from _filter_nbd

 python/qemu/machine.py   | 15 +++---
 python/qemu/qtest.py |  9 ++---
 tests/qemu-iotests/083   |  6 +++---
 tests/qemu-iotests/083.out   | 34 
 tests/qemu-iotests/140   |  8 
 tests/qemu-iotests/140.out   |  2 +-
 tests/qemu-iotests/143   |  6 +++---
 tests/qemu-iotests/143.out   |  2 +-
 tests/qemu-iotests/147   |  2 +-
 tests/qemu-iotests/181   |  2 +-
 tests/qemu-iotests/182   |  4 ++--
 tests/qemu-iotests/183   |  2 +-
 tests/qemu-iotests/192   |  4 ++--
 tests/qemu-iotests/192.out   |  2 +-
 tests/qemu-iotests/194   |  4 ++--
 tests/qemu-iotests/201   |  2 +-
 tests/qemu-iotests/205   |  2 +-
 tests/qemu-iotests/208   |  2 +-
 tests/qemu-iotests/209   |  3 ++-
 tests/qemu-iotests/222   |  2 +-
 tests/qemu-iotests/223   | 14 ++---
 tests/qemu-iotests/240   |  4 ++--
 tests/qemu-iotests/241   |  2 --
 tests/qemu-iotests/267   |  4 ++--
 tests/qemu-iotests/267.out   |  2 +-
 tests/qemu-iotests/check | 17 
 tests/qemu-iotests/common.filter |  7 +--
 tests/qemu-iotests/common.nbd|  2 +-
 tests/qemu-iotests/iotests.py| 16 ---
 29 files changed, 107 insertions(+), 74 deletions(-)

-- 
2.21.0

[PATCH 04/23] iotests: Filter $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/common.filter | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 9f418b4881..cd42f5e7e3 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -43,7 +43,8 @@ _filter_qom_path()
 # replace occurrences of the actual TEST_DIR value with TEST_DIR
 _filter_testdir()
 {
-$SED -e "s#$TEST_DIR/#TEST_DIR/#g"
+$SED -e "s#$TEST_DIR/#TEST_DIR/#g" \
+ -e "s#$SOCK_DIR/#SOCK_DIR/#g"
 }
 
 # replace occurrences of the actual IMGFMT value with IMGFMT
@@ -124,6 +125,7 @@ _filter_img_create()
 $SED -e "s#$REMOTE_TEST_DIR#TEST_DIR#g" \
 -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
 -e "s#$TEST_DIR#TEST_DIR#g" \
+-e "s#$SOCK_DIR#SOCK_DIR#g" \
 -e "s#$IMGFMT#IMGFMT#g" \
 -e 's#nbd:127.0.0.1:10810#TEST_DIR/t.IMGFMT#g' \
 -e "s# encryption=off##g" \
@@ -160,6 +162,7 @@ _filter_img_info()
 $SED -e "s#$REMOTE_TEST_DIR#TEST_DIR#g" \
 -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
 -e "s#$TEST_DIR#TEST_DIR#g" \
+-e "s#$SOCK_DIR#SOCK_DIR#g" \
 -e "s#$IMGFMT#IMGFMT#g" \
 -e 's#nbd://127.0.0.1:10810$#TEST_DIR/t.IMGFMT#g' \
 -e 's#json.*vdisk-id.*vxhs"}}#TEST_DIR/t.IMGFMT#' \
@@ -218,7 +221,8 @@ _filter_nbd()
 # Filter out the TCP port number since this changes between runs.
 $SED -e '/nbd\/.*\.c:/d' \
 -e 's#127\.0\.0\.1:[0-9]*#127.0.0.1:PORT#g' \
--e "s#?socket=$TEST_DIR#?socket=TEST_DIR#g" \
+-e "s#?socket=$SOCK_DIR#?socket=TEST_DIR#g" \
+-e "s#?socket=$SOCK_DIR#?socket=SOCK_DIR#g" \
 -e 's#\(foo\|PORT/\?\|.sock\): Failed to .*$#\1#'
 }
 
-- 
2.21.0

[PATCH 18/23] iotests/209: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/209 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/209 b/tests/qemu-iotests/209
index 259e991ec6..e0f464bcbe 100755
--- a/tests/qemu-iotests/209
+++ b/tests/qemu-iotests/209
@@ -24,7 +24,8 @@ from iotests import qemu_img_create, qemu_io, 
qemu_img_verbose, qemu_nbd, \
 
 iotests.verify_image_format(supported_fmts=['qcow2'])
 
-disk, nbd_sock = file_path('disk', 'nbd-sock')
+disk = file_path('disk')
+nbd_sock = file_path('nbd-sock', base_dir=iotests.sock_dir)
 nbd_uri = 'nbd+unix:///exp?socket=' + nbd_sock
 
 qemu_img_create('-f', iotests.imgfmt, disk, '1M')
-- 
2.21.0

[PATCH 17/23] iotests/208: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/208 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/208 b/tests/qemu-iotests/208
index 1e202388dc..546eb1de3e 100755
--- a/tests/qemu-iotests/208
+++ b/tests/qemu-iotests/208
@@ -26,7 +26,7 @@ iotests.verify_image_format(supported_fmts=['generic'])
 
 with iotests.FilePath('disk.img') as disk_img_path, \
  iotests.FilePath('disk-snapshot.img') as disk_snapshot_img_path, \
- iotests.FilePath('nbd.sock') as nbd_sock_path, \
+ iotests.FilePath('nbd.sock', iotests.sock_dir) as nbd_sock_path, \
  iotests.VM() as vm:
 
 img_size = '10M'
-- 
2.21.0

[PATCH 23/23] iotests: Drop TEST_DIR filter from _filter_nbd

[Max Reitz]

Sockets should be placed into $SOCK_DIR instead of $TEST_DIR, so remove
the $TEST_DIR filter from _filter_nbd.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/common.filter | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index cd42f5e7e3..f870e00e44 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -221,7 +221,6 @@ _filter_nbd()
 # Filter out the TCP port number since this changes between runs.
 $SED -e '/nbd\/.*\.c:/d' \
 -e 's#127\.0\.0\.1:[0-9]*#127.0.0.1:PORT#g' \
--e "s#?socket=$SOCK_DIR#?socket=TEST_DIR#g" \
 -e "s#?socket=$SOCK_DIR#?socket=SOCK_DIR#g" \
 -e 's#\(foo\|PORT/\?\|.sock\): Failed to .*$#\1#'
 }
-- 
2.21.0

[PATCH 09/23] iotests/147: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/147 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
index ab8480b9a4..03fc2fabcf 100755
--- a/tests/qemu-iotests/147
+++ b/tests/qemu-iotests/147
@@ -32,7 +32,7 @@ NBD_IPV6_PORT_START = NBD_PORT_END
 NBD_IPV6_PORT_END   = NBD_IPV6_PORT_START + 1024
 
 test_img = os.path.join(iotests.test_dir, 'test.img')
-unix_socket = os.path.join(iotests.test_dir, 'nbd.socket')
+unix_socket = os.path.join(iotests.sock_dir, 'nbd.socket')
 
 
 def flatten_sock_addr(crumpled_address):
-- 
2.21.0

[PATCH 12/23] iotests/183: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/183 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/183 b/tests/qemu-iotests/183
index 04fb344d08..bced83fae0 100755
--- a/tests/qemu-iotests/183
+++ b/tests/qemu-iotests/183
@@ -26,7 +26,7 @@ echo "QA output created by $seq"
 
 status=1 # failure is the default!
 
-MIG_SOCKET="${TEST_DIR}/migrate"
+MIG_SOCKET="${SOCK_DIR}/migrate"
 
 _cleanup()
 {
-- 
2.21.0

[PATCH 19/23] iotests/222: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/222 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/222 b/tests/qemu-iotests/222
index 0ead56d574..3f9f934ad8 100644
--- a/tests/qemu-iotests/222
+++ b/tests/qemu-iotests/222
@@ -48,7 +48,7 @@ remainder = [("0xd5", "0x108000",  "32k"), # Right-end of 
partial-left [1]
 
 with iotests.FilePath('base.img') as base_img_path, \
  iotests.FilePath('fleece.img') as fleece_img_path, \
- iotests.FilePath('nbd.sock') as nbd_sock_path, \
+ iotests.FilePath('nbd.sock', iotests.sock_dir) as nbd_sock_path, \
  iotests.VM() as vm:
 
 log('--- Setting up images ---')
-- 
2.21.0

[PATCH 20/23] iotests/223: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/223 | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/223 b/tests/qemu-iotests/223
index 2ba3d8124b..b5a80e50bb 100755
--- a/tests/qemu-iotests/223
+++ b/tests/qemu-iotests/223
@@ -28,7 +28,7 @@ _cleanup()
 nbd_server_stop
 _cleanup_test_img
 _cleanup_qemu
-rm -f "$TEST_DIR/nbd"
+rm -f "$SOCK_DIR/nbd"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -125,11 +125,11 @@ _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",
   "arguments":{"device":"n"}}' "error" # Attempt add without server
 _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-start",
   "arguments":{"addr":{"type":"unix",
-"data":{"path":"'"$TEST_DIR/nbd"'"' "return"
+"data":{"path":"'"$SOCK_DIR/nbd"'"' "return"
 _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-start",
   "arguments":{"addr":{"type":"unix",
-"data":{"path":"'"$TEST_DIR/nbd"1'"' "error" # Attempt second server
-$QEMU_NBD_PROG -L -k "$TEST_DIR/nbd"
+"data":{"path":"'"$SOCK_DIR/nbd"1'"' "error" # Attempt second server
+$QEMU_NBD_PROG -L -k "$SOCK_DIR/nbd"
 _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",
   "arguments":{"device":"n", "bitmap":"b"}}' "return"
 _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",
@@ -145,14 +145,14 @@ _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",
 _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add",
   "arguments":{"device":"n", "name":"n2", "writable":true,
   "bitmap":"b2"}}' "return"
-$QEMU_NBD_PROG -L -k "$TEST_DIR/nbd"
+$QEMU_NBD_PROG -L -k "$SOCK_DIR/nbd"
 
 echo
 echo "=== Contrast normal status to large granularity dirty-bitmap ==="
 echo
 
 QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
-IMG="driver=nbd,export=n,server.type=unix,server.path=$TEST_DIR/nbd"
+IMG="driver=nbd,export=n,server.type=unix,server.path=$SOCK_DIR/nbd"
 $QEMU_IO -r -c 'r -P 0x22 512 512' -c 'r -P 0 512k 512k' -c 'r -P 0x11 1m 1m' \
   -c 'r -P 0x33 2m 2m' --image-opts "$IMG" | _filter_qemu_io
 $QEMU_IMG map --output=json --image-opts \
@@ -164,7 +164,7 @@ echo
 echo "=== Contrast to small granularity dirty-bitmap ==="
 echo
 
-IMG="driver=nbd,export=n2,server.type=unix,server.path=$TEST_DIR/nbd"
+IMG="driver=nbd,export=n2,server.type=unix,server.path=$SOCK_DIR/nbd"
 $QEMU_IMG map --output=json --image-opts \
   "$IMG,x-dirty-bitmap=qemu:dirty-bitmap:b2" | _filter_qemu_img_map
 
-- 
2.21.0

[PATCH 05/23] iotests: Let common.nbd create socket in $SOCK_DIR

[Max Reitz]

In addition, drop the nbd_unix_socket assignment in 241 because it does
not really do anything.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/241| 2 --
 tests/qemu-iotests/common.nbd | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/241 b/tests/qemu-iotests/241
index 58b64ebf41..8dae8d39e4 100755
--- a/tests/qemu-iotests/241
+++ b/tests/qemu-iotests/241
@@ -23,8 +23,6 @@ echo "QA output created by $seq"
 
 status=1 # failure is the default!
 
-nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket
-
 _cleanup()
 {
 _cleanup_test_img
diff --git a/tests/qemu-iotests/common.nbd b/tests/qemu-iotests/common.nbd
index 24b01b60aa..a8cae8fe2c 100644
--- a/tests/qemu-iotests/common.nbd
+++ b/tests/qemu-iotests/common.nbd
@@ -19,7 +19,7 @@
 # along with this program.  If not, see .
 #
 
-nbd_unix_socket="${TEST_DIR}/qemu-nbd.sock"
+nbd_unix_socket="${SOCK_DIR}/qemu-nbd.sock"
 nbd_tcp_addr="127.0.0.1"
 nbd_pid_file="${TEST_DIR}/qemu-nbd.pid"
 nbd_stderr_fifo="${TEST_DIR}/qemu-nbd.fifo"
-- 
2.21.0

[PATCH 03/23] iotests.py: Add @base_dir to FilePaths etc.

[Max Reitz]

Specifying this optional parameter allows creating temporary files in
other directories than the test_dir; for example in sock_dir.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/iotests.py | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 49cd205a97..5373149ae1 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -375,10 +375,10 @@ class FilePaths(object):
 qemu_img('create', img_path, '1G')
 # migration_sock_path is automatically deleted
 """
-def __init__(self, names):
+def __init__(self, names, base_dir=test_dir):
 self.paths = []
 for name in names:
-self.paths.append(os.path.join(test_dir, file_pattern(name)))
+self.paths.append(os.path.join(base_dir, file_pattern(name)))
 
 def __enter__(self):
 return self.paths
@@ -395,8 +395,8 @@ class FilePath(FilePaths):
 """
 FilePath is a specialization of FilePaths that takes a single filename.
 """
-def __init__(self, name):
-super(FilePath, self).__init__([name])
+def __init__(self, name, base_dir=test_dir):
+super(FilePath, self).__init__([name], base_dir)
 
 def __enter__(self):
 return self.paths[0]
@@ -409,7 +409,7 @@ def file_path_remover():
 pass
 
 
-def file_path(*names):
+def file_path(*names, base_dir=test_dir):
 ''' Another way to get auto-generated filename that cleans itself up.
 
 Use is as simple as:
@@ -425,7 +425,7 @@ def file_path(*names):
 paths = []
 for name in names:
 filename = file_pattern(name)
-path = os.path.join(test_dir, filename)
+path = os.path.join(base_dir, filename)
 file_path_remover.paths.append(path)
 paths.append(path)
 
-- 
2.21.0

[PATCH 11/23] iotests/182: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/182 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/182 b/tests/qemu-iotests/182
index 7f494eb9bb..1ccb850055 100755
--- a/tests/qemu-iotests/182
+++ b/tests/qemu-iotests/182
@@ -31,7 +31,7 @@ _cleanup()
 {
 _cleanup_test_img
 rm -f "$TEST_IMG.overlay"
-rm -f "$TEST_DIR/nbd.socket"
+rm -f "$SOCK_DIR/nbd.socket"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -133,7 +133,7 @@ success_or_failure=y _send_qemu_cmd $QEMU_HANDLE \
   'addr': {
   'type': 'unix',
   'data': {
-  'path': '$TEST_DIR/nbd.socket'
+  'path': '$SOCK_DIR/nbd.socket'
   } } } }" \
 'return' \
 'error'
-- 
2.21.0

[PATCH 08/23] iotests/143: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/143 | 6 +++---
 tests/qemu-iotests/143.out | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/143 b/tests/qemu-iotests/143
index 92249ac8da..f649b36195 100755
--- a/tests/qemu-iotests/143
+++ b/tests/qemu-iotests/143
@@ -29,7 +29,7 @@ status=1  # failure is the default!
 _cleanup()
 {
 _cleanup_qemu
-rm -f "$TEST_DIR/nbd"
+rm -f "$SOCK_DIR/nbd"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -51,12 +51,12 @@ _send_qemu_cmd $QEMU_HANDLE \
 _send_qemu_cmd $QEMU_HANDLE \
 "{ 'execute': 'nbd-server-start',
'arguments': { 'addr': { 'type': 'unix',
-'data': { 'path': '$TEST_DIR/nbd' " \
+'data': { 'path': '$SOCK_DIR/nbd' " \
 'return'
 
 # This should just result in a client error, not in the server crashing
 $QEMU_IO_PROG -f raw -c quit \
-"nbd+unix:///no_such_export?socket=$TEST_DIR/nbd" 2>&1 \
+"nbd+unix:///no_such_export?socket=$SOCK_DIR/nbd" 2>&1 \
 | _filter_qemu_io | _filter_nbd
 
 _send_qemu_cmd $QEMU_HANDLE \
diff --git a/tests/qemu-iotests/143.out b/tests/qemu-iotests/143.out
index ee71b5aa42..037d34a409 100644
--- a/tests/qemu-iotests/143.out
+++ b/tests/qemu-iotests/143.out
@@ -1,7 +1,7 @@
 QA output created by 143
 {"return": {}}
 {"return": {}}
-qemu-io: can't open device nbd+unix:///no_such_export?socket=TEST_DIR/nbd: 
Requested export not available
+qemu-io: can't open device nbd+unix:///no_such_export?socket=SOCK_DIR/nbd: 
Requested export not available
 server reported: export 'no_such_export' not present
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
-- 
2.21.0

[PATCH 10/23] iotests/181: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/181 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/181 b/tests/qemu-iotests/181
index e317e63422..378c2899d1 100755
--- a/tests/qemu-iotests/181
+++ b/tests/qemu-iotests/181
@@ -26,7 +26,7 @@ echo "QA output created by $seq"
 
 status=1   # failure is the default!
 
-MIG_SOCKET="${TEST_DIR}/migrate"
+MIG_SOCKET="${SOCK_DIR}/migrate"
 
 _cleanup()
 {
-- 
2.21.0

[PATCH 21/23] iotests/240: Create socket in $SOCK_DIR

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/240 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/240 b/tests/qemu-iotests/240
index f73bc07d80..8b4337b58d 100755
--- a/tests/qemu-iotests/240
+++ b/tests/qemu-iotests/240
@@ -29,7 +29,7 @@ status=1  # failure is the default!
 
 _cleanup()
 {
-rm -f "$TEST_DIR/nbd"
+rm -f "$SOCK_DIR/nbd"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -135,7 +135,7 @@ echo
 run_qemu <

Re: [PATCH] qcow2-bitmaps: fix qcow2_can_store_new_dirty_bitmap

[Eric Blake]

On 6/7/19 1:53 PM, Vladimir Sementsov-Ogievskiy wrote:

07.06.2019 21:48, Vladimir Sementsov-Ogievskiy wrote:

qcow2_can_store_new_dirty_bitmap works wrong, as it considers only
bitmaps already stored in the qcow2 image and ignores persistent
BdrvDirtyBitmap objects.

So, let's instead count persistent BdrvDirtyBitmaps. We load all qcow2
bitmaps on open, so there should not be any bitmap in the image for
which we don't have BdrvDirtyBitmaps version. If it is - it's a kind of
corruption, and no reason to check for corruptions here (open() and
close() are better places for it).

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Hi!

Patch is better than discussing I thing, so here is my counter-suggestion for
"[PATCH 0/5] block/dirty-bitmap: check number and size constraints against queued 
bitmaps"
by John.

It's of course needs some additional refactoring, as first assert shows bad 
design,
I just wrote it in such manner to make minimum changes to fix the bug.




+assert(!bdrv_find_dirty_bitmap(bs, name));


exactly bad, this should be checked in qmp_block_dirty_bitmap_add(), before 
checks around
persistence. and aio_context_acquire may be dropped..

But anyway, idea is clear I think, consider it as RFC patch


Are you planning to post a v2, as this affects at least 
https://bugzilla.redhat.com/show_bug.cgi?id=1712636


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 02/23] iotests.py: Store socket files in $SOCK_DIR

[Eric Blake]

On 10/10/19 10:24 AM, Max Reitz wrote:

iotests.py itself does not store socket files, but it machine.py and


s/it //


qtest.py do.  iotests.py needs to pass the respective path to them, and
they need to adhere to it.

Signed-off-by: Max Reitz 
---
  python/qemu/machine.py| 15 ---
  python/qemu/qtest.py  |  9 ++---
  tests/qemu-iotests/iotests.py |  4 +++-
  3 files changed, 21 insertions(+), 7 deletions(-)



Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 12/23] iotests/183: Create socket in $SOCK_DIR

[Eric Blake]

On 10/10/19 10:24 AM, Max Reitz wrote:

Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/183 | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)



Reviewed-by: Eric Blake 


diff --git a/tests/qemu-iotests/183 b/tests/qemu-iotests/183
index 04fb344d08..bced83fae0 100755
--- a/tests/qemu-iotests/183
+++ b/tests/qemu-iotests/183
@@ -26,7 +26,7 @@ echo "QA output created by $seq"
  
  status=1 # failure is the default!
  
-MIG_SOCKET="${TEST_DIR}/migrate"

+MIG_SOCKET="${SOCK_DIR}/migrate"
  
  _cleanup()

  {



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 13/23] iotests/192: Create socket in $SOCK_DIR

[Eric Blake]

On 10/10/19 10:24 AM, Max Reitz wrote:

Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/192 | 4 ++--
  tests/qemu-iotests/192.out | 2 +-
  2 files changed, 3 insertions(+), 3 deletions(-)



Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 15/23] iotests/201: Create socket in $SOCK_DIR

[Eric Blake]

On 10/10/19 10:24 AM, Max Reitz wrote:

Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/201 | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)



Reviewed-by: Eric Blake 


diff --git a/tests/qemu-iotests/201 b/tests/qemu-iotests/201
index 7abf740fe4..86fa37e714 100755
--- a/tests/qemu-iotests/201
+++ b/tests/qemu-iotests/201
@@ -24,7 +24,7 @@ echo "QA output created by $seq"
  
  status=1	# failure is the default!
  
-MIG_SOCKET="${TEST_DIR}/migrate"

+MIG_SOCKET="${SOCK_DIR}/migrate"
  
  # get standard environment, filters and checks

  . ./common.rc



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 22/23] iotests/267: Create socket in $SOCK_DIR

[Eric Blake]

On 10/10/19 10:24 AM, Max Reitz wrote:

Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/267 | 4 ++--
  tests/qemu-iotests/267.out | 2 +-
  2 files changed, 3 insertions(+), 3 deletions(-)



Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v3 3/8] hw/ide/piix: Convert reset handler to DeviceReset

[Li Qiang]

Philippe Mathieu-Daudé  于2019年10月10日周四 下午9:16写道：

> The PIIX/IDE is a PCI device within a PIIX chipset, it will be reset
> when the PCI bus it stands on is reset.
>
> Convert its reset handler into a proper Device reset method.
>
> Signed-off-by: Philippe Mathieu-Daudé 
>


Reviewed-by: Li Qiang 


> ---
> v3: Also convert PIIX4 (Li Qiang)
> ---
>  hw/ide/piix.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index fba6bc8bff..db313dd3b1 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -30,7 +30,6 @@
>  #include "sysemu/block-backend.h"
>  #include "sysemu/blockdev.h"
>  #include "sysemu/dma.h"
> -#include "sysemu/reset.h"
>
>  #include "hw/ide/pci.h"
>  #include "trace.h"
> @@ -103,9 +102,9 @@ static void bmdma_setup_bar(PCIIDEState *d)
>  }
>  }
>
> -static void piix3_reset(void *opaque)
> +static void piix_ide_reset(DeviceState *dev)
>  {
> -PCIIDEState *d = opaque;
> +PCIIDEState *d = PCI_IDE(dev);
>  PCIDevice *pd = PCI_DEVICE(d);
>  uint8_t *pci_conf = pd->config;
>  int i;
> @@ -154,8 +153,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error
> **errp)
>
>  pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
>
> -qemu_register_reset(piix3_reset, d);
> -
>  bmdma_setup_bar(d);
>  pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, >bmdma_bar);
>
> @@ -247,6 +244,7 @@ static void piix3_ide_class_init(ObjectClass *klass,
> void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>
> +dc->reset = piix_ide_reset;
>  k->realize = pci_piix_ide_realize;
>  k->exit = pci_piix_ide_exitfn;
>  k->vendor_id = PCI_VENDOR_ID_INTEL;
> @@ -273,6 +271,7 @@ static void piix4_ide_class_init(ObjectClass *klass,
> void *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>  PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>
> +dc->reset = piix_ide_reset;
>  k->realize = pci_piix_ide_realize;
>  k->exit = pci_piix_ide_exitfn;
>  k->vendor_id = PCI_VENDOR_ID_INTEL;
> --
> 2.21.0
>
>