Re: [PATCH v7 00/32] Add subcluster allocation to qcow2

2020-05-25 Thread no-reply
Patchew URL: https://patchew.org/QEMU/cover.1590429901.git.be...@igalia.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN ===

Re: [PATCH v7 00/32] Add subcluster allocation to qcow2

2020-05-25 Thread no-reply
Patchew URL: https://patchew.org/QEMU/cover.1590429901.git.be...@igalia.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN ===

Re: [PATCH v7 00/32] Add subcluster allocation to qcow2

2020-05-25 Thread no-reply
Patchew URL: https://patchew.org/QEMU/cover.1590429901.git.be...@igalia.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN ===

Re: [PATCH] hw/block/pflash_cfi01: Limit maximum flash size to 256 MiB

2020-05-25 Thread Peter Maydell
On Mon, 25 May 2020 at 16:58, Philippe Mathieu-Daudé wrote: > > As of this commit, the biggest CFI01 NOR flash documented is > the Micron PC28F00BP33EF. Its size is 2 GiB (256 MiB). > > Actually this "2Gb device employs a virtual chip enable feature, > which combines two 1Gb die with a common

Re: [PATCH v3 2/2] vhost-user-blk: delay vhost_user_blk_disconnect

2020-05-25 Thread Raphael Norwitz
On Mon, May 25, 2020 at 4:58 AM Dima Stepanov wrote: > > On Wed, May 20, 2020 at 06:53:13PM +0300, Dima Stepanov wrote: > > A socket write during vhost-user communication may trigger a disconnect > > event, calling vhost_user_blk_disconnect() and clearing all the > > vhost_dev structures holding

Re: [PATCH] hw/block/pflash_cfi01: Limit maximum flash size to 256 MiB

2020-05-25 Thread Laszlo Ersek
On 05/25/20 17:58, Philippe Mathieu-Daudé wrote: > As of this commit, the biggest CFI01 NOR flash documented is > the Micron PC28F00BP33EF. Its size is 2 GiB (256 MiB). I don't understand what "2 GiB (256 MiB)" means; please clarify. > > Actually this "2Gb device employs a virtual chip enable

[PATCH v7 32/32] iotests: Add tests for qcow2 images with extended L2 entries

2020-05-25 Thread Alberto Garcia
Signed-off-by: Alberto Garcia --- tests/qemu-iotests/271 | 705 + tests/qemu-iotests/271.out | 603 +++ tests/qemu-iotests/group | 1 + 3 files changed, 1309 insertions(+) create mode 100755 tests/qemu-iotests/271 create

[PATCH v7 26/32] qcow2: Clear the L2 bitmap when allocating a compressed cluster

2020-05-25 Thread Alberto Garcia
Compressed clusters always have the bitmap part of the extended L2 entry set to 0. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz --- block/qcow2-cluster.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 2276cee6d6..deff838fe8

[PATCH v7 02/32] qcow2: Convert qcow2_get_cluster_offset() into qcow2_get_host_offset()

2020-05-25 Thread Alberto Garcia
qcow2_get_cluster_offset() takes an (unaligned) guest offset and returns the (aligned) offset of the corresponding cluster in the qcow2 image. In practice none of the callers need to know where the cluster starts so this patch makes the function calculate and return the final host offset

[PATCH v7 21/32] qcow2: Add subcluster support to qcow2_get_host_offset()

2020-05-25 Thread Alberto Garcia
The logic of this function remains pretty much the same, except that it uses count_contiguous_subclusters(), which combines the logic of count_contiguous_clusters() / count_contiguous_clusters_unallocated() and checks individual subclusters. qcow2_cluster_to_subcluster_type() is not necessary as

[PATCH v7 25/32] qcow2: Update L2 bitmap in qcow2_alloc_cluster_link_l2()

2020-05-25 Thread Alberto Garcia
The L2 bitmap needs to be updated after each write to indicate what new subclusters are now allocated. This needs to happen even if the cluster was already allocated and the L2 entry was otherwise valid. In some cases however a write operation doesn't need change the L2 bitmap (because all

[PATCH v7 27/32] qcow2: Add subcluster support to handle_alloc_space()

