Re: [PATCH v3 20/22] block: fix -Werror=maybe-uninitialized false-positive

2024-10-02 Thread Kevin Wolf
ff-by: Marc-André Lureau Reviewed-by: Kevin Wolf

Re: [PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations

2024-09-27 Thread Kevin Wolf
d: bindings::Error should then implement Drop for the inner value (with an error_free_inner() which needs to be exported separately from C first), and then ForeignBox can just drop the Error object and g_free() the pointer itself like it would do with any other value. (Your implementation actually calls free() instead of g_free(). We generally try to avoid that in our C code, so we should probably avoid it in Rust, too.) Kevin

Re: -chardev with a JSON argument (was: [PATCH] chardev: introduce 'reconnect-ms' and deprecate 'reconnect')

2024-09-16 Thread Kevin Wolf
ould not only add JSON support, but also make sure that everything that works in JSON also works with keyval syntax, both ways stay in sync in the future and that you can access non-scalar options without JSON: https://repo.or.cz/qemu/kevin.git/shortlog/refs/tags/qapi-alias-chardev-v4 Without QAPI aliases, they will have to contain some ugly code to do the compatibility conversion, but whatever can be generated can also be written manually... Kevin

Re: [PATCH 09/39] qobject: replace assert(0) with g_assert_not_reached()

2024-09-11 Thread Kevin Wolf
Am 11.09.2024 um 00:15 hat Pierrick Bouvier geschrieben: > Signed-off-by: Pierrick Bouvier Reviewed-by: Kevin Wolf

Re: [PATCH 31/39] hw/scsi: remove break after g_assert_not_reached()

2024-09-11 Thread Kevin Wolf
Am 11.09.2024 um 00:15 hat Pierrick Bouvier geschrieben: > Signed-off-by: Pierrick Bouvier Reviewed-by: Kevin Wolf

Re: [PATCH 25/39] block: remove break after g_assert_not_reached()

2024-09-11 Thread Kevin Wolf
Am 11.09.2024 um 00:15 hat Pierrick Bouvier geschrieben: > Signed-off-by: Pierrick Bouvier Reviewed-by: Kevin Wolf

Re: [PATCH 15/39] block: replace assert(false) with g_assert_not_reached()

2024-09-11 Thread Kevin Wolf
Am 11.09.2024 um 00:15 hat Pierrick Bouvier geschrieben: > Signed-off-by: Pierrick Bouvier Reviewed-by: Kevin Wolf

Re: [PATCH] block: support locking on change medium

2024-09-09 Thread Kevin Wolf
Am 09.09.2024 um 16:25 hat Joelle van Dyne geschrieben: > On Mon, Sep 9, 2024 at 2:56 AM Kevin Wolf wrote: > > > > Am 09.09.2024 um 03:58 hat Joelle van Dyne geschrieben: > > > New optional argument for 'blockdev-change-medium' QAPI command to allow > >

Re: [PATCH] block: support locking on change medium

2024-09-09 Thread Kevin Wolf
"file" child is referring to a file-posix driver rather than using a different protocol or being a filter driver above yet another node. It also doesn't consider backing files and other non-primary children of the opened node. So this is not correct, and I don't think there is any realistic way of making it correct with this approach. Kevin

[PATCH] raw-format: Fix error message for invalid offset/size

2024-08-29 Thread Kevin Wolf
s->offset and s->size are only set at the end of the function and still contain the old values when formatting the error message. Print the parameters with the new values that we actually checked instead. Fixes: 500e2434207d ('raw-format: Split raw_read_options()') Signed-of

Re: [PATCH v5 0/5] vvfat: Fix write bugs for large files and add iotests

2024-08-13 Thread Kevin Wolf
s: Add `vvfat` tests > > Actually, maybe the whole series is a good candidate for -stable, not > just the first fix. What do you think? Yes, if you consider vvfat relevant for stable at all, then I think you want to take all of the fixes, each one fixes some corruption in read-write mode. (Though as far as I can tell, read-write support is still broken even after this series.) Kevin

[PULL v3 00/13] Block layer patches

2024-08-06 Thread Kevin Wolf
vvfat` tests Andrey Drobyshev (1): iotests/024: exclude 'backing file format' field from the output Kevin Wolf (6): block-copy: Fix missing graph lock block/graph-lock: Make WITH_GRAPH_RDLOCK_GUARD() fully checked scsi-disk: Use positive return value for status

[PULL v2 00/13] Block layer patches

2024-08-06 Thread Kevin Wolf
Andrey Drobyshev (1): iotests/024: exclude 'backing file format' field from the output Kevin Wolf (6): block-copy: Fix missing graph lock block/graph-lock: Make WITH_GRAPH_RDLOCK_GUARD() fully checked scsi-disk: Use positive return value for status in dma_readv/write

[PULL 11/13] vvfat: Fix reading files with non-continuous clusters

2024-08-05 Thread Kevin Wolf
f-by: Amjad Alsharafi Message-ID: <1f3ea115779abab62ba32c788073cdc99f9ad5dd.1721470238.git.amjadsharaf...@gmail.com> [kwolf: Simplified the patch based on Amjad's analysis and input] Signed-off-by: Kevin Wolf --- block/vvfat.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff

[PULL 07/13] scsi-disk: Always report RESERVATION_CONFLICT to guest

2024-08-05 Thread Kevin Wolf
n the guest is running the validation tests in Windows 2019's Failover Cluster Manager, which intentionally tries to execute invalid requests to see if they are properly rejected. Buglink: https://issues.redhat.com/browse/RHEL-5 Signed-off-by: Kevin Wolf Acked-by: Paolo Bonzini

[PULL 13/13] iotests/024: exclude 'backing file format' field from the output

2024-08-05 Thread Kevin Wolf
out and exclude it from the output. This is a fixup for the commit f93e65ee51 ("iotests/{024, 271}: add testcases for qemu-img rebase"). Reported-by: Thomas Huth Signed-off-by: Andrey Drobyshev Message-ID: <20240730094701.790624-1-andrey.drobys...@virtuozzo.com> Reviewed-by: Er

[PULL 10/13] vvfat: Fix wrong checks for cluster mappings invariant

2024-08-05 Thread Kevin Wolf
ed-off-by: Amjad Alsharafi Reviewed-by: Kevin Wolf Message-ID: Signed-off-by: Kevin Wolf --- block/vvfat.c | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index cfde468c2e..e3a83fbc88 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @

[PULL 03/13] block/graph-lock: Make WITH_GRAPH_RDLOCK_GUARD() fully checked

2024-08-05 Thread Kevin Wolf
Signed-off-by: Kevin Wolf Message-ID: <20240627181245.281403-3-kw...@redhat.com> Reviewed-by: Stefan Hajnoczi Reviewed-by: Manos Pitsidianakis Signed-off-by: Kevin Wolf --- include/block/graph-lock.h | 21 ++--- meson.build| 14 +- 2 files changed

[PULL 06/13] scsi-disk: Add warning comments that host_status errors take a shortcut

2024-08-05 Thread Kevin Wolf
ly on the callback doing some cleanup until this code smell is fixed. For now, just add warnings to make people aware of the trap. Signed-off-by: Kevin Wolf Acked-by: Paolo Bonzini Message-ID: <20240731123207.27636-4-kw...@redhat.com> Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 7

[PULL 08/13] vvfat: Fix bug in writing to middle of file

2024-08-05 Thread Kevin Wolf
y-one error here, where `i=0x2000 !< offset=0x2000`, thus not fetching the next cluster. Signed-off-by: Amjad Alsharafi Reviewed-by: Kevin Wolf Tested-by: Kevin Wolf Message-ID: Signed-off-by: Kevin Wolf --- block/vvfat.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git

[PULL 12/13] iotests: Add `vvfat` tests

2024-08-05 Thread Kevin Wolf
-off-by: Amjad Alsharafi Reviewed-by: Kevin Wolf Tested-by: Kevin Wolf Message-ID: [kwolf: Made mypy and pylint happy to unbreak 297] Signed-off-by: Kevin Wolf --- tests/qemu-iotests/fat16.py| 690 + tests/qemu-iotests/testenv.py | 2 +- tests/qemu

[PULL 09/13] vvfat: Fix usage of `info.file.offset`

2024-08-05 Thread Kevin Wolf
From: Amjad Alsharafi The field is marked as "the offset in the file (in clusters)", but it was being used like this `cluster_size*(nums)+mapping->info.file.offset`, which is incorrect. Signed-off-by: Amjad Alsharafi Reviewed-by: Kevin Wol

[PULL 05/13] scsi-block: Don't skip callback for sgio error status/driver_status

2024-08-05 Thread Kevin Wolf
: inline sg_io_sense_from_errno() into the callers.") Buglink: https://issues.redhat.com/browse/RHEL-5 Signed-off-by: Kevin Wolf Acked-by: Paolo Bonzini Message-ID: <20240731123207.27636-3-kw...@redhat.com> Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 10 -- 1 file changed, 10 deletions(

[PULL 02/13] block-copy: Fix missing graph lock

2024-08-05 Thread Kevin Wolf
which allows calling functions that require the lock), but we never deal with the unlocking (so even after the scope of the guard, the compiler assumes that the lock is still held). This is why the compiler didn't catch this locking error. Signed-off-by: Kevin Wolf Message-ID: <2024062718

[PULL 00/13] Block layer patches

2024-08-05 Thread Kevin Wolf
vvfat` tests Andrey Drobyshev (1): iotests/024: exclude 'backing file format' field from the output Kevin Wolf (6): block-copy: Fix missing graph lock block/graph-lock: Make WITH_GRAPH_RDLOCK_GUARD() fully checked scsi-disk: Use positive return value for status

[PULL 04/13] scsi-disk: Use positive return value for status in dma_readv/writev

2024-08-05 Thread Kevin Wolf
callbacks that represent the status code that should be used to complete the request. scsi_handle_rw_error() already handles positive values for its ret parameter because scsi_block_sgio_complete() calls directly into it. Signed-off-by: Kevin Wolf Acked-by: Paolo Bonzini Message-ID

[PULL 01/13] qapi-block-core: Clean up blockdev-snapshot-internal-sync doc

2024-08-05 Thread Kevin Wolf
e it to the command's doc comment, where it is more prominently visible, with a slight rephrasing for clarity. Signed-off-by: Markus Armbruster Message-ID: <20240718123609.3063055-1-arm...@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- qapi/block-core.json | 7 +++

Re: [PATCH v3 2/3] iotests/298: add testcase for async writes with preallocation filter

2024-08-05 Thread Kevin Wolf
Am 05.08.2024 um 14:56 hat Andrey Drobyshev geschrieben: > On 8/5/24 3:04 PM, Kevin Wolf wrote: > > Am 16.07.2024 um 16:41 hat Andrey Drobyshev geschrieben: > >> The testcase simply creates a 64G image with 1M clusters, generates a list > >> of 1M aligned offsets and fe

Re: [PATCH v3 3/3] scripts: add filev2p.py script for mapping virtual file offsets mapping

2024-08-05 Thread Kevin Wolf
> +partition = Partition(args.partpath, args.diskpath, args.part_offt) > + > +# Fill extents list from the output > +extents = [] > +for line in lines: > +if not re.match(r'^\s*[0-9]+:', line): > +continue > +extents += [parse_frag_line(line, partition)] > + > +chunk_start = args.offset > +chunk_end = args.offset + args.size - 1 > +ext_offsets = [ext.log_start for ext in extents] > +start_ind = bisect_right(ext_offsets, chunk_start) - 1 > +end_ind = bisect_right(ext_offsets, chunk_end) - 1 > + > +res_extents = extents[start_ind : end_ind + 1] > +for i, ext in enumerate(res_extents): > +start = max(chunk_start, ext.log_start) > +end = min(chunk_end, ext.log_end) > +res_extents[i] = Extent.ext_slice(ext, start, end) > + > +return res_extents > + > + > +def parse_args() -> argparse.Namespace: > +'Define program arguments and parse user input.' > + > +parser = argparse.ArgumentParser(description=''' > +Map file offset to physical offset on the block device > + > +With --size provided get a list of mappings for the chunk''', > +formatter_class=argparse.RawTextHelpFormatter) > + > +parser.add_argument('filename', type=str, help='filename to process') > +parser.add_argument('offset', type=str, > +help='logical offset inside the file') > +parser.add_argument('-s', '--size', required=False, type=str, > +help='size of the file chunk to get offsets for') > +args = parser.parse_args() > + > +args.offset = parse_size(args.offset) > +if args.size: > +args.size = parse_size(args.size) > +else: > +# When no chunk size is provided (only offset), it's equivalent to > +# chunk size == 1 > +args.size = 1 > + > +return args > + > + > +def main() -> int: > +args = parse_args() > +preliminary_checks(args) > +extents = get_extent_maps(args) > +for ext in extents: > +print(ext) > + > + > +if __name__ == '__main__': > +sys.exit(main()) Kevin

Re: [PATCH v3 2/3] iotests/298: add testcase for async writes with preallocation filter

2024-08-05 Thread Kevin Wolf
7; > +n = 1 - n This looks like a complicated way to write the following? # Alternate between write_zeroes and writing data def gen_write_pattern(): while True: yield '-z' yield '-P 0xaa' > +def gen_read_pattern(): > +n = 0 > +while True: > +yield '-P 0xaa' if n else '-P 0x00' > +n = 1 - n Same here. Kevin

Re: [PATCH v3 1/3] block: zero data data corruption using prealloc-filter

2024-08-05 Thread Kevin Wolf
Am 18.07.2024 um 21:46 hat Denis V. Lunev geschrieben: > On 7/18/24 17:51, Kevin Wolf wrote: > > Am 16.07.2024 um 16:41 hat Andrey Drobyshev geschrieben: > > > From: "Denis V. Lunev" > > > > > > We have observed that some clusters in the QCOW2 files

Re: [PATCH] iotests/024: exclude 'backing file format' field from the output

2024-08-05 Thread Kevin Wolf
> > This leads to the 024 test failure with -qed. Let's just filter the > field out and exclude it from the output. > > This is a fixup for the commit f93e65ee51 ("iotests/{024, 271}: add > testcases for qemu-img rebase"). > > Found-by: Thomas Huth > Signed-off-by: Andrey Drobyshev Thanks, applied to the block branch. Kevin

Re: [PATCH v6 0/5] vvfat: Fix write bugs for large files and add iotests

2024-08-05 Thread Kevin Wolf
s reading, writing, and creating files on the > filesystem. > Including tests for reading/writing the first cluster which > would pass even before this patch. Thanks, applied to the block branch (with patch 4 changed as indicated in my comment there). Kevin

Re: [PATCH v6 4/5] vvfat: Fix reading files with non-continuous clusters

2024-08-05 Thread Kevin Wolf
s->current_fd = fd; -s->current_mapping = mapping; } + +s->current_mapping = mapping; return 0; } That should be all that is needed, and it passes your test case. I don't usually do this, but since time is running out for QEMU 9.1, I'll just replace the code of this patch with the above and go ahead with that. If you think it's wrong, let me know and we'll fix it on top of this series. Kevin

Re: [PATCH 7/7] block/ssh.c: Don't double-check that characters are hex digits

2024-07-31 Thread Kevin Wolf
_check[0]) * 16 + > -hex2decimal(host_key_check[1]); > +} > +c = c0 * 16 + c1; > if (c - *fingerprint != 0) > return c - *fingerprint; > fingerprint++; Reviewed-by: Kevin Wolf

Re: [PATCH 6/7] hw/ide/pci.c: Remove dead code from bmdma_prepare_buf()

2024-07-31 Thread Kevin Wolf
s->io_buffer_size += l; > } > } > - > -qemu_sglist_destroy(&s->sg); > -s->io_buffer_size = 0; > -return -1; > } Should we put a g_assert_not_reached() here instead to make it easier for the reader to understand how this function works? Either way: Reviewed-by: Kevin Wolf

Re: [PATCH 3/7] hw/block/pflash_cfi01: Don't decrement pfl->counter below 0

2024-07-31 Thread Kevin Wolf
> 0" if(), to avoid the decrement-below-zero. > > Resolves: Coverity CID 1547611 > Signed-off-by: Peter Maydell Reviewed-by: Kevin Wolf

Re: [PATCH 2/7] block/gluster: Use g_autofree for string in qemu_gluster_parse_json()

2024-07-31 Thread Kevin Wolf
ckdevOptionsGluster > *gconf, > tail = &gconf->server; > > for (i = 0; i < num_servers; i++) { > -str = g_strdup_printf(GLUSTER_OPT_SERVER_PATTERN"%d.", i); > +g_autofree char *str = > g_strdup_printf(GLUSTER_OPT_SERVER_PATTERN"%d.", i); This line is too long now. With this fixed: Reviewed-by: Kevin Wolf

Re: [PATCH 1/7] block/vdi.c: Avoid potential overflow when calculating size of write

2024-07-31 Thread Kevin Wolf
_SIZE, base, 0); > + n_sectors * (uint64_t)SECTOR_SIZE, base, 0); > } I wonder if we shouldn't just make VDI's SECTOR_SIZE 64 bits like BDRV_SECTOR_SIZE. It's easy to miss the cast in individual places. Kevin

Re: [PATCH 4/7] hw/ide/atapi: Be explicit that assigning to s->lcyl truncates

2024-07-31 Thread Kevin Wolf
rity warns that the first line here can overflow the > 8-bit s->lcyl variable. This is true, and in this case we're > deliberately only after the low 8 bits of the value. The > code is clearer to both humans and Coverity if we're explicit > that we only wanted the low 8 bits

Re: [PATCH 5/7] hw/block/fdc-isa: Assert that isa_fdc_get_drive_max_chs() found something

2024-07-31 Thread Kevin Wolf
ible bugs in the data in fd_formats. > > Resolves: Coverity CID 1547663 > Signed-off-by: Peter Maydell Reviewed-by: Kevin Wolf

[PATCH v2 1/4] scsi-disk: Use positive return value for status in dma_readv/writev

2024-07-31 Thread Kevin Wolf
callbacks that represent the status code that should be used to complete the request. scsi_handle_rw_error() already handles positive values for its ret parameter because scsi_block_sgio_complete() calls directly into it. Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 21

[PATCH v2 2/4] scsi-block: Don't skip callback for sgio error status/driver_status

2024-07-31 Thread Kevin Wolf
: inline sg_io_sense_from_errno() into the callers.") Buglink: https://issues.redhat.com/browse/RHEL-5 Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 3ff6798bde..6e1a5c98df 100644 --- a/hw/scs

[PATCH v2 4/4] scsi-disk: Always report RESERVATION_CONFLICT to guest

2024-07-31 Thread Kevin Wolf
n the guest is running the validation tests in Windows 2019's Failover Cluster Manager, which intentionally tries to execute invalid requests to see if they are properly rejected. Buglink: https://issues.redhat.com/browse/RHEL-5 Signed-off-by: Kevin Wolf --- hw/scsi/scsi-d

[PATCH v2 0/4] scsi-block: Fix error handling with r/werror=stop

2024-07-31 Thread Kevin Wolf
Patch 4: [Paolo] * Set error = 0 explicitly, remove useless variable initialisation * Add comment to explain why we consider it a guest error * Mention scsi-block specifically in the commit message Kevin Wolf (4): scsi-disk: Use positive return value for status in dma_readv/writev scsi-bl

[PATCH v2 3/4] scsi-disk: Add warning comments that host_status errors take a shortcut

2024-07-31 Thread Kevin Wolf
ly on the callback doing some cleanup until this code smell is fixed. For now, just add warnings to make people aware of the trap. Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 6e1a5

Re: [PATCH 01/18] qapi: Smarter camel_to_upper() to reduce need for 'prefix'

2024-07-31 Thread Kevin Wolf
e is one > +# already, or it's at the beginning > +if len(ret) > 2 and ret[-2] != '_': > +ret = ret[:-1] + '_' + ret[-1] I think in the code this means I would have expected len(ret) >= 2. Kevin

Re: [PATCH 4/4] scsi-disk: Always report RESERVATION_CONFLICT to guest

2024-07-29 Thread Kevin Wolf
Am 29.07.2024 um 14:26 hat Paolo Bonzini geschrieben: > Il lun 29 lug 2024, 14:20 Kevin Wolf ha scritto: > > > Apparently both oVirt and Kubevirt unconditionally use the stop policy, > > so I'm afraid in this case we must acknowledge that our expectations > > don&#

Re: [PATCH v2 1/3] block/commit: implement final flush

2024-07-29 Thread Kevin Wolf
Am 19.07.2024 um 12:35 hat Vladimir Sementsov-Ogievskiy geschrieben: > On 18.07.24 22:22, Kevin Wolf wrote: > > Am 26.06.2024 um 16:50 hat Vladimir Sementsov-Ogievskiy geschrieben: > > > Actually block job is not completed without the final flush. It's > > > rather

Re: [PATCH 4/4] scsi-disk: Always report RESERVATION_CONFLICT to guest

2024-07-29 Thread Kevin Wolf
Am 29.07.2024 um 13:55 hat Paolo Bonzini geschrieben: > On Mon, Jul 29, 2024 at 11:47 AM Kevin Wolf wrote: > > RESERVATION_CONFLICT is not a backend error, but indicates that the > > guest tried to make a request that it isn't allowed to execute. Pass the > > error t

[PATCH 1/4] scsi-disk: Use positive return value for status in dma_readv/writev

2024-07-29 Thread Kevin Wolf
callbacks that represent the status code that should be used to complete the request. scsi_handle_rw_error() already handles positive values for its ret parameter because scsi_block_sgio_complete() calls directly into it. Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 21

[PATCH 4/4] scsi-disk: Always report RESERVATION_CONFLICT to guest

2024-07-29 Thread Kevin Wolf
entionally tries to execute invalid requests to see if they are properly rejected. Buglink: https://issues.redhat.com/browse/RHEL-5 Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw

[PATCH 0/4] scsi-block: Fix error handling with r/werror=stop

2024-07-29 Thread Kevin Wolf
CT is treated as a host error and stops the VM, which in some cases can't be resumed at all because nothing will make the error go away on retry. The error should always go to the guest instead, it's an invalid request from the guest. This series fixes these problems. Kevin Wolf (

[PATCH 3/4] scsi-disk: Add warning comments that host_status errors take a shortcut

2024-07-29 Thread Kevin Wolf
ly on the callback doing some cleanup until this code smell is fixed. For now, just add warnings to make people aware of the trap. Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 6e1a5

[PATCH 2/4] scsi-block: Don't skip callback for sgio error status/driver_status

2024-07-29 Thread Kevin Wolf
: inline sg_io_sense_from_errno() into the callers.") Buglink: https://issues.redhat.com/browse/RHEL-5 Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 3ff6798bde..6e1a5c98df 100644 --- a/hw/scs

RE: [RFC PATCH 0/6] Enable shared device assignment

2024-07-25 Thread Tian, Kevin
> From: David Hildenbrand > Sent: Thursday, July 25, 2024 10:04 PM > > > Open > > > > Implementing a RamDiscardManager to notify VFIO of page conversions > > causes changes in semantics: private memory is treated as discarded (or > > hot-removed) memory. This isn't aligned with the expectati

Re: [PATCH v5 4/5] vvfat: Fix reading files with non-continuous clusters

2024-07-19 Thread Kevin Wolf
Am 19.07.2024 um 02:29 hat Amjad Alsharafi geschrieben: > > > On Jul 19 2024, at 8:20 am, Amjad Alsharafi wrote: > > > On Thu, Jul 18, 2024 at 05:20:36PM +0200, Kevin Wolf wrote: > >> Am 12.06.2024 um 14:43 hat Amjad Alsharafi geschrieben: > >> > When

Re: [PATCH v2] block-backend: per-device throttling of BLOCK_IO_ERROR reports

2024-07-19 Thread Kevin Wolf
Am 19.07.2024 um 06:54 hat Markus Armbruster geschrieben: > Kevin Wolf writes: > > > Am 09.01.2024 um 14:13 hat Vladimir Sementsov-Ogievskiy geschrieben: > >> From: Leonid Kaplan > >> > >> BLOCK_IO_ERROR events comes from guest, so we must throt

Re: [PATCH v2 1/3] block/commit: implement final flush

2024-07-18 Thread Kevin Wolf
assert(need_final_flush); > +ret = blk_co_flush(s->base); > +if (ret < 0) { > +error_in_source = false; > +} else { > +need_final_flush = false; > +} Should we set n = 0 in this block to avoid counting the last chunk twice for the progress? > } > + > if (ret < 0) { > BlockErrorAction action = > block_job_error_action(&s->common, s->on_error, Kevin

Re: [PATCH v2] block-backend: per-device throttling of BLOCK_IO_ERROR reports

2024-07-18 Thread Kevin Wolf
> +} > + > return TRUE; > } > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index ca390c5700..32c2c2f030 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -5559,6 +5559,8 @@ > # Note: If action is "stop", a STOP event will eventually follow the > # BLOCK_IO_ERROR event > # > +# Note: This event is rate-limited. > +# > # Since: 0.13 > # > # Example: Kevin

Re: [PATCH] block/curl: rewrite http header parsing function

2024-07-18 Thread Kevin Wolf
h strings */ > +s->accept_range = true; > +} Maybe make the generic comparison with a template a separate function (maybe even in cutils.c?) so that curl_header_cb() essentially only has something like this any more: if (!qemu_memcasecmp_space(ptr, end, "accept-ranges: bytes ")) { s->accept_range = true; } (A better name for the function would be preferable, of course. Maybe also a bool return value, but if it has a name related to memcmp() or strcmp(), then 0 must mean it matches.) Then this would really highlight the curl specific logic rather than the string parser in curl_header_cb(). Kevin

Re: [PATCH] qapi-block-core: Clean up blockdev-snapshot-internal-sync doc

2024-07-18 Thread Kevin Wolf
he command's Errors documentation. Drop. > > The remainder is fine. Move it to the command's doc comment, where it > is more prominently visible, with a slight rephrasing for clarity. > > Signed-off-by: Markus Armbruster Thanks, applied to the block branch. Kevin

Re: [PATCH v3 1/3] block: zero data data corruption using prealloc-filter

2024-07-18 Thread Kevin Wolf
le size is unchanged. In other words, if anything calls preallocate_co_getlength() in the meantime, don't we run into... ret = bdrv_co_getlength(bs->file->bs); if (has_prealloc_perms(bs)) { s->file_end = s->zero_start = s->data_end = ret; } ...and reset s->file_end back to the old value, re-enabling the bug you're trying to fix here? Or do we know that no such code path can be called concurrently for some reason? Kevin

Re: [PATCH v5 3/5] vvfat: Fix wrong checks for cluster mappings invariant

2024-07-18 Thread Kevin Wolf
first_mapping_index != -1` since we know that this is the > value for the only entry where `offset == 0` (i.e. first mapping). > > Signed-off-by: Amjad Alsharafi Reviewed-by: Kevin Wolf

Re: [PATCH v5 2/5] vvfat: Fix usage of `info.file.offset`

2024-07-18 Thread Kevin Wolf
Am 12.06.2024 um 14:43 hat Amjad Alsharafi geschrieben: > The field is marked as "the offset in the file (in clusters)", but it > was being used like this > `cluster_size*(nums)+mapping->info.file.offset`, which is incorrect. > > Signed-off-by: Amjad Alsharafi Reviewed-by: Kevin Wolf

Re: [PATCH v5 5/5] iotests: Add `vvfat` tests

2024-07-18 Thread Kevin Wolf
+self.update_direntry(entry) > + > +# trigger every affected cluster > +for cluster in affected_clusters: > +first_sector = self.boot_sector.first_sector_of_cluster(cluster) > +first_sector_data = self.read_sectors(first_sector, 1) > +self.write_sectors(first_sector, first_sector_data) Other than this, the patch looks good to me and we seem to test all the cases that are fixed by the previous patches. Reviewed-by: Kevin Wolf Tested-by: Kevin Wolf Kevin

Re: [PATCH v5 4/5] vvfat: Fix reading files with non-continuous clusters

2024-07-18 Thread Kevin Wolf
s->current_fd = fd; > + if (fd < 0) { > +return -1; > +} > +vvfat_close_current_file(s); > + > +s->current_fd = fd; > +} > +assert(s->current_fd); > s->current_mapping = mapping; > } Kevin

Re: [PULL 4/4] block: Parse filenames only when explicitly requested

2024-07-04 Thread Kevin Wolf
Am 03.07.2024 um 23:16 hat Michael Tokarev geschrieben: > 02.07.2024 19:39, Kevin Wolf wrote: > > When handling image filenames from legacy options such as -drive or from > > tools, these filenames are parsed for protocol prefixes, including for > > the json:{} pseudo-pr

[PULL 3/4] iotests/270: Don't store data-file with json: prefix in image

2024-07-02 Thread Kevin Wolf
We want to disable filename parsing for data files because it's too easy to abuse in malicious image files. Make the test ready for the change by passing the data file explicitly in command line options. Cc: qemu-sta...@nongnu.org Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Review

[PULL 0/4] Block layer patches (CVE-2024-4467)

2024-07-02 Thread Kevin Wolf
open qcow2 data files in 'qemu-img info' - Disallow protocol prefixes for qcow2 data files, VMDK extent files and other child nodes that are neither 'file' nor 'backing' ---- Kevin Wolf (4): qcow2: Don

[PULL 4/4] block: Parse filenames only when explicitly requested

2024-07-02 Thread Kevin Wolf
file child. All other callers pass false and disable filename parsing this way. Cc: qemu-sta...@nongnu.org Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Stefan Hajnoczi Reviewed-by: Hanna Czenczek --- block.c | 90 -

[PULL 2/4] iotests/244: Don't store data-file with protocol in image

2024-07-02 Thread Kevin Wolf
We want to disable filename parsing for data files because it's too easy to abuse in malicious image files. Make the test ready for the change by passing the data file explicitly in command line options. Cc: qemu-sta...@nongnu.org Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Review

[PULL 1/4] qcow2: Don't open data_file with BDRV_O_NO_IO

2024-07-02 Thread Kevin Wolf
ucceeds. Replace this part of the test with a qemu-io call, but keep the final 'qemu-img info' to show that the invalid data file is correctly displayed in the output. Fixes: CVE-2024-4467 Cc: qemu-sta...@nongnu.org Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Stef

Re: [PATCH] scsi: Don't ignore most usb-storage properties

2024-07-02 Thread Kevin Wolf
Am 01.07.2024 um 15:08 hat Fiona Ebner geschrieben: > Hi, > > we got a user report about bootindex for an 'usb-storage' device not > working anymore [0] and I reproduced it and bisected it to this patch. > > Am 31.01.24 um 14:06 schrieb Kevin Wolf: > >

[PATCH 2/2] block/graph-lock: Make WITH_GRAPH_RDLOCK_GUARD() fully checked

2024-06-27 Thread Kevin Wolf
Signed-off-by: Kevin Wolf --- include/block/graph-lock.h | 21 ++--- meson.build| 14 +- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/include/block/graph-lock.h b/include/block/graph-lock.h index d7545e82d0..dc8d949184 100644 --- a/incl

[PATCH 1/2] block-copy: Fix missing graph lock

2024-06-27 Thread Kevin Wolf
which allows calling functions that require the lock), but we never deal with the unlocking (so even after the scope of the guard, the compiler assumes that the lock is still held). This is why the compiler didn't catch this locking error. Signed-off-by: Kevin Wolf --- block/block-copy.c |

[PATCH 0/2] block/graph-lock: Make WITH_GRAPH_RDLOCK_GUARD() fully checked

2024-06-27 Thread Kevin Wolf
Newer clang versions allow us to check scoped guards more thoroughly. Surprisingly, we only seem to have missed one instance with the old incomplete checks. Kevin Wolf (2): block-copy: Fix missing graph lock block/graph-lock: Make WITH_GRAPH_RDLOCK_GUARD() fully checked include/block/graph

Re: [PATCH] block/curl: explicitly assert that strchr returns non-NULL value

2024-06-27 Thread Kevin Wolf
original line is clear enough. My first thought was that maybe what we want is a comment, but we actually already know where the colon is. So how about this instead: char *p = header + strlen(accept_ranges); Kevin > /* Skip whitespace between the header name and value. */ > while (p < end && *p && g_ascii_isspace(*p)) { > -- > 2.34.1 >

Re: [PATCH v2] Consider discard option when writing zeros

2024-06-27 Thread Kevin Wolf
Am 26.06.2024 um 18:27 hat Nir Soffer geschrieben: > On Wed, Jun 26, 2024 at 12:17 PM Daniel P. Berrangé > wrote: > > > On Mon, Jun 24, 2024 at 06:08:26PM +0200, Kevin Wolf wrote: > > > Am 24.06.2024 um 17:23 hat Stefan Hajnoczi geschrieben: > > > > On Wed, J

Re: [PATCH v2] Consider discard option when writing zeros

2024-06-26 Thread Kevin Wolf
Am 24.06.2024 um 23:12 hat Nir Soffer geschrieben: > On Mon, Jun 24, 2024 at 7:08 PM Kevin Wolf wrote: > > > Am 24.06.2024 um 17:23 hat Stefan Hajnoczi geschrieben: > > > On Wed, Jun 19, 2024 at 08:43:25PM +0300, Nir Soffer wrote: > > > > Tested using: > >

Re: [PATCH v2] Consider discard option when writing zeros

2024-06-24 Thread Kevin Wolf
depend on the filesystem. A test case replicating what Nir did manually would likely fail on XFS with its preallocation. Maybe we could operate on a file exposed by the FUSE export that is backed by qcow2, and then you can use 'qemu-img map' on that qcow2 image to verify the allocatio

Re: [PATCH] block/file-posix: Consider discard flag when opening

2024-06-19 Thread Kevin Wolf
ffc010 in ??? () #24 0x in ??? () I haven't checked the details yet, but my first impression is that this check should probably move to bdrv_co_do_pwrite_zeroes(). Kevin [1] Tests I did: # For discard $ echo -e 'qemu-io none0 "discard 0 1M"\nquit' | ./qemu-s

Re: [PATCH v5 5/5] iotests: Add `vvfat` tests

2024-06-13 Thread Kevin Wolf
ons(-) > > create mode 100644 tests/qemu-iotests/fat16.py > > create mode 100755 tests/qemu-iotests/tests/vvfat > > create mode 100755 tests/qemu-iotests/tests/vvfat.out > > Btw Kevin, I'm not sure if I should add myself to the MAINTAINERS file > for the new files

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

2024-06-13 Thread Kevin Wolf
Am 12.06.2024 um 21:21 hat Vladimir Sementsov-Ogievskiy geschrieben: > On 11.06.24 20:49, Kevin Wolf wrote: > > Am 13.03.2024 um 16:28 hat Vladimir Sementsov-Ogievskiy geschrieben: > > > Add test for a new backup option: discard-source. > > > > > > Signed-

Re: [PATCH v4 2/4] vvfat: Fix usage of `info.file.offset`

2024-06-11 Thread Kevin Wolf
Am 11.06.2024 um 18:22 hat Amjad Alsharafi geschrieben: > On Tue, Jun 11, 2024 at 04:30:53PM +0200, Kevin Wolf wrote: > > Am 11.06.2024 um 14:31 hat Amjad Alsharafi geschrieben: > > > On Mon, Jun 10, 2024 at 06:49:43PM +0200, Kevin Wolf wrote: > > > > Am 05.06.202

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

2024-06-11 Thread Kevin Wolf
his commit that introduced it. I haven't checked what get_actual_size(), but I'm running on XFS, so its preallocation could be causing this. We generally avoid checking the number of allocated blocks in image files for this reason. Kevin backup-discard-source fail [19

[PULL 3/8] aio: warn about iohandler_ctx special casing

2024-06-11 Thread Kevin Wolf
texts. Document this in order to reduce the chance of future bugs. Signed-off-by: Stefan Hajnoczi Message-ID: <20240506190622.56095-3-stefa...@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- include/block/aio.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/in

[PULL 6/8] linux-aio: add IO_CMD_FDSYNC command support

2024-06-11 Thread Kevin Wolf
pthreads results in TLB flushes. In a real-time guest environment, TLB flushes cause a latency spike. This patch helps to avoid such spikes. Reviewed-by: Stefan Hajnoczi Signed-off-by: Prasad Pandit Message-ID: <20240425070412.37248-1-ppan...@redhat.com> Reviewed-by: Kevin Wolf Signed-

[PULL 0/8] Block layer patches

2024-06-11 Thread Kevin Wolf
The following changes since commit 80e8f0602168f451a93e71cbb1d59e93d745e62e: Merge tag 'bsd-user-misc-2024q2-pull-request' of gitlab.com:bsdimp/qemu into staging (2024-06-09 11:21:55 -0700) are available in the Git repository at: https://repo.or.cz/qemu/kevin.git tags/for-upstream for you

[PULL 4/8] qemu-io: add cvtnum() error handling for zone commands

2024-06-11 Thread Kevin Wolf
l Cc: Sam Li Signed-off-by: Stefan Hajnoczi Message-ID: <20240507180558.377233-1-stefa...@redhat.com> Reviewed-by: Sam Li Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- qemu-io-cmds.c | 48 +++- 1 file changed, 47 insertions(+), 1 deleti

[PULL 5/8] block/copy-before-write: use uint64_t for timeout in nanoseconds

2024-06-11 Thread Kevin Wolf
442154-1-f.eb...@proxmox.com> Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/copy-before-write.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/copy-before-write.c b/block/copy-befo

[PULL 8/8] crypto/block: drop qcrypto_block_open() n_threads argument

2024-06-11 Thread Kevin Wolf
From: Stefan Hajnoczi The n_threads argument is no longer used since the previous commit. Remove it. Signed-off-by: Stefan Hajnoczi Message-ID: <20240527155851.892885-3-stefa...@redhat.com> Reviewed-by: Kevin Wolf Acked-by: Daniel P. Berrangé Signed-off-by: Kevin Wolf ---

[PULL 2/8] Revert "monitor: use aio_co_reschedule_self()"

2024-06-11 Thread Kevin Wolf
From: Stefan Hajnoczi Commit 1f25c172f837 ("monitor: use aio_co_reschedule_self()") was a code cleanup that uses aio_co_reschedule_self() instead of open coding coroutine rescheduling. Bug RHEL-34618 was reported and Kevin Wolf identified the root cause. I missed that aio_co_resch

[PULL 1/8] block: drop force_dup parameter of raw_reconfigure_getfd()

2024-06-11 Thread Kevin Wolf
From: "Denis V. Lunev via" Since commit 72373e40fbc, this parameter is always passed as 'false' from the caller. Signed-off-by: Denis V. Lunev CC: Andrey Zhadchenko CC: Kevin Wolf CC: Hanna Reitz Message-ID: <20240430170213.148558-1-...@openvz.org> Reviewed-by:

[PULL 7/8] block/crypto: create ciphers on demand

2024-06-11 Thread Kevin Wolf
rid of qcrypto_block_init_cipher() n_thread's argument and allocate ciphers on demand. Reported-by: Qing Wang Buglink: https://issues.redhat.com/browse/RHEL-36159 Signed-off-by: Stefan Hajnoczi Message-ID: <20240527155851.892885-2-stefa...@redhat.com> Reviewed-by: Kevin Wolf Acked-by: Daniel P. Berran

Re: [PATCH v4 2/4] vvfat: Fix usage of `info.file.offset`

2024-06-11 Thread Kevin Wolf
Am 11.06.2024 um 14:31 hat Amjad Alsharafi geschrieben: > On Mon, Jun 10, 2024 at 06:49:43PM +0200, Kevin Wolf wrote: > > Am 05.06.2024 um 02:58 hat Amjad Alsharafi geschrieben: > > > The field is marked as "the offset in the file (in clusters)", but it &g

Re: [PATCH v4 4/4] iotests: Add `vvfat` tests

2024-06-10 Thread Kevin Wolf
Am 10.06.2024 um 16:11 hat Amjad Alsharafi geschrieben: > On Mon, Jun 10, 2024 at 02:01:24PM +0200, Kevin Wolf wrote: > > With the updated test, I can catch the problems that are fixed by > > patches 1 and 2, but it still doesn't need patch 3 to pass. > > > > Kevi

Re: [PATCH v4 2/4] vvfat: Fix usage of `info.file.offset`

2024-06-10 Thread Kevin Wolf
mapping->end - mapping->begin); > } else > next_mapping->info.file.offset = mapping->info.file.offset + > -mapping->end - mapping->begin; > +(mapping->end - mapping->begin); > > mapping = next_mapping; > } Kevin

Re: [PATCH v4 4/4] iotests: Add `vvfat` tests

2024-06-10 Thread Kevin Wolf
the file without changing the number > of clusters > +""" > +fat16 = self.init_fat16() > + > +file = fat16.find_direntry("/FILE0.TXT") > +self.assertIsNotNone(file) > + > +

[PATCH] scsi-disk: Don't silently truncate serial number

2024-06-04 Thread Kevin Wolf
n't silently truncate the serial number string any more, but just error out if it would be truncated. Buglink: https://issues.redhat.com/browse/RHEL-3542 Suggested-by: Peter Krempa Signed-off-by: Kevin Wolf --- hw/scsi/scsi-disk.c | 20 +--- 1 file changed, 17 insertions(+), 3

  1   2   3   4   5   6   7   8   9   10   >