Re: [PATCH v2 1/6] virtio/virtio-pci: Handle extra notification data

2024-03-13 Thread Jason Wang
On Wed, Mar 13, 2024 at 7:55 PM Jonah Palmer wrote: > > Add support to virtio-pci devices for handling the extra data sent > from the driver to the device when the VIRTIO_F_NOTIFICATION_DATA > transport feature has been negotiated. > > The extra data that's passed to the virtio-pci device when

Re: [PATCH for-9.0] mirror: Don't call job_pause_point() under graph lock

2024-03-13 Thread Eric Blake
On Wed, Mar 13, 2024 at 04:30:00PM +0100, Kevin Wolf wrote: > Calling job_pause_point() while holding the graph reader lock > potentially results in a deadlock: bdrv_graph_wrlock() first drains > everything, including the mirror job, which pauses it. The job is only > unpaused at the end of the

Re: [PATCH-for-9.0 3/4] qtest/libqos: Un-inline size_to_prdtl()

2024-03-13 Thread Philippe Mathieu-Daudé
On 13/3/24 20:39, Peter Maydell wrote: On Wed, 13 Mar 2024 at 18:50, Philippe Mathieu-Daudé wrote: See previous commit and commit 9de9fa5cf2 ("Avoid using inlined functions with external linkage") for rationale. Signed-off-by: Philippe Mathieu-Daudé --- tests/qtest/libqos/ahci.c | 2 +-

Re: [PATCH-for-9.0 4/4] meson: Enable -Wstatic-in-inline

2024-03-13 Thread Richard Henderson
On 3/13/24 08:49, Philippe Mathieu-Daudé wrote: Compilers are clever enough to inline code when necessary. The only case we accept an inline function is static in header (we use C, not C++). Add the -Wstatic-in-inline CPPFLAG to prevent public and inline function to be added in the code base.

Re: [PATCH-for-9.0 2/4] accel/hvf: Un-inline hvf_arch_supports_guest_debug()

2024-03-13 Thread Richard Henderson
On 3/13/24 08:49, Philippe Mathieu-Daudé wrote: See previous commit and commit 9de9fa5cf2 ("Avoid using inlined functions with external linkage") for rationale. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/hvf/hvf.c | 2 +- target/i386/hvf/hvf.c | 2 +- 2 files changed, 2

Re: [PATCH-for-9.0 2/4] accel/hvf: Un-inline hvf_arch_supports_guest_debug()

2024-03-13 Thread Peter Maydell
On Wed, 13 Mar 2024 at 18:50, Philippe Mathieu-Daudé wrote: > > See previous commit and commit 9de9fa5cf2 ("Avoid using inlined > functions with external linkage") for rationale. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell thanks -- PMM

Re: [PATCH-for-9.0 3/4] qtest/libqos: Un-inline size_to_prdtl()

2024-03-13 Thread Peter Maydell
On Wed, 13 Mar 2024 at 18:50, Philippe Mathieu-Daudé wrote: > > See previous commit and commit 9de9fa5cf2 ("Avoid using inlined > functions with external linkage") for rationale. > > Signed-off-by: Philippe Mathieu-Daudé > --- > tests/qtest/libqos/ahci.c | 2 +- > 1 file changed, 1

[PATCH-for-9.0 2/4] accel/hvf: Un-inline hvf_arch_supports_guest_debug()

2024-03-13 Thread Philippe Mathieu-Daudé
See previous commit and commit 9de9fa5cf2 ("Avoid using inlined functions with external linkage") for rationale. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/hvf/hvf.c | 2 +- target/i386/hvf/hvf.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git

[PATCH-for-9.0 1/4] hw/arm/smmu: Avoid using inlined functions with external linkage again

2024-03-13 Thread Philippe Mathieu-Daudé
Similarly to commit 9de9fa5cf2 ("hw/arm/smmu-common: Avoid using inlined functions with external linkage"): None of our code base require / use inlined functions with external linkage. Some places use internal inlining in the hot path. These two functions are certainly not in any hot path

[PATCH-for-9.0 4/4] meson: Enable -Wstatic-in-inline