2020-05-25 Thread Alberto Garcia
The bdrv_co_pwrite_zeroes() call here fills complete clusters with zeroes, but it can happen that some subclusters are not part of the write request or the copy-on-write. This patch makes sure that only the affected subclusters are overwritten. A potential improvement would be to also fill with

[PATCH v7 11/32] qcow2: Add offset_into_subcluster() and size_to_subclusters()

2020-05-25 Thread Alberto Garcia
Like offset_into_cluster() and size_to_clusters(), but for subclusters. Signed-off-by: Alberto Garcia Reviewed-by: Eric Blake --- block/qcow2.h | 10 ++ 1 file changed, 10 insertions(+) diff --git a/block/qcow2.h b/block/qcow2.h index ca73ac9b67..79c4f82383 100644 --- a/block/qcow2.h

[PATCH v7 12/32] qcow2: Add l2_entry_size()

2020-05-25 Thread Alberto Garcia
qcow2 images with subclusters have 128-bit L2 entries. The first 64 bits contain the same information as traditional images and the last 64 bits form a bitmap with the status of each individual subcluster. Because of that we cannot assume that L2 entries are sizeof(uint64_t) anymore. This

[PATCH v7 30/32] qcow2: Add the 'extended_l2' option and the QCOW2_INCOMPAT_EXTL2 bit

2020-05-25 Thread Alberto Garcia
Now that the implementation of subclusters is complete we can finally add the necessary options to create and read images with this feature, which we call "extended L2 entries". Signed-off-by: Alberto Garcia --- qapi/block-core.json | 7 +++ block/qcow2.h| 8

[PATCH v7 31/32] qcow2: Assert that expand_zero_clusters_in_l1() does not support subclusters

2020-05-25 Thread Alberto Garcia
This function is only used by qcow2_expand_zero_clusters() to downgrade a qcow2 image to a previous version. It is however not possible to downgrade an image with extended L2 entries because older versions of qcow2 do not have this feature. Signed-off-by: Alberto Garcia Reviewed-by: Eric Blake

[PATCH v7 18/32] qcow2: Replace QCOW2_CLUSTER_* with QCOW2_SUBCLUSTER_*

2020-05-25 Thread Alberto Garcia
In order to support extended L2 entries some functions of the qcow2 driver need to start dealing with subclusters instead of clusters. qcow2_get_host_offset() is modified to return the subcluster type instead of the cluster type, and all callers are updated to replace all values of

[PATCH v7 20/32] qcow2: Add subcluster support to calculate_l2_meta()

2020-05-25 Thread Alberto Garcia
If an image has subclusters then there are more copy-on-write scenarios that we need to consider. Let's say we have a write request from the middle of subcluster #3 until the end of the cluster: 1) If we are writing to a newly allocated cluster then we need copy-on-write. The previous contents

[PATCH v7 04/32] qcow2: Split cluster_needs_cow() out of count_cow_clusters()

2020-05-25 Thread Alberto Garcia
We are going to need it in other places. Signed-off-by: Alberto Garcia Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Max Reitz --- block/qcow2-cluster.c | 34 +++--- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/block/qcow2-cluster.c

[PATCH v7 17/32] qcow2: Add cluster type parameter to qcow2_get_host_offset()

2020-05-25 Thread Alberto Garcia
This function returns an integer that can be either an error code or a cluster type (a value from the QCow2ClusterType enum). We are going to start using subcluster types instead of cluster types in some functions so it's better to use the exact data types instead of integers for clarity and in

[PATCH v7 05/32] qcow2: Process QCOW2_CLUSTER_ZERO_ALLOC clusters in handle_copied()

2020-05-25 Thread Alberto Garcia
When writing to a qcow2 file there are two functions that take a virtual offset and return a host offset, possibly allocating new clusters if necessary: - handle_copied() looks for normal data clusters that are already allocated and have a reference count of 1. In those clusters we

[PATCH v7 19/32] qcow2: Handle QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC

2020-05-25 Thread Alberto Garcia
When dealing with subcluster types there is a new value called QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC that has no equivalent in QCow2ClusterType. This patch handles that value in all places where subcluster types are processed. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Reviewed-by:

[PATCH v7 03/32] qcow2: Add calculate_l2_meta()

