Re: [PATCH 09/23] hw/net/e1000e_core: Use definition to avoid dynamic stack allocation

2021-05-05 Thread Jason Wang
在 2021/5/6 上午5:10, Philippe Mathieu-Daudé 写道: The compiler isn't clever enough to figure 'min_buf_size' is a constant, so help it by using a definitions instead. Signed-off-by: Philippe Mathieu-Daudé Acked-by: Jason Wang --- hw/net/e1000e_core.c | 7 --- 1 file changed, 4

Re: [PATCH 1/7] migration: use GDateTime for formatting timestamp in snapshot names

2021-05-05 Thread Brad Smith
Thank you. On 5/5/2021 6:36 AM, Daniel P. Berrangé wrote: The GDateTime APIs provided by GLib avoid portability pitfalls, such as some platforms where 'struct timeval.tv_sec' field is still 'long' instead of 'time_t'. When combined with automatic cleanup, GDateTime often results in simpler code

Re: [PATCH 15/23] net: Avoid dynamic stack allocation

2021-05-05 Thread David Gibson
On Wed, May 05, 2021 at 11:10:39PM +0200, Philippe Mathieu-Daudé wrote: > Use autofree heap allocation instead of variable-length > array on the stack. > > Signed-off-by: Philippe Mathieu-Daudé fsl_etsec parts Acked-by: David Gibson > --- > hw/net/fsl_etsec/rings.c | 9 - >

Re: [PATCH 10/23] hw/ppc/pnv: Avoid dynamic stack allocation

2021-05-05 Thread David Gibson
On Wed, May 05, 2021 at 11:10:34PM +0200, Philippe Mathieu-Daudé wrote: > Use autofree heap allocation instead of variable-length > array on the stack. > > Signed-off-by: Philippe Mathieu-Daudé Acked-by: David Gibson > --- > hw/ppc/pnv.c | 4 ++-- > hw/ppc/spapr.c |

Re: [PATCH 21/23] target/ppc/kvm: Avoid dynamic stack allocation

2021-05-05 Thread David Gibson
On Wed, May 05, 2021 at 11:10:45PM +0200, Philippe Mathieu-Daudé wrote: > Use autofree heap allocation instead of variable-length > array on the stack. > > Signed-off-by: Philippe Mathieu-Daudé Acked-by: David Gibson > --- > target/ppc/kvm.c | 2 +- > 1 file changed, 1 insertion(+), 1

Re: [PATCH 11/23] hw/intc/xics: Avoid dynamic stack allocation

2021-05-05 Thread David Gibson
On Wed, May 05, 2021 at 11:10:35PM +0200, Philippe Mathieu-Daudé wrote: > Use autofree heap allocation instead of variable-length > array on the stack. > > Signed-off-by: Philippe Mathieu-Daudé Acked-by: David Gibson > --- > hw/intc/xics.c | 2 +- > 1 file changed, 1 insertion(+), 1

Re: [PATCH 07/23] hw/block/nvme: Use definition to avoid dynamic stack allocation

2021-05-05 Thread Keith Busch
On Wed, May 05, 2021 at 06:09:10PM -0500, Eric Blake wrote: > On 5/5/21 5:07 PM, Philippe Mathieu-Daudé wrote: > > +Eric > > > > On 5/5/21 11:22 PM, Keith Busch wrote: > >> On Wed, May 05, 2021 at 11:10:31PM +0200, Philippe Mathieu-Daudé wrote: > >>> The compiler isn't clever enough to figure

Re: [PATCH 07/23] hw/block/nvme: Use definition to avoid dynamic stack allocation

2021-05-05 Thread Warner Losh
On Wed, May 5, 2021, 5:10 PM Eric Blake wrote: > On 5/5/21 5:07 PM, Philippe Mathieu-Daudé wrote: > > +Eric > > > > On 5/5/21 11:22 PM, Keith Busch wrote: > >> On Wed, May 05, 2021 at 11:10:31PM +0200, Philippe Mathieu-Daudé wrote: > >>> The compiler isn't clever enough to figure

Re: [PATCH 07/23] hw/block/nvme: Use definition to avoid dynamic stack allocation

2021-05-05 Thread Eric Blake
On 5/5/21 5:07 PM, Philippe Mathieu-Daudé wrote: > +Eric > > On 5/5/21 11:22 PM, Keith Busch wrote: >> On Wed, May 05, 2021 at 11:10:31PM +0200, Philippe Mathieu-Daudé wrote: >>> The compiler isn't clever enough to figure 'SEG_CHUNK_SIZE' is >>> a constant! Help it by using a definitions instead.

Re: [PATCH 07/23] hw/block/nvme: Use definition to avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
+Eric On 5/5/21 11:22 PM, Keith Busch wrote: > On Wed, May 05, 2021 at 11:10:31PM +0200, Philippe Mathieu-Daudé wrote: >> The compiler isn't clever enough to figure 'SEG_CHUNK_SIZE' is >> a constant! Help it by using a definitions instead. > > I don't understand. Neither do I TBH... > It's