2024-03-13 Thread Philippe Mathieu-Daudé
Compilers are clever enough to inline code when necessary. The only case we accept an inline function is static in header (we use C, not C++). Add the -Wstatic-in-inline CPPFLAG to prevent public and inline function to be added in the code base. Signed-off-by: Philippe Mathieu-Daudé ---

[PATCH-for-9.0 3/4] qtest/libqos: Un-inline size_to_prdtl()

2024-03-13 Thread Philippe Mathieu-Daudé
See previous commit and commit 9de9fa5cf2 ("Avoid using inlined functions with external linkage") for rationale. Signed-off-by: Philippe Mathieu-Daudé --- tests/qtest/libqos/ahci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qtest/libqos/ahci.c

[PATCH-for-9.0 0/4] overall: Avoid using inlined functions with external linkage again

2024-03-13 Thread Philippe Mathieu-Daudé
Mostly as a C style cleanup, use -Wstatic-in-inline to avoid using inlined function with external linkage. Philippe Mathieu-Daudé (4): hw/arm/smmu: Avoid using inlined functions with external linkage again accel/hvf: Un-inline hvf_arch_supports_guest_debug() qtest/libqos: Un-inline

Re: [PATCH v3] linux-aio: add IO_CMD_FDSYNC command support

2024-03-13 Thread Stefan Hajnoczi
On Wed, Mar 13, 2024 at 10:49:31PM +0530, Prasad Pandit wrote: > On Wed, 13 Mar 2024 at 20:48, Stefan Hajnoczi wrote: > > > +extern bool laio_has_fdsync(int); > > Please declare this in include/block/raw-aio.h alongside the other laio > > APIs. > > > > FDSYNC support should be probed at open()

Re: [PATCH v3] linux-aio: add IO_CMD_FDSYNC command support

2024-03-13 Thread Prasad Pandit
On Wed, 13 Mar 2024 at 20:48, Stefan Hajnoczi wrote: > > +extern bool laio_has_fdsync(int); > Please declare this in include/block/raw-aio.h alongside the other laio APIs. > > FDSYNC support should be probed at open() time and the result should be > stored in a new bool field like

Re: [PATCH v4 4/5] qapi: blockdev-backup: add discard-source parameter

2024-03-13 Thread Markus Armbruster
Vladimir Sementsov-Ogievskiy writes: > Add a parameter that enables discard-after-copy. That is mostly useful > in "push backup with fleecing" scheme, when source is snapshot-access > format driver node, based on copy-before-write filter snapshot-access > API: > > [guest] [snapshot-access]

[PATCH v4 0/5] backup: discard-source parameter

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
Hi all! The main patch is 04, please look at it for description and diagram. v4: add t-b by Fiona add r-b by Fiona to 02-05 (patch 01 still lack an r-b) 05: fix copyrights and subject in the test 04: since 9.0 --> since 9.1 (we missed a soft freeze for 9.0) Vladimir

[PATCH for-9.0] mirror: Don't call job_pause_point() under graph lock

2024-03-13 Thread Kevin Wolf
Calling job_pause_point() while holding the graph reader lock potentially results in a deadlock: bdrv_graph_wrlock() first drains everything, including the mirror job, which pauses it. The job is only unpaused at the end of the drain section, which is when the graph writer lock has been

[PATCH v4 4/5] qapi: blockdev-backup: add discard-source parameter

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
Add a parameter that enables discard-after-copy. That is mostly useful in "push backup with fleecing" scheme, when source is snapshot-access format driver node, based on copy-before-write filter snapshot-access API: [guest] [snapshot-access] ~~ blockdev-backup ~~> [backup target] |

[PATCH v4 3/5] block/copy-before-write: create block_copy bitmap in filter node

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
Currently block_copy creates copy_bitmap in source node. But that is in bad relation with .independent_close=true of copy-before-write filter: source node may be detached and removed before .bdrv_close() handler called, which should call block_copy_state_free(), which in turn should remove