2020-05-25 Thread Alberto Garcia
handle_alloc() creates a QCowL2Meta structure in order to update the image metadata and perform the necessary copy-on-write operations. This patch moves that code to a separate function so it can be used from other places. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz ---

[PATCH v7 10/32] qcow2: Add offset_to_sc_index()

2020-05-25 Thread Alberto Garcia
For a given offset, return the subcluster number within its cluster (i.e. with 32 subclusters per cluster it returns a number between 0 and 31). Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/qcow2.h | 5 + 1 file changed, 5

[PATCH v7 29/32] qcow2: Add subcluster support to qcow2_measure()

2020-05-25 Thread Alberto Garcia
Extended L2 entries are bigger than normal L2 entries so this has an impact on the amount of metadata needed for a qcow2 file. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz --- block/qcow2.c | 20 +--- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git

[PATCH v7 22/32] qcow2: Add subcluster support to zero_in_l2_slice()

2020-05-25 Thread Alberto Garcia
The QCOW_OFLAG_ZERO bit that indicates that a cluster reads as zeroes is only used in standard L2 entries. Extended L2 entries use individual 'all zeroes' bits for each subcluster. This must be taken into account when updating the L2 entry and also when deciding that an existing entry does not

[PATCH v7 14/32] qcow2: Add QCow2SubclusterType and qcow2_get_subcluster_type()

2020-05-25 Thread Alberto Garcia
This patch adds QCow2SubclusterType, which is the subcluster-level version of QCow2ClusterType. All QCOW2_SUBCLUSTER_* values have the the same meaning as their QCOW2_CLUSTER_* equivalents (when they exist). See below for details and caveats. In images without extended L2 entries clusters are

[PATCH v7 16/32] qcow2: Add qcow2_cluster_is_allocated()

2020-05-25 Thread Alberto Garcia
This helper function tells us if a cluster is allocated (that is, there is an associated host offset for it). Signed-off-by: Alberto Garcia Reviewed-by: Eric Blake --- block/qcow2.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/block/qcow2.h b/block/qcow2.h index

[PATCH v7 00/32] Add subcluster allocation to qcow2

2020-05-25 Thread Alberto Garcia
Hi, this is the same as v6 but with a single fix in patch 23. Please refer to the cover letter of the first version for a full description of the patches: https://lists.gnu.org/archive/html/qemu-block/2019-10/msg00983.html The big change (in v6) is that the code does not need to iterate

[PATCH v7 24/32] qcow2: Add subcluster support to check_refcounts_l2()

2020-05-25 Thread Alberto Garcia
Setting the QCOW_OFLAG_ZERO bit of the L2 entry is forbidden if an image has subclusters. Instead, the individual 'all zeroes' bits must be used. Signed-off-by: Alberto Garcia Reviewed-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Eric Blake --- block/qcow2-refcount.c

[PATCH v7 13/32] qcow2: Update get/set_l2_entry() and add get/set_l2_bitmap()

2020-05-25 Thread Alberto Garcia
Extended L2 entries are 128-bit wide: 64 bits for the entry itself and 64 bits for the subcluster allocation bitmap. In order to support them correctly get/set_l2_entry() need to be updated so they take the entry width into account in order to calculate the correct offset. This patch also adds

[PATCH v7 08/32] qcow2: Add dummy has_subclusters() function

2020-05-25 Thread Alberto Garcia
This function will be used by the qcow2 code to check if an image has subclusters or not. At the moment this simply returns false. Once all patches needed for subcluster support are ready then QEMU will be able to create and read images with subclusters and this function will return the actual

[PATCH v7 15/32] qcow2: Add qcow2_get_subcluster_range_type()

2020-05-25 Thread Alberto Garcia
There are situations in which we want to know how many contiguous subclusters of the same type there are in a given cluster. This can be done by simply iterating over the subclusters and repeatedly calling qcow2_get_subcluster_type() for each one of them. However once we determined the type of a

[PATCH v7 23/32] qcow2: Add subcluster support to discard_in_l2_slice()

2020-05-25 Thread Alberto Garcia
Two things need to be taken into account here: 1) With full_discard == true the L2 entry must be cleared completely. This also includes the L2 bitmap if the image has extended L2 entries. 2) With full_discard == false we have to make the discarded cluster read back as zeroes. With