Re: [PATCH 04/23] chardev/baum: Avoid dynamic stack allocation

2021-05-05 Thread Samuel Thibault
Philippe Mathieu-Daudé, le mer. 05 mai 2021 23:10:28 +0200, a ecrit: > Use autofree heap allocation instead of variable-length > array on the stack. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Samuel Thibault > --- > chardev/baum.c | 3 ++- > 1 file changed, 2 insertions(+), 1

Re: [PATCH 02/23] chardev/baum: Replace magic values by X_MAX / Y_MAX definitions

2021-05-05 Thread Samuel Thibault
Philippe Mathieu-Daudé, le mer. 05 mai 2021 23:10:26 +0200, a ecrit: > Replace '84' magic value by the X_MAX definition, and '1' by Y_MAX. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Samuel Thibault > --- > chardev/baum.c | 11 +++ > 1 file changed, 7 insertions(+), 4

Re: [PATCH 03/23] chardev/baum: Use definitions to avoid dynamic stack allocation

2021-05-05 Thread Samuel Thibault
Marc-André Lureau, le jeu. 06 mai 2021 01:27:25 +0400, a ecrit: > @@ -408,7 +408,7 @@ static int baum_eat_packet(BaumChardev *baum, const > uint8_t *buf, int len) >          } >          timer_del(baum->cellCount_timer); > > -        memset(zero, 0, sizeof(zero)); > +     

[PATCH 22/23] tests/unit/test-vmstate: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé --- tests/unit/test-vmstate.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c index

[PATCH 16/23] ui/curses: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé --- ui/curses.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/curses.c b/ui/curses.c index e4f9588c3e8..f490b2d839d 100644 --- a/ui/curses.c +++

Re: [PATCH 04/23] chardev/baum: Avoid dynamic stack allocation

2021-05-05 Thread Marc-André Lureau
On Thu, May 6, 2021 at 1:15 AM Philippe Mathieu-Daudé wrote: > Use autofree heap allocation instead of variable-length > array on the stack. > > Signed-off-by: Philippe Mathieu-Daudé > Reviewed-by: Marc-André Lureau --- > chardev/baum.c | 3 ++- > 1 file changed, 2 insertions(+), 1

Re: [PATCH 02/23] chardev/baum: Replace magic values by X_MAX / Y_MAX definitions

2021-05-05 Thread Marc-André Lureau
On Thu, May 6, 2021 at 1:13 AM Philippe Mathieu-Daudé wrote: > Replace '84' magic value by the X_MAX definition, and '1' by Y_MAX. > > Signed-off-by: Philippe Mathieu-Daudé > Reviewed-by: Marc-André Lureau > --- > chardev/baum.c | 11 +++ > 1 file changed, 7 insertions(+), 4

Re: [PATCH 03/23] chardev/baum: Use definitions to avoid dynamic stack allocation

2021-05-05 Thread Marc-André Lureau
On Thu, May 6, 2021 at 1:14 AM Philippe Mathieu-Daudé wrote: > We know 'x * y' will be at most 'X_MAX * Y_MAX' (which is not > a big value, it is actually 84). Instead of having the compiler > use variable-length array, declare an array able to hold the > maximum 'x * y'. > > Signed-off-by:

[PATCH 13/23] hw/usb/hcd-xhci: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé --- hw/usb/hcd-xhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 7acfb8137bc..59a267e3c8b 100644 ---

Re: [PATCH 07/23] hw/block/nvme: Use definition to avoid dynamic stack allocation

2021-05-05 Thread Keith Busch
On Wed, May 05, 2021 at 11:10:31PM +0200, Philippe Mathieu-Daudé wrote: > The compiler isn't clever enough to figure 'SEG_CHUNK_SIZE' is > a constant! Help it by using a definitions instead. I don't understand. It's labeled 'const', so any reasonable compiler will place it in the 'text' segment

[PATCH 12/23] hw/i386/multiboot: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Replace the snprintf() call by g_strdup_printf(). Signed-off-by: Philippe Mathieu-Daudé --- hw/i386/multiboot.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hw/i386/multiboot.c

Re: [PATCH 03/23] chardev/baum: Use definitions to avoid dynamic stack allocation

2021-05-05 Thread Samuel Thibault
Philippe Mathieu-Daudé, le mer. 05 mai 2021 23:10:27 +0200, a ecrit: > We know 'x * y' will be at most 'X_MAX * Y_MAX' (which is not > a big value, it is actually 84). Instead of having the compiler > use variable-length array, declare an array able to hold the > maximum 'x * y'. > >

[PATCH 23/23] configure: Prohibit variable-length allocations by using -Wvla CPPFLAG

2021-05-05 Thread Philippe Mathieu-Daudé
Now that we converted all variable-length allocations in the repository, add the -Wvla CPPFLAG to trigger a build failure if such allocation is used. This should help avoiding vulnerabilities such CVE-2021-3527 (see commit range 3f67e2e7f13..05a40b172e4). Inspired-by: Gerd Hoffmann