[PATCH v4 5/5] iotests: add backup-discard-source

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
Add test for a new backup option: discard-source. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Fiona Ebner Tested-by: Fiona Ebner --- .../qemu-iotests/tests/backup-discard-source | 152 ++ .../tests/backup-discard-source.out | 5 + 2 files changed, 157

[PATCH v4 1/5] block/copy-before-write: fix permission

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
In case when source node does not have any parents, the condition still works as required: backup job do create the parent by block_job_create -> block_job_add_bdrv -> bdrv_root_attach_child Still, in this case checking @perm variable doesn't work, as backup job creates the root blk with empty

[PATCH v4 2/5] block/copy-before-write: support unligned snapshot-discard

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
First thing that crashes on unligned access here is bdrv_reset_dirty_bitmap(). Correct way is to align-down the snapshot-discard request. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Fiona Ebner Tested-by: Fiona Ebner --- block/copy-before-write.c | 16 +--- 1 file

Re: [PATCH v3] linux-aio: add IO_CMD_FDSYNC command support

2024-03-13 Thread Stefan Hajnoczi
On Wed, Mar 13, 2024 at 02:19:35PM +0530, Prasad Pandit wrote: > From: Prasad Pandit > > Libaio defines IO_CMD_FDSYNC command to sync all outstanding > asynchronous I/O operations, by flushing out file data to the > disk storage. > > Enable linux-aio to submit such aio request. This helps to >

[RFC 10/15] qapi: query-jobs: add information specific for job type

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
Duplicate the feature from query-block-jobs. It's a step to finally deprecate query-block-jobs command and move to query-jobs. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/mirror.c | 14 -- include/qemu/job.h | 5 + job-qmp.c | 6 ++ qapi/job.json

[RFC 05/15] qapi: JobChangeOptions: make type member optional and deprecated

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
Now QEMU can understand type directly from the job itself, type is redundant. Signed-off-by: Vladimir Sementsov-Ogievskiy --- blockjob.c | 2 +- docs/about/deprecated.rst| 6 ++ job-qmp.c| 17 +

[RFC 01/15] scripts/qapi: support type-based unions

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
Look at block-job-change command: we have to specify both 'id' to chose the job to operate on and 'type' for QAPI union be parsed. But for user this looks redundant: when we specify 'id', QEMU should be able to get corresponding job's type. This commit brings such a possibility: just specify some

[RFC 12/15] qapi: rename BlockDeviceIoStatus to IoStatus

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
This status is already shared between block-jobs and block-devices, so structure name is misleading a bit. We also will need to use the structure both in block-core.json and job.json. So give it more generic name first. The commit is made by commands: git grep -l BlockDeviceIoStatus | \

[RFC 11/15] job-qmp: job_query_single_locked: add assertion on job ret

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
job->ret must be always set together with job->err. Let's assert this. Reproting no-error to the user, when job->err is unset and job->ret is somehow set would be a bug. Signed-off-by: Vladimir Sementsov-Ogievskiy --- job-qmp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/job-qmp.c

[RFC 07/15] qapi: add job-change

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
Add a new-style command job-change, doing same thing as block-job-change. The aim is finally deprecate block-job-* APIs and move to job-* APIs. We add a new command to qapi/block-core.json, not to qapi/job.json to avoid resolving json file including loops for now. This all would be a lot simple

[RFC 14/15] qapi: query-job: add block-job specific information

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
Add io-status and speed, which make sense only for block-jobs. This allows us to finally deprecate old query-block-jobs API in the next commit. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/backup.c | 6 ++ block/commit.c | 6 ++ block/mirror.c |

[RFC 06/15] blockjob: move change action implementation to job from block-job

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
Like for other block-job-* APIs we want have the actual functionality in job layer and make block-job-change to be a deprecated duplication of job-change in the following commit. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/mirror.c | 7 +++ blockdev.c

[RFC 13/15] qapi: move IoStatus to common.json

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
We will need to use the structure both in block-core.json and job.json. So, move it to common.json, which could be then included to job.json. Signed-off-by: Vladimir Sementsov-Ogievskiy --- qapi/block-core.json | 16 qapi/common.json | 16 2 files changed,