[PATCH v7 06/32] qcow2: Add get_l2_entry() and set_l2_entry()

2020-05-25 Thread Alberto Garcia
The size of an L2 entry is 64 bits, but if we want to have subclusters we need extended L2 entries. This means that we have to access L2 tables and slices differently depending on whether an image has extended L2 entries or not. This patch replaces all l2_slice[] accesses with calls to

[PATCH v7 28/32] qcow2: Add subcluster support to qcow2_co_pwrite_zeroes()

2020-05-25 Thread Alberto Garcia
This works now at the subcluster level and pwrite_zeroes_alignment is updated accordingly. qcow2_cluster_zeroize() is turned into qcow2_subcluster_zeroize() with the following changes: - The request can now be subcluster-aligned. - The cluster-aligned body of the request is still zeroized

[PATCH v7 09/32] qcow2: Add subcluster-related fields to BDRVQcow2State

2020-05-25 Thread Alberto Garcia
This patch adds the following new fields to BDRVQcow2State: - subclusters_per_cluster: Number of subclusters in a cluster - subcluster_size: The size of each subcluster, in bytes - subcluster_bits: No. of bits so 1 << subcluster_bits = subcluster_size Images without subclusters are treated as if

[PATCH v7 07/32] qcow2: Document the Extended L2 Entries feature

2020-05-25 Thread Alberto Garcia
Subcluster allocation in qcow2 is implemented by extending the existing L2 table entries and adding additional information to indicate the allocation status of each subcluster. This patch documents the changes to the qcow2 format and how they affect the calculation of the L2 cache size.

[PATCH v7 01/32] qcow2: Make Qcow2AioTask store the full host offset