[PATCH 11/23] hw/intc/xics: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé --- hw/intc/xics.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/intc/xics.c b/hw/intc/xics.c index 68f9d44feb4..c293d00d5c4 100644 --- a/hw/intc/xics.c +++

[PATCH 21/23] target/ppc/kvm: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé --- target/ppc/kvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index ae62daddf7d..90d0230eb86 100644 ---

[PATCH 10/23] hw/ppc/pnv: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé --- hw/ppc/pnv.c | 4 ++-- hw/ppc/spapr.c | 8 hw/ppc/spapr_pci_nvlink2.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git

[PATCH 20/23] util/iov: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé --- util/iov.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/iov.c b/util/iov.c index 58c7b35..fc76d717e14 100644 --- a/util/iov.c +++ b/util/iov.c @@

[PATCH 19/23] ui/vnc-enc-tight: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé --- ui/vnc-enc-tight.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c index cebd35841a9..ff6027cf8d4 100644

[PATCH 18/23] ui/vnc-enc-hextile: Use definitions to avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
We know 'pf.bytes_per_pixel' will be at most 'VNC_SERVER_FB_BYTES' (which is actually 4 bytes for 32bpp). Instead of having the compiler use variable-length array, use this 'small' maximum length and autofree to allocate the buffer on the heap. Signed-off-by: Philippe Mathieu-Daudé ---

[PATCH 17/23] ui/spice-display: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé --- ui/spice-display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/spice-display.c b/ui/spice-display.c index d22781a23d0..61c4259363b 100644 ---

[PATCH 15/23] net: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé --- hw/net/fsl_etsec/rings.c | 9 - hw/net/rocker/rocker_of_dpa.c | 2 +- net/dump.c| 2 +- net/tap.c | 2 +- 4 files changed, 7

[PATCH 14/23] hw/usb/hcd-ohci: Use definition to avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
The compiler isn't clever enough to figure 'width' is a constant, so help it by using a definitions instead. Signed-off-by: Philippe Mathieu-Daudé --- hw/usb/hcd-ohci.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index

[PATCH 07/23] hw/block/nvme: Use definition to avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
The compiler isn't clever enough to figure 'SEG_CHUNK_SIZE' is a constant! Help it by using a definitions instead. Signed-off-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index

[PATCH 05/23] io/channel-websock: Replace strlen(const_str) by sizeof(const_str) - 1

2021-05-05 Thread Philippe Mathieu-Daudé
The combined_key[... QIO_CHANNEL_WEBSOCK_GUID_LEN ...] array in qio_channel_websock_handshake_send_res_ok() expands to a call to strlen(QIO_CHANNEL_WEBSOCK_GUID), and the compiler doesn't realize the string is const, so consider combined_key[] being a variable-length array. To remove the

[PATCH 09/23] hw/net/e1000e_core: Use definition to avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
The compiler isn't clever enough to figure 'min_buf_size' is a constant, so help it by using a definitions instead. Signed-off-by: Philippe Mathieu-Daudé --- hw/net/e1000e_core.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/net/e1000e_core.c

[PATCH 06/23] hw/block/dataplane/virtio-blk: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé --- hw/block/dataplane/virtio-blk.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c

[PATCH 08/23] hw/block/nvme: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 2f6d4925826..905c4bb57af 100644 ---

[PATCH 04/23] chardev/baum: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé --- chardev/baum.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/chardev/baum.c b/chardev/baum.c index 0822e9ed5f3..bc09cda3471 100644 --- a/chardev/baum.c

[PATCH 03/23] chardev/baum: Use definitions to avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
We know 'x * y' will be at most 'X_MAX * Y_MAX' (which is not a big value, it is actually 84). Instead of having the compiler use variable-length array, declare an array able to hold the maximum 'x * y'. Signed-off-by: Philippe Mathieu-Daudé --- chardev/baum.c | 8 1 file changed, 4

[PATCH 01/23] block/vpc: Avoid dynamic stack allocation

2021-05-05 Thread Philippe Mathieu-Daudé
Use autofree heap allocation instead of variable-length array on the stack. Signed-off-by: Philippe Mathieu-Daudé --- block/vpc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/vpc.c b/block/vpc.c index 17a705b482a..9ed144331fd 100644 --- a/block/vpc.c +++

[PATCH 02/23] chardev/baum: Replace magic values by X_MAX / Y_MAX definitions

2021-05-05 Thread Philippe Mathieu-Daudé
Replace '84' magic value by the X_MAX definition, and '1' by Y_MAX. Signed-off-by: Philippe Mathieu-Daudé --- chardev/baum.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/chardev/baum.c b/chardev/baum.c index 5deca778bc4..adc3d7b3b56 100644 --- a/chardev/baum.c

[PATCH 00/23] misc: Remove variable-length arrays on the stack

2021-05-05 Thread Philippe Mathieu-Daudé
Hi, This series is inspired by Gerd Hoffmann and CVE-2021-3527. It removes all uses of variable-length arrays in the repository, then enable the '-Wvla' warning to avoid new code using vla to be merged. Mostly trivial patches using GLib autofree. Please review, Phil. (based on usb-20210505

Re: [PATCH] block/snapshot: Clarify goto fallback behavior

2021-05-05 Thread Vladimir Sementsov-Ogievskiy
05.05.2021 19:25, Max Reitz wrote: On 05.05.21 17:05, Vladimir Sementsov-Ogievskiy wrote: 03.05.2021 12:54, Max Reitz wrote: In the bdrv_snapshot_goto() fallback code, we work with a pointer to either bs->file or bs->backing. We close that child, Do we? We *detach it. close the node

Re: [PATCH v2 9/9] block/write-threshold: drop extra includes

2021-05-05 Thread Vladimir Sementsov-Ogievskiy
05.05.2021 19:23, Max Reitz wrote: On 04.05.21 10:25, Vladimir Sementsov-Ogievskiy wrote: Signed-off-by: Vladimir Sementsov-Ogievskiy ---   block/write-threshold.c | 3 ---   1 file changed, 3 deletions(-) diff --git a/block/write-threshold.c b/block/write-threshold.c index

[PATCH v3] Document qemu-img options data_file and data_file_raw

2021-05-05 Thread Connor Kuehl
The contents of this patch were initially developed and posted by Han Han[1], however, it appears the original patch was not applied. Since then, the relevant documentation has been moved and adapted to a new format. I've taken most of the original wording and tweaked it according to some of the

Re: [PATCH] xen-block: Use specific blockdev driver

2021-05-05 Thread Paul Durrant
On 30/04/2021 17:34, Anthony PERARD wrote: From: Anthony PERARD ... when a xen-block backend instance is created via xenstore. Following 8d17adf34f50 ("block: remove support for using "file" driver with block/char devices"), using the "file" blockdev driver for everything doesn't work

RE: [PATCH] block/rbd: Add support for rbd image encryption

2021-05-05 Thread Or Ozeri
Thanks Daniel!I prepared a modified patch addressing all of your suggestions (including resizing after formatting to increase the image size).The only thing I'm not sure about is your last point regarding reporting image is encrypted.When I followed the flow of "qemu-img info" on an

[PATCH] block/rbd: Add support for rbd image encryption

2021-05-05 Thread Or Ozeri
Starting from ceph Pacific, RBD has built-in support for image-level encryption. Currently supported formats are LUKS version 1 and 2. There are 2 new relevant librbd APIs for controlling encryption, both expect an open image context: rbd_encryption_format: formats an image (i.e. writes the LUKS

Re: [PATCH v4 4/6] block: Support multiple reopening with x-blockdev-reopen

2021-05-05 Thread Kevin Wolf
Am 18.03.2021 um 15:45 hat Vladimir Sementsov-Ogievskiy geschrieben: > 17.03.2021 20:15, Alberto Garcia wrote: > > Signed-off-by: Alberto Garcia > > --- > > qapi/block-core.json | 18 + > > blockdev.c | 78 +++--- > >

Re: [ANNOUNCE] libblkio v0.1.0 preview release

2021-05-05 Thread Kevin Wolf
Am 05.05.2021 um 18:19 hat Stefan Hajnoczi geschrieben: > On Tue, May 04, 2021 at 03:44:23PM +0200, Kevin Wolf wrote: > > Am 30.04.2021 um 17:49 hat Stefan Hajnoczi geschrieben: > > > On Thu, Apr 29, 2021 at 05:51:16PM +0200, Kevin Wolf wrote: > > > > Am 29.04.2021 um 16:05 hat Stefan Hajnoczi

Re: [PATCH v2 9/9] block/write-threshold: drop extra includes

2021-05-05 Thread Max Reitz
On 04.05.21 10:25, Vladimir Sementsov-Ogievskiy wrote: Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/write-threshold.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/block/write-threshold.c b/block/write-threshold.c index fbf4e6f5c4..db271c5537 100644 ---

Re: [ANNOUNCE] libblkio v0.1.0 preview release

2021-05-05 Thread Stefan Hajnoczi
On Tue, May 04, 2021 at 03:44:23PM +0200, Kevin Wolf wrote: > Am 30.04.2021 um 17:49 hat Stefan Hajnoczi geschrieben: > > On Thu, Apr 29, 2021 at 05:51:16PM +0200, Kevin Wolf wrote: > > > Am 29.04.2021 um 16:05 hat Stefan Hajnoczi geschrieben: > There is one more thing I'm wondering right now: Why

Re: [PATCH] block/snapshot: Clarify goto fallback behavior

2021-05-05 Thread Max Reitz
On 05.05.21 17:05, Vladimir Sementsov-Ogievskiy wrote: 03.05.2021 12:54, Max Reitz wrote: In the bdrv_snapshot_goto() fallback code, we work with a pointer to either bs->file or bs->backing. We close that child, Do we? We *detach it. close the node (with .bdrv_close()), apply the

Re: [PATCH v2 8/9] test-write-threshold: drop extra includes

2021-05-05 Thread Max Reitz
On 04.05.21 10:25, Vladimir Sementsov-Ogievskiy wrote: Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/unit/test-write-threshold.c | 2 -- 1 file changed, 2 deletions(-) Reviewed-by: Max Reitz

Re: [PATCH v2 7/9] test-write-threshold: drop extra TestStruct structure

2021-05-05 Thread Max Reitz
On 04.05.21 10:25, Vladimir Sementsov-Ogievskiy wrote: We don't need this extra logic: it doesn't make code simpler. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/unit/test-write-threshold.c | 20 +++- 1 file changed, 3 insertions(+), 17 deletions(-) Reviewed-by:

Re: [PATCH v2 6/9] test-write-threshold: drop extra tests

2021-05-05 Thread Max Reitz
On 04.05.21 10:25, Vladimir Sementsov-Ogievskiy wrote: Testing set/get of one 64bit variable doesn't seem necessary. We have a lot of such variables. Also remaining tests do test set/get anyway. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/unit/test-write-threshold.c | 43

Re: [PATCH v2 5/9] block/write-threshold: don't use aio context lock

2021-05-05 Thread Max Reitz
On 04.05.21 10:25, Vladimir Sementsov-Ogievskiy wrote: Instead of relying on aio context lock, let's make use of atomic operations. The tricky place is bdrv_write_threshold_check_write(): we want atomically unset bs->write_threshold_offset iff offset + bytes > bs->write_threshold_offset We

Re: [PATCH v4 3/6] iotests: Test replacing files with x-blockdev-reopen

2021-05-05 Thread Kevin Wolf
Am 17.03.2021 um 18:15 hat Alberto Garcia geschrieben: > This patch adds new tests in which we use x-blockdev-reopen to change > bs->file > > Signed-off-by: Alberto Garcia Looks good, but I wonder if we should also test the error paths. I think we're testing them for backing files, but as patch

Re: [PATCH] block/rbd: Add support for rbd image encryption

2021-05-05 Thread Daniel P . Berrangé
On Wed, May 05, 2021 at 03:32:09PM +, Or Ozeri wrote: >Thanks Daniel! >I prepared a modified patch addressing all of your suggestions (including >resizing after formatting to increase the image size). >The only thing I'm not sure about is your last point regarding reporting >

Re: [PATCH v2 3/9] test-write-threshold: rewrite test_threshold_(not_)trigger tests

2021-05-05 Thread Vladimir Sementsov-Ogievskiy
05.05.2021 17:28, Max Reitz wrote: On 04.05.21 10:25, Vladimir Sementsov-Ogievskiy wrote: These tests use bdrv_write_threshold_exceeded() API, which is used only for test. Well, now.  That used to be different before patch 1. Better is testing real API, which is used in block.c as well.

Re: [PATCH] block/snapshot: Clarify goto fallback behavior

2021-05-05 Thread Vladimir Sementsov-Ogievskiy
03.05.2021 12:54, Max Reitz wrote: In the bdrv_snapshot_goto() fallback code, we work with a pointer to either bs->file or bs->backing. We close that child, Do we? close the node (with .bdrv_close()), apply the snapshot on the child node, and then re-open the node (with .bdrv_open()).

Re: [PATCH 7/7] virtiofsd: use GDateTime for formatting timestamp for debug messages

2021-05-05 Thread Dr. David Alan Gilbert
* Daniel P. Berrangé (berra...@redhat.com) wrote: > The GDateTime APIs provided by GLib avoid portability pitfalls, such > as some platforms where 'struct timeval.tv_sec' field is still 'long' > instead of 'time_t'. When combined with automatic cleanup, GDateTime > often results in simpler code

Re: [PATCH v2 4/9] block/write-threshold: drop extra APIs

2021-05-05 Thread Max Reitz
On 04.05.21 10:25, Vladimir Sementsov-Ogievskiy wrote: bdrv_write_threshold_exceeded() is unused at all. bdrv_write_threshold_is_set() is used only to double check the value of bs->write_threshold_offset in tests. No real sense in it (both tests do check real value with help of

Re: [PATCH 1/7] migration: use GDateTime for formatting timestamp in snapshot names

2021-05-05 Thread Dr. David Alan Gilbert
* Daniel P. Berrangé (berra...@redhat.com) wrote: > The GDateTime APIs provided by GLib avoid portability pitfalls, such > as some platforms where 'struct timeval.tv_sec' field is still 'long' > instead of 'time_t'. When combined with automatic cleanup, GDateTime > often results in simpler code

Re: [PATCH v2 1/9] block/write-threshold: don't use write notifiers

2021-05-05 Thread Max Reitz
On 05.05.21 15:35, Vladimir Sementsov-Ogievskiy wrote: 05.05.2021 15:37, Max Reitz wrote: write-notifiers are used only for write-threshold. New code for such purpose should create filters. Let's better special-case write-threshold and drop write notifiers at all. (Actually, write-threshold is

Re: [PATCH v2 3/9] test-write-threshold: rewrite test_threshold_(not_)trigger tests

2021-05-05 Thread Max Reitz
On 04.05.21 10:25, Vladimir Sementsov-Ogievskiy wrote: These tests use bdrv_write_threshold_exceeded() API, which is used only for test. Well, now. That used to be different before patch 1. Better is testing real API, which is used in block.c as well. So, let's call

[PATCH v2 2/3] docs/interop/bitmaps: use blockdev-backup

2021-05-05 Thread Vladimir Sementsov-Ogievskiy
We are going to deprecate drive-backup, so use modern interface here. In examples where target image creation is shown, show blockdev-add as well. If target creation omitted, omit blockdev-add as well. Signed-off-by: Vladimir Sementsov-Ogievskiy --- docs/interop/bitmaps.rst | 285

[PATCH v2 3/3] qapi: deprecate drive-backup

2021-05-05 Thread Vladimir Sementsov-Ogievskiy
Modern way is using blockdev-add + blockdev-backup, which provides a lot more control on how target is opened. As example of drive-backup problems consider the following: User of drive-backup expects that target will be opened in the same cache and aio mode as source. Corresponding logic is in

[PATCH v2 1/3] docs/block-replication: use blockdev-backup

2021-05-05 Thread Vladimir Sementsov-Ogievskiy
We are going to deprecate drive-backup, so don't mention it here. Moreover, blockdev-backup seems more correct in the context. Signed-off-by: Vladimir Sementsov-Ogievskiy --- docs/block-replication.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git

Re: [PATCH v4 2/6] block: Allow changing bs->file on reopen

2021-05-05 Thread Kevin Wolf
Am 17.03.2021 um 18:15 hat Alberto Garcia geschrieben: > When the x-blockdev-reopen was added it allowed reconfiguring the > graph by replacing backing files, but changing the 'file' option was > forbidden. Because of this restriction some operations are not > possible, notably inserting and

[PATCH v2 0/3] qapi & doc: deprecate drive-backup

2021-05-05 Thread Vladimir Sementsov-Ogievskiy
Hi all! See 03 commit message for details. 01-02 are preparation docs update. v2: add a lot of documentation changes v1 was "[PATCH] qapi: deprecate drive-backup" Supersedes: <20210423125900.3640-1-vsement...@virtuozzo.com> Vladimir Sementsov-Ogievskiy (3): docs/block-replication: use

Re: [PATCH 4/4] block/vvfat: Avoid out of bounds write in create_long_filename()

2021-05-05 Thread Eric Blake
On 4/30/21 11:25 AM, Philippe Mathieu-Daudé wrote: > The direntry_t::name holds 11 bytes: > > typedef struct direntry_t { > uint8_t name[8 + 3]; > ... struct direntry_t is poorly laid out. A quick google search finds: https://www.ntfs.com/fat-filenames.htm which shows that a

Re: [PATCH v2 1/9] block/write-threshold: don't use write notifiers

2021-05-05 Thread Vladimir Sementsov-Ogievskiy
05.05.2021 15:37, Max Reitz wrote: write-notifiers are used only for write-threshold. New code for such purpose should create filters. Let's better special-case write-threshold and drop write notifiers at all. (Actually, write-threshold is special-cased anyway, as the only user of

Re: [PATCH 0/4] block/vvfat: Fix leaks and out of bounds writes

2021-05-05 Thread Johannes Schindelin
Hi Philippe, On Fri, 30 Apr 2021, Philippe Mathieu-Daudé wrote: > The first 3 patches are trivial leak fixes, the last > one is a RFC since I have no clue about this code. > > Johannes, you wrote this 18 years ago, do you still > remember? =) I do remember writing them, but I remember almost

Re: [PATCH v2 1/9] block/write-threshold: don't use write notifiers

2021-05-05 Thread Vladimir Sementsov-Ogievskiy
05.05.2021 15:37, Max Reitz wrote: On 04.05.21 10:25, Vladimir Sementsov-Ogievskiy wrote: write-notifiers are used only for write-threshold. New code for such purpose should create filters. Let's better special-case write-threshold and drop write notifiers at all. (Actually, write-threshold is

Re: [PATCH 4/7] usb/dev-mtp: use GDateTime for formatting timestamp for objects

2021-05-05 Thread Gerd Hoffmann
On Wed, May 05, 2021 at 11:36:59AM +0100, Daniel P. Berrangé wrote: > The GDateTime APIs provided by GLib avoid portability pitfalls, such > as some platforms where 'struct timeval.tv_sec' field is still 'long' > instead of 'time_t'. When combined with automatic cleanup, GDateTime > often results

Re: [PATCH v2 2/9] block: drop write notifiers

2021-05-05 Thread Max Reitz
On 04.05.21 10:25, Vladimir Sementsov-Ogievskiy wrote: They are unused now. Signed-off-by: Vladimir Sementsov-Ogievskiy --- include/block/block_int.h | 12 block.c | 1 - block/io.c| 6 -- 3 files changed, 19 deletions(-) Reviewed-by:

Re: [PATCH v2 1/9] block/write-threshold: don't use write notifiers

2021-05-05 Thread Max Reitz
On 04.05.21 10:25, Vladimir Sementsov-Ogievskiy wrote: write-notifiers are used only for write-threshold. New code for such purpose should create filters. Let's better special-case write-threshold and drop write notifiers at all. (Actually, write-threshold is special-cased anyway, as the only

Re: [PATCH v2 2/8] block: protect write threshold QMP commands from concurrent requests

2021-05-05 Thread Paolo Bonzini
On 05/05/21 10:55, Stefan Hajnoczi wrote: @@ -119,10 +118,8 @@ void qmp_block_set_write_threshold(const char *node_name, return; } -aio_context = bdrv_get_aio_context(bs); -aio_context_acquire(aio_context); - +/* Avoid a concurrent write_threshold_disable. */ +

Re: [PATCH 6/7] linux-user: use GDateTime for formatting timestamp for core file

2021-05-05 Thread Laurent Vivier
Le 05/05/2021 à 12:37, Daniel P. Berrangé a écrit : > The GDateTime APIs provided by GLib avoid portability pitfalls, such > as some platforms where 'struct timeval.tv_sec' field is still 'long' > instead of 'time_t'. When combined with automatic cleanup, GDateTime > often results in simpler code

Re: [for-6.1 2/4] virtio-blk: Configure all host notifiers in a single MR transaction

2021-05-05 Thread Greg Kurz
On Wed, 5 May 2021 11:14:23 +0100 Stefan Hajnoczi wrote: > On Wed, Apr 07, 2021 at 04:34:59PM +0200, Greg Kurz wrote: > > This allows the virtio-blk-pci device to batch the setup of all its > > host notifiers. This significantly improves boot time of VMs with a > > high number of vCPUs, e.g.

Re: Prevent compiler warning on block.c

2021-05-05 Thread Miroslav Rezanina
- Original Message - > From: "Peter Maydell" > To: "Miroslav Rezanina" > Cc: "QEMU Developers" , "Vladimir Sementsov-Ogievskiy" > , > "Qemu-block" > Sent: Wednesday, May 5, 2021 12:43:44 PM > Subject: Re: Prevent compiler warning on block.c > > On Wed, 5 May 2021 at 09:06, Miroslav

[PATCH 7/7] virtiofsd: use GDateTime for formatting timestamp for debug messages

2021-05-05 Thread Daniel P . Berrangé
The GDateTime APIs provided by GLib avoid portability pitfalls, such as some platforms where 'struct timeval.tv_sec' field is still 'long' instead of 'time_t'. When combined with automatic cleanup, GDateTime often results in simpler code too. Signed-off-by: Daniel P. Berrangé ---

Re: Prevent compiler warning on block.c

2021-05-05 Thread Peter Maydell
On Wed, 5 May 2021 at 09:06, Miroslav Rezanina wrote: > > Commit 3108a15cf (block: introduce bdrv_drop_filter()) introduced > uninitialized > variable to_cow_parent in bdrv_replace_node_common function that is used only > when > detach_subchain is true. It is used in two places. First if block

[PATCH 5/7] io: use GDateTime for formatting timestamp for websock headers

2021-05-05 Thread Daniel P . Berrangé
The GDateTime APIs provided by GLib avoid portability pitfalls, such as some platforms where 'struct timeval.tv_sec' field is still 'long' instead of 'time_t'. When combined with automatic cleanup, GDateTime often results in simpler code too. Signed-off-by: Daniel P. Berrangé ---

[PATCH 2/7] block: use GDateTime for formatting timestamp when dumping snapshot info

2021-05-05 Thread Daniel P . Berrangé
The GDateTime APIs provided by GLib avoid portability pitfalls, such as some platforms where 'struct timeval.tv_sec' field is still 'long' instead of 'time_t'. When combined with automatic cleanup, GDateTime often results in simpler code too. Signed-off-by: Daniel P. Berrangé --- block/qapi.c |

[PATCH 6/7] linux-user: use GDateTime for formatting timestamp for core file

2021-05-05 Thread Daniel P . Berrangé
The GDateTime APIs provided by GLib avoid portability pitfalls, such as some platforms where 'struct timeval.tv_sec' field is still 'long' instead of 'time_t'. When combined with automatic cleanup, GDateTime often results in simpler code too. Signed-off-by: Daniel P. Berrangé ---

[PATCH 3/7] net/rocker: use GDateTime for formatting timestamp in debug messages

2021-05-05 Thread Daniel P . Berrangé
The GDateTime APIs provided by GLib avoid portability pitfalls, such as some platforms where 'struct timeval.tv_sec' field is still 'long' instead of 'time_t'. When combined with automatic cleanup, GDateTime often results in simpler code too. Signed-off-by: Daniel P. Berrangé ---

[PATCH 1/7] migration: use GDateTime for formatting timestamp in snapshot names

2021-05-05 Thread Daniel P . Berrangé
The GDateTime APIs provided by GLib avoid portability pitfalls, such as some platforms where 'struct timeval.tv_sec' field is still 'long' instead of 'time_t'. When combined with automatic cleanup, GDateTime often results in simpler code too. Signed-off-by: Daniel P. Berrangé ---

[PATCH 0/7] replace all use of strftime() with g_date_time_format()

2021-05-05 Thread Daniel P . Berrangé
The GDateTime APIs provided by GLib avoid portability pitfalls, such as some platforms where 'struct timeval.tv_sec' field is still 'long' instead of 'time_t'. When combined with automatic cleanup, GDateTime often results in simpler code too. Daniel P. Berrangé (7): migration: use GDateTime for

Re: [PATCH v2 7/8] block/replication: do not acquire AioContext

2021-05-05 Thread Paolo Bonzini
On 19/04/21 10:55, Emanuele Giuseppe Esposito wrote: Replication functions are mostly called when the BDS is quiescent and does not have any pending I/O. They do not need to synchronize on anything since BDS and BB are now thread-safe. Signed-off-by: Paolo Bonzini Signed-off-by: Emanuele

[PATCH 4/7] usb/dev-mtp: use GDateTime for formatting timestamp for objects

2021-05-05 Thread Daniel P . Berrangé
The GDateTime APIs provided by GLib avoid portability pitfalls, such as some platforms where 'struct timeval.tv_sec' field is still 'long' instead of 'time_t'. When combined with automatic cleanup, GDateTime often results in simpler code too. Signed-off-by: Daniel P. Berrangé ---

Re: Prevent compiler warning on block.c

2021-05-05 Thread Vladimir Sementsov-Ogievskiy
05.05.2021 13:03, Paolo Bonzini wrote: On 05/05/21 10:05, Vladimir Sementsov-Ogievskiy wrote: diff --git a/block.c b/block.c index 874c22c43e..3ca27bd2d9 100644 --- a/block.c +++ b/block.c @@ -4851,7 +4851,7 @@ static int bdrv_replace_node_common(BlockDriverState *from,   Transaction *tran

Re: [PATCH 0/2] block: Fix Transaction leaks

2021-05-05 Thread Kevin Wolf
Am 04.05.2021 um 08:56 hat Vladimir Sementsov-Ogievskiy geschrieben: > 03.05.2021 14:05, Kevin Wolf wrote: > > These are two follow-up fixes for Vladimir's "block: update graph > > permissions update". The bugs were reported by Coverity. > > > > Kevin Wolf (2): > >block: Fix Transaction leak

Re: [for-6.1 4/4] virtio-scsi: Configure all host notifiers in a single MR transaction

2021-05-05 Thread Stefan Hajnoczi
On Wed, Apr 07, 2021 at 04:35:01PM +0200, Greg Kurz wrote: > This allows the virtio-scsi-pci device to batch the setup of all its > host notifiers. This significantly improves boot time of VMs with a > high number of vCPUs, e.g. from 6m5.563s down to 1m2.884s for a > pseries machine with 384

Re: [PATCH] block: simplify write-threshold and drop write notifiers

2021-05-05 Thread Vladimir Sementsov-Ogievskiy
05.05.2021 13:10, Stefan Hajnoczi wrote: On Thu, Apr 22, 2021 at 01:09:50AM +0300, Vladimir Sementsov-Ogievskiy wrote: @@ -1981,8 +1985,15 @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, int64_t bytes, } else { assert(child->perm & BLK_PERM_WRITE);

Re: [for-6.1 3/4] virtio-scsi: Set host notifiers and callbacks separately

2021-05-05 Thread Stefan Hajnoczi
On Wed, Apr 07, 2021 at 04:35:00PM +0200, Greg Kurz wrote: > Host notifiers are guaranteed to be idle until the callbacks are > hooked up with virtio_queue_aio_set_host_notifier_handler(). They > thus don't need to be set or unset with the AioContext lock held. > > Do this outside the critical

Re: [for-6.1 2/4] virtio-blk: Configure all host notifiers in a single MR transaction

2021-05-05 Thread Stefan Hajnoczi
On Wed, Apr 07, 2021 at 04:34:59PM +0200, Greg Kurz wrote: > This allows the virtio-blk-pci device to batch the setup of all its > host notifiers. This significantly improves boot time of VMs with a > high number of vCPUs, e.g. from 3m26.186s down to 0m58.023s for a > pseries machine with 384

Re: [PATCH] qcow2: set bdi->is_dirty

2021-05-05 Thread Kevin Wolf
Am 04.05.2021 um 18:06 hat Vladimir Sementsov-Ogievskiy geschrieben: > Set bdi->is_dirty, so that qemu-img info could show dirty flag. > > After this commit the following check will show '"dirty-flag": true': > > ./build/qemu-img create -f qcow2 -o lazy_refcounts=on x 1M > ./build/qemu-io x >

Re: [PATCH v2 6/8] block: do not acquire AioContext in check_to_replace_node

2021-05-05 Thread Paolo Bonzini
On 19/04/21 10:55, Emanuele Giuseppe Esposito wrote: + * + * Called with AioContext lock held. ... for @to_replace_bs. aio_context_acquire(replace_aio_context); +if (!check_to_replace_node(bs, to_replace_bs, replaces, errp)) { A release is missing here. Paolo +

  1   2   >