[RFC 09/15] qapi: job-complete: introduce no-block-replace option for mirror

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
That's an alternative to block-job-cancel(hard=false) behavior for mirror, which exactly is: complete the job, wait for in-flight requests, but don't do any block graph modifications. Why: 1. We move to deprecating block-job-* APIs and use job-* APIs instead. So we need to port somehow the

[RFC 04/15] qapi: block-job-change: make copy-mode parameter optional

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
We are going to add more parameters to change. We want to make possible to change only one or any subset of available options. So all the options should be optional. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/mirror.c | 5 + qapi/block-core.json | 2 +- 2 files changed, 6

[RFC 02/15] qapi: rename BlockJobChangeOptions to JobChangeOptions

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
We are going to move change action from block-job to job implementation, and then move to job-* extenral APIs, deprecating block-job-* APIs. This commit simplifies further transition. The commit is made by command git grep -l BlockJobChangeOptions | \ xargs sed -i

[RFC 08/15] qapi: job-change: support speed parameter

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
Allow change job speed through job-change command. Old block-job-set-speed would be deprecated soon. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/backup.c | 8 ++ block/commit.c | 10 ++- block/mirror.c | 60

[RFC 15/15] qapi/block-core: derpecate block-job- APIs

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
For change, pause, resume, complete, dismiss and finalize actions corresponding job- and block-job commands are almost equal. The difference is in find_block_job_locked() vs find_job_locked() functions. What's different? 1. find_block_job_locked() do check, is found job a block-job. This OK

[RFC 03/15] blockjob: block_job_change_locked(): check job type

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
User may specify wrong type for the job id. Let's check it. Signed-off-by: Vladimir Sementsov-Ogievskiy --- blockjob.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/blockjob.c b/blockjob.c index 8cfbb15543..788cb1e07d 100644 --- a/blockjob.c +++ b/blockjob.c @@ -319,6 +319,12 @@

[RFC 00/15] block job API

2024-03-13 Thread Vladimir Sementsov-Ogievskiy
Hi all! The series aims to reach feature parity between job-* and block-job-* APIs and finally deprecate block-job-* APIs. 01: new type-based unions for QAPI 02-07: rework block-job-change and add similar job-change command 08: support set-speed in new API (as an option to job-change) 09:

Re: [PATCH v2 2/6] virtio: Prevent creation of device using notification-data with ioeventfd

2024-03-13 Thread Jonah Palmer
On 3/13/24 10:35 AM, Eugenio Perez Martin wrote: On Wed, Mar 13, 2024 at 12:55 PM Jonah Palmer wrote: Prevent the realization of a virtio device that attempts to use the VIRTIO_F_NOTIFICATION_DATA transport feature without disabling ioeventfd. Due to ioeventfd not being able to carry the

Re: [PATCH v2 2/6] virtio: Prevent creation of device using notification-data with ioeventfd

2024-03-13 Thread Eugenio Perez Martin
On Wed, Mar 13, 2024 at 12:55 PM Jonah Palmer wrote: > > Prevent the realization of a virtio device that attempts to use the > VIRTIO_F_NOTIFICATION_DATA transport feature without disabling > ioeventfd. > > Due to ioeventfd not being able to carry the extra data associated with > this feature,

Re: [PULL v2 0/6] hw/nvme updates

2024-03-13 Thread Peter Maydell
On Tue, 12 Mar 2024 at 17:26, Klaus Jensen wrote: > > From: Klaus Jensen > > Hi, > > Sorry about the compilation error in v1. Did a full CI run for v2. > > https://gitlab.com/birkelund/qemu/-/pipelines/1210559370 > > > > The following changes since commit

[PATCH v2 6/6] virtio: Add VIRTIO_F_NOTIFICATION_DATA property definition

2024-03-13 Thread Jonah Palmer
Extend the virtio device property definitions to include the VIRTIO_F_NOTIFICATION_DATA feature. The default state of this feature is disabled, allowing it to be explicitly enabled where it's supported. Tested-by: Lei Yang Reviewed-by: Eugenio Pérez Signed-off-by: Jonah Palmer ---

[PATCH v2 0/6] virtio,vhost: Add VIRTIO_F_NOTIFICATION_DATA support

2024-03-13 Thread Jonah Palmer
The goal of these patches are to add support to a variety of virtio and vhost devices for the VIRTIO_F_NOTIFICATION_DATA transport feature. This feature indicates that a driver will pass extra data (instead of just a virtqueue's index) when notifying the corresponding device. The data passed in

[PATCH v2 5/6] vhost/vhost-user: Add VIRTIO_F_NOTIFICATION_DATA to vhost feature bits

2024-03-13 Thread Jonah Palmer
Add support for the VIRTIO_F_NOTIFICATION_DATA feature across a variety of vhost devices. The inclusion of VIRTIO_F_NOTIFICATION_DATA in the feature bits arrays for these devices ensures that the backend is capable of offering and providing support for this feature, and that it can be disabled if

[PATCH v2 3/6] virtio-mmio: Handle extra notification data

2024-03-13 Thread Jonah Palmer
Add support to virtio-mmio devices for handling the extra data sent from the driver to the device when the VIRTIO_F_NOTIFICATION_DATA transport feature has been negotiated. The extra data that's passed to the virtio-mmio device when this feature is enabled varies depending on the device's

[PATCH v2 2/6] virtio: Prevent creation of device using notification-data with ioeventfd

2024-03-13 Thread Jonah Palmer
Prevent the realization of a virtio device that attempts to use the VIRTIO_F_NOTIFICATION_DATA transport feature without disabling ioeventfd. Due to ioeventfd not being able to carry the extra data associated with this feature, having both enabled is a functional mismatch and therefore Qemu

[PATCH v2 1/6] virtio/virtio-pci: Handle extra notification data

2024-03-13 Thread Jonah Palmer
Add support to virtio-pci devices for handling the extra data sent from the driver to the device when the VIRTIO_F_NOTIFICATION_DATA transport feature has been negotiated. The extra data that's passed to the virtio-pci device when this feature is enabled varies depending on the device's virtqueue

[PATCH v2 4/6] virtio-ccw: Handle extra notification data

2024-03-13 Thread Jonah Palmer
Add support to virtio-ccw devices for handling the extra data sent from the driver to the device when the VIRTIO_F_NOTIFICATION_DATA transport feature has been negotiated. The extra data that's passed to the virtio-ccw device when this feature is enabled varies depending on the device's virtqueue

[PATCH v2] block: Use LVM tools for LV block device truncation

2024-03-13 Thread Alexander Ivanov
If a block device is an LVM logical volume we can resize it using standard LVM tools. Add a helper to detect if a device is a DM device. In raw_co_truncate() check if the block device is DM and resize it executing lvresize. Signed-off-by: Alexander Ivanov --- block/file-posix.c | 61

Re: [PATCH for 9.0] migration: Skip only empty block devices

2024-03-13 Thread Kevin Wolf
Am 12.03.2024 um 13:27 hat Peter Xu geschrieben: > On Tue, Mar 12, 2024 at 01:04:31PM +0100, Cédric Le Goater wrote: > > The block .save_setup() handler calls a helper routine > > init_blk_migration() which builds a list of block devices to take into > > account for migration. When one device is

[PATCH v3] linux-aio: add IO_CMD_FDSYNC command support

2024-03-13 Thread Prasad Pandit
From: Prasad Pandit Libaio defines IO_CMD_FDSYNC command to sync all outstanding asynchronous I/O operations, by flushing out file data to the disk storage. Enable linux-aio to submit such aio request. This helps to reduce latency induced via pthread_create calls by thread-pool (aio=threads).

Re: [PATCH for 9.0] migration: Skip only empty block devices

2024-03-13 Thread Markus Armbruster
Peter Xu writes: > On Tue, Mar 12, 2024 at 05:34:26PM -0400, Stefan Hajnoczi wrote: >> I understand now. I missed that returning from init_blk_migration_it() >> did not abort iteration. Thank you! > > I queued it, thanks both! Thanks for cleaning up the mess I made!