2020-05-25 Thread Alberto Garcia
The file_cluster_offset field of Qcow2AioTask stores a cluster-aligned host offset. In practice this is not very useful because all users(*) of this structure need the final host offset into the cluster, which they calculate using host_offset = file_cluster_offset + offset_into_cluster(s,

Re: [RFC PATCH 2/3] block: Allow bdrv_run_co() from different AioContext

2020-05-25 Thread Kevin Wolf
Am 25.05.2020 um 16:18 hat Stefan Reiter geschrieben: > On 5/12/20 4:43 PM, Kevin Wolf wrote: > > Coroutine functions that are entered through bdrv_run_co() are already > > safe to call from synchronous code in a different AioContext because > > bdrv_coroutine_enter() will schedule them in the

Re: [PATCH v6 00/32] Add subcluster allocation to qcow2

2020-05-25 Thread Alberto Garcia
On Sun 24 May 2020 06:34:17 PM CEST, no-re...@patchew.org wrote: > /tmp/qemu-test/src/block/qcow2-cluster.c:1912:54: error: implicit conversion > from enumeration type 'QCow2ClusterType' (aka 'enum QCow2ClusterType') to > different enumeration type 'enum qcow2_discard_type' >

[PATCH] hw/block/pflash_cfi01: Limit maximum flash size to 256 MiB

2020-05-25 Thread Philippe Mathieu-Daudé
As of this commit, the biggest CFI01 NOR flash documented is the Micron PC28F00BP33EF. Its size is 2 GiB (256 MiB). Actually this "2Gb device employs a virtual chip enable feature, which combines two 1Gb die with a common chip enable". Since we do not want to model unrealistic hardware, cap the

[PATCH v4 3/4] chardev/char-socket.c: Add yank feature

2020-05-25 Thread Lukas Straub
Register a yank function to shutdown the socket on yank. Signed-off-by: Lukas Straub --- Makefile.objs | 1 + chardev/char-socket.c | 24 2 files changed, 25 insertions(+) diff --git a/Makefile.objs b/Makefile.objs index 8e403b81f3..5582f4eda9 100644 ---

[PATCH v4 0/4] Introduce 'yank' oob qmp command to recover from hanging qemu

2020-05-25 Thread Lukas Straub
Hello Everyone, In many cases, if qemu has a network connection (qmp, migration, chardev, etc.) to some other server and that server dies or hangs, qemu hangs too. These patches introduce the new 'yank' out-of-band qmp command to recover from these kinds of hangs. The different subsystems register

[PATCH v4 2/4] block/nbd.c: Add yank feature

2020-05-25 Thread Lukas Straub
Register a yank function which shuts down the socket and sets s->state = NBD_CLIENT_QUIT. This is the same behaviour as if an error occured. Signed-off-by: Lukas Straub --- Makefile.objs | 1 + block/nbd.c | 101 -- 2 files changed, 65

[PATCH v4 1/4] Introduce yank feature

2020-05-25 Thread Lukas Straub
The yank feature allows to recover from hanging qemu by "yanking" at various parts. Other qemu systems can register themselves and multiple yank functions. Then all yank functions for selected instances can be called by the 'yank' out-of-band qmp command. Available instances can be queried by a

[PATCH v4 4/4] migration: Add yank feature

2020-05-25 Thread Lukas Straub
Register yank functions on sockets to shut them down. Signed-off-by: Lukas Straub --- Makefile.objs | 1 + migration/channel.c | 12 migration/migration.c | 18 +- migration/multifd.c | 10 ++

Re: [PATCH v4 3/3] block: make BlockConf.*_size properties 32-bit

2020-05-25 Thread Kevin Wolf
Am 20.05.2020 um 23:50 hat Roman Kagan geschrieben: > On Wed, May 20, 2020 at 05:54:44PM +0200, Kevin Wolf wrote: > > Am 20.05.2020 um 10:06 hat Roman Kagan geschrieben: > > > Devices (virtio-blk, scsi, etc.) and the block layer are happy to use > > > 32-bit for logical_block_size,

[PATCH v3 9/9] qapi/misc: Restrict device memory commands to machine code

2020-05-25 Thread Philippe Mathieu-Daudé
Acked-by: Igor Mammedov Signed-off-by: Philippe Mathieu-Daudé --- qapi/machine.json | 131 +++ qapi/misc.json | 132 include/hw/mem/memory-device.h | 1 + include/hw/virtio/virtio-pmem.h | 2 +- 4

[PATCH v3 8/9] qapi/misc: Restrict PCI commands to machine code

2020-05-25 Thread Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé --- qapi/machine.json | 304 ++ qapi/misc.json| 304 -- hw/pci/pci-stub.c | 2 +- hw/pci/pci.c | 2 +- 4 files changed, 306 insertions(+), 306 deletions(-)

[PATCH v3 7/9] qapi/misc: Restrict ACPI commands to machine code

2020-05-25 Thread Philippe Mathieu-Daudé
Acked-by: Igor Mammedov Signed-off-by: Philippe Mathieu-Daudé --- qapi/machine.json| 154 +++ qapi/misc.json | 154 --- include/hw/acpi/acpi_dev_interface.h | 2 +- hw/acpi/core.c |

[PATCH v3 2/9] qapi/misc: Restrict LostTickPolicy enum to machine code

2020-05-25 Thread Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé --- qapi/machine.json| 32 qapi/misc.json | 32 include/hw/rtc/mc146818rtc.h | 2 +- hw/core/qdev-properties.c| 1 + hw/i386/kvm/i8254.c | 2 +- 5

[PATCH v3 3/9] qapi/misc.json: Correct balloon documentation

2020-05-25 Thread Philippe Mathieu-Daudé
The documentation incorrectly uses the "size of the balloon" description when it should be "logical size of the VM". Fix it. The relation between both values is: logical_vm_size = vm_ram_size - balloon_size Reported-by: David Hildenbrand Suggested-by: David Hildenbrand Signed-off-by:

[PATCH v3 1/9] target/i386: Restrict X86CPUFeatureWord to X86 targets

2020-05-25 Thread Philippe Mathieu-Daudé
Move out x86-specific structures from generic machine code. Acked-by: Richard Henderson Signed-off-by: Philippe Mathieu-Daudé --- qapi/machine-target.json | 45 ++ qapi/machine.json | 42 --- target/i386/cpu.c

[PATCH v3 6/9] qapi/misc: Move query-uuid command to machine code

2020-05-25 Thread Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé --- qapi/machine.json | 30 ++ qapi/misc.json| 30 -- block/iscsi.c | 2 +- stubs/uuid.c | 2 +- 4 files changed, 32 insertions(+), 32 deletions(-) diff --git a/qapi/machine.json

[PATCH v3 4/9] qapi/misc: Restrict balloon-related commands to machine code

2020-05-25 Thread Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé --- qapi/machine.json | 90 ++ qapi/misc.json | 90 -- include/sysemu/balloon.h | 2 +- balloon.c | 2 +- hw/virtio/virtio-balloon.c | 2 +-

[PATCH v3 5/9] qapi/misc: Restrict query-vm-generation-id command to machine code

2020-05-25 Thread Philippe Mathieu-Daudé
Acked-by: Igor Mammedov Signed-off-by: Philippe Mathieu-Daudé --- qapi/machine.json | 20 qapi/misc.json| 21 - hw/acpi/vmgenid.c | 2 +- stubs/vmgenid.c | 2 +- 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/qapi/machine.json

[PATCH v3 0/9] user-mode: Prune build dependencies (part 2)

2020-05-25 Thread Philippe Mathieu-Daudé
Missing review: - #2 restrict LostTickPolicy to x86 - #3 correct balloon documentation (new) - #4 restrict balloon to machine - #6 move query-uuid to machine - #8 restrict PCI commands This is the second part of a series reducing user-mode dependencies. By stripping out unused code, the build and

Re: [RFC v2] nbd/proto: introduce extended request and 64bit commands

2020-05-25 Thread Vladimir Sementsov-Ogievskiy
25.05.2020 16:37, Wouter Verhelst wrote: Hi guys, Sorry for slacking here. On Wed, Mar 18, 2020 at 07:22:39AM -0500, Eric Blake wrote: On 3/18/20 3:04 AM, Wouter Verhelst wrote: On Wed, Mar 18, 2020 at 09:19:45AM +0300, Vladimir Sementsov-Ogievskiy wrote: OK, understand. Reasonable thing,

Re: [RFC PATCH 2/3] block: Allow bdrv_run_co() from different AioContext

2020-05-25 Thread Stefan Reiter
On 5/12/20 4:43 PM, Kevin Wolf wrote: Coroutine functions that are entered through bdrv_run_co() are already safe to call from synchronous code in a different AioContext because bdrv_coroutine_enter() will schedule them in the context of the node. However, the coroutine fastpath still requires

Re: [PATCH v2 2/8] qapi/misc: Restrict LostTickPolicy enum to machine code

2020-05-25 Thread Philippe Mathieu-Daudé
ping? On 3/16/20 1:03 AM, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé > --- > qapi/machine.json| 32 > qapi/misc.json | 32 > include/hw/rtc/mc146818rtc.h | 2 +- >

Re: [PATCH v2 7/8] qapi/misc: Restrict PCI commands to machine code

2020-05-25 Thread Philippe Mathieu-Daudé
ping? On 3/16/20 1:03 AM, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé > --- > qapi/machine.json | 304 ++ > qapi/misc.json| 304 -- > hw/pci/pci-stub.c | 2 +- > hw/pci/pci.c

Re: [PATCH v4 0/5] coroutines: generate wrapper code

2020-05-25 Thread Vladimir Sementsov-Ogievskiy
25.05.2020 16:14, no-re...@patchew.org wrote: Patchew URL:https://patchew.org/QEMU/20200525100801.13859-1-vsement...@virtuozzo.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can

Re: [RFC v2] nbd/proto: introduce extended request and 64bit commands

2020-05-25 Thread Wouter Verhelst
Hi guys, Sorry for slacking here. On Wed, Mar 18, 2020 at 07:22:39AM -0500, Eric Blake wrote: > On 3/18/20 3:04 AM, Wouter Verhelst wrote: > > On Wed, Mar 18, 2020 at 09:19:45AM +0300, Vladimir Sementsov-Ogievskiy > > wrote: > > > > OK, understand. Reasonable thing, agreed. I'll resend. > > >

Re: [PATCH v4 0/5] coroutines: generate wrapper code

2020-05-25 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200525100801.13859-1-vsement...@virtuozzo.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT

[PATCH v4 4/5] block: drop bdrv_prwv

2020-05-25 Thread Vladimir Sementsov-Ogievskiy
Now, when we are not more paying extra code for coroutine wrappers, there no more sence in extra indirection layer: bdrv_prwv(). Let's drop it and instead genereate pure bdrv_preadv() and bdrv_pwritev(). Currently, bdrv_pwritev() and bdrv_preadv() are returning bytes on success, auto generated

[PATCH v4 3/5] block: generate coroutine-wrapper code

2020-05-25 Thread Vladimir Sementsov-Ogievskiy
We have a very frequent pattern of creating coroutine from function with several arguments: - create structure to pack parameters - create _entry function to call original function taking parameters from struct - do different magic to handle completion: set ret to NOT_DONE or

[PATCH v4 1/5] block/io: refactor coroutine wrappers

2020-05-25 Thread Vladimir Sementsov-Ogievskiy
Most of our coroutine wrappers already follow this convention: We have 'coroutine_fn bdrv_co_()' as the core function, and a wrapper 'bdrv_()' which does a polling loop. The only outsiders are the bdrv_prwv_co and bdrv_common_block_status_above wrappers. Let's refactor them to behave as the

[PATCH v4 2/5] block: declare some coroutine functions in block/coroutines.h

2020-05-25 Thread Vladimir Sementsov-Ogievskiy
We are going to keep coroutine-wrappers code (structure-packing parameters, BDRV_POLL wrapper functions) in a separate auto-generated files. So, we'll need a header with declaration of original _co_ functions, for those which are static now. As well, we'll need declarations for wrapper functions.

[PATCH v4 0/5] coroutines: generate wrapper code

2020-05-25 Thread Vladimir Sementsov-Ogievskiy
Hi all! The aim of the series is to reduce code-duplication and writing parameters structure-packing by hand around coroutine function wrappers. It's an alternative to "[PATCH v3] block: Factor out bdrv_run_co()" patch. Benefits: - no code duplication - less indirection v4: 01: wording in

[PATCH v4 5/5] block/io: refactor save/load vmstate

2020-05-25 Thread Vladimir Sementsov-Ogievskiy
Like for read/write in a previous commit, drop extra indirection layer, generate directly bdrv_readv_vmstate() and bdrv_writev_vmstate(). Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/coroutines.h| 10 +++ include/block/block.h | 6 ++-- block/io.c| 67

Re: [PATCH v3 2/2] vhost-user-blk: delay vhost_user_blk_disconnect

2020-05-25 Thread Dima Stepanov
On Wed, May 20, 2020 at 06:53:13PM +0300, Dima Stepanov wrote: > A socket write during vhost-user communication may trigger a disconnect > event, calling vhost_user_blk_disconnect() and clearing all the > vhost_dev structures holding data that vhost-user functions expect to > remain valid to roll

Re: [PATCH v3 2/2] vhost-user-blk: delay vhost_user_blk_disconnect

2020-05-25 Thread Dima Stepanov
On Mon, May 25, 2020 at 12:00:10AM -0400, Raphael Norwitz wrote: > I'm mostly happy with this. A couple comments. > > On Wed, May 20, 2020 at 11:54 AM Dima Stepanov > wrote: > > > > A socket write during vhost-user communication may trigger a disconnect > > event, calling

Re: [PATCH 7/7] block/nvme: support nested aio_poll()

2020-05-25 Thread Sergio Lopez
On Tue, May 19, 2020 at 06:11:38PM +0100, Stefan Hajnoczi wrote: > QEMU block drivers are supposed to support aio_poll() from I/O > completion callback functions. This means completion processing must be > re-entrant. > > The standard approach is to schedule a BH during completion processing >

Re: [PATCH v3 2/2] vhost-user-blk: delay vhost_user_blk_disconnect

2020-05-25 Thread Dima Stepanov
On Mon, May 25, 2020 at 11:31:16AM +0800, Jason Wang wrote: > > On 2020/5/20 下午11:53, Dima Stepanov wrote: > >A socket write during vhost-user communication may trigger a disconnect > >event, calling vhost_user_blk_disconnect() and clearing all the > >vhost_dev structures holding data that

Re: [PATCH v6 07/20] hw/block/nvme: fix pin-based interrupt behavior

2020-05-25 Thread Klaus Jensen
On May 18 07:02, Klaus Jensen wrote: > On May 14 06:45, Klaus Jensen wrote: > > From: Klaus Jensen > > > > First, since the device only supports MSI-X or pin-based interrupt, if > > MSI-X is not enabled, it should not accept interrupt vectors different > > from 0 when creating completion queues.

Re: [PATCH 6/7] block/nvme: keep BDRVNVMeState pointer in NVMeQueuePair

2020-05-25 Thread Sergio Lopez
On Tue, May 19, 2020 at 06:11:37PM +0100, Stefan Hajnoczi wrote: > Passing around both BDRVNVMeState and NVMeQueuePair is unwiedly. Reduce > the number of function arguments by keeping the BDRVNVMeState pointer in > NVMeQueuePair. This will come in handly when a BH is introduced in a > later patch

Re: [PATCH 5/7] block/nvme: clarify that free_req_queue is protected by q->lock

2020-05-25 Thread Sergio Lopez
On Tue, May 19, 2020 at 06:11:36PM +0100, Stefan Hajnoczi wrote: > Existing users access free_req_queue under q->lock. Document this. > > Signed-off-by: Stefan Hajnoczi > --- > block/nvme.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Sergio Lopez signature.asc

Re: [PATCH 3/7] block/nvme: don't access CQE after moving cq.head

2020-05-25 Thread Sergio Lopez
On Tue, May 19, 2020 at 06:11:34PM +0100, Stefan Hajnoczi wrote: > Do not access a CQE after incrementing q->cq.head and releasing q->lock. > It is unlikely that this causes problems in practice but it's a latent > bug. > > The reason why it should be safe at the moment is that completion >

Re: [PATCH 4/7] block/nvme: switch to a NVMeRequest freelist

2020-05-25 Thread Sergio Lopez
On Tue, May 19, 2020 at 06:11:35PM +0100, Stefan Hajnoczi wrote: > There are three issues with the current NVMeRequest->busy field: > 1. The busy field is accidentally accessed outside q->lock when request >submission fails. > 2. Waiters on free_req_queue are not woken when a request is

Re: [PATCH 2/7] block/nvme: drop tautologous assertion

2020-05-25 Thread Sergio Lopez
On Tue, May 19, 2020 at 06:11:33PM +0100, Stefan Hajnoczi wrote: > nvme_process_completion() explicitly checks cid so the assertion that > follows is always true: > > if (cid == 0 || cid > NVME_QUEUE_SIZE) { > ... > continue; > } > assert(cid <= NVME_QUEUE_SIZE); > >

Re: [PATCH 1/7] block/nvme: poll queues without q->lock

2020-05-25 Thread Sergio Lopez
On Tue, May 19, 2020 at 06:11:32PM +0100, Stefan Hajnoczi wrote: > A lot of CPU time is spent simply locking/unlocking q->lock during > polling. Check for completion outside the lock to make q->lock disappear > from the profile. > > Signed-off-by: Stefan Hajnoczi > --- > block/nvme.c | 12

Re: [PATCH v6 4/5] qemu-img: Add convert --bitmaps option

2020-05-25 Thread Vladimir Sementsov-Ogievskiy
21.05.2020 22:21, Eric Blake wrote: Make it easier to copy all the persistent bitmaps of (the top layer of) a source image along with its guest-visible contents, by adding a boolean flag for use with qemu-img convert. This is basically shorthand, as the same effect could be accomplished with a

Re: [PATCH v6 3/5] qemu-img: Factor out code for merging bitmaps

2020-05-25 Thread Vladimir Sementsov-Ogievskiy
21.05.2020 22:21, Eric Blake wrote: The next patch will add another client that wants to merge dirty bitmaps; it will be easier to refactor the code to construct the QAPI struct correctly into a helper function. Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best

Re: [PATCH v6 2/5] qcow2: Expose bitmaps' size during measure

2020-05-25 Thread Vladimir Sementsov-Ogievskiy
21.05.2020 22:21, Eric Blake wrote: It's useful to know how much space can be occupied by qcow2 persistent bitmaps, even though such metadata is unrelated to the guest-visible data. Report this value as an additional QMP field, present when measuring an existing image and output format that