[PATCH v5 1/7] new configure option to enable gstreamer

2025-04-30 Thread Dietmar Maurer
GStreamer is required to implement H264 encoding for VNC. Please note that QEMU already depends on this library when you enable Spice. Signed-off-by: Dietmar Maurer --- meson.build | 10 ++ meson_options.txt | 2 ++ scripts/meson-buildoptions.sh | 3

[PATCH v5 6/7] h264: add hardware encoders

2025-04-30 Thread Dietmar Maurer
add most common hardware encoders: - nvh264enc: for NVidia hardware - vaapih264enc: for common AMD and Intel cards Signed-off-by: Dietmar Maurer --- ui/vnc-enc-h264.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c

[PATCH v5 3/7] vnc: h264: send additional frames after the display is clean

2025-04-30 Thread Dietmar Maurer
flush the H264 encoder data. Signed-off-by: Dietmar Maurer --- ui/vnc.c | 13 - ui/vnc.h | 3 +++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/ui/vnc.c b/ui/vnc.c index ba71589c6f..975f3325e1 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3234,6 +3234,7 @@ static void vnc_re

[PATCH v5 0/7] Add VNC Open H.264 Encoding

2025-04-30 Thread Dietmar Maurer
initialize gst during argument processing - add hardware encoders Changes in v2: - cleanup: h264: remove wrong libavcodec_ prefix from function names - search for available h264 encoder, and only enable h264 if a encoder is available - new vnc option to configure h264 at server side Dietm

[PATCH v5 4/7] h264: search for available h264 encoder

2025-04-30 Thread Dietmar Maurer
et reasonable/usable results. Signed-off-by: Dietmar Maurer --- ui/vnc-enc-h264.c | 82 ++- ui/vnc.h | 1 + 2 files changed, 68 insertions(+), 15 deletions(-) diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c index 26e8c19270..191e3aeb39 100644 ---

[PATCH v5 5/7] h264: new vnc options to configure h264 at server side

2025-04-30 Thread Dietmar Maurer
h264: on/off (default is on) h264-encoders: A colon separated list of allowed gstreamer encoders. Select the first available encoder from that list (default is "x264enc:openh264enc"). Signed-off-by: Dietmar Maurer --- ui/vnc-enc-h264.c | 40 ++--

[PATCH v5 2/7] add vnc h264 encoder

2025-04-30 Thread Dietmar Maurer
-> videoconvert -> x264enc -> appsink Note: videoconvert is required for RGBx to YUV420 conversion. The code still use the VNC server framebuffer change detection, and only encodes and sends video frames if there are changes. Signed-off-by: Dietmar Maurer --- ui/meson.build| 1 + u

[PATCH v5 7/7] h264: stop gstreamer pipeline before destroying, cleanup on exit

2025-04-30 Thread Dietmar Maurer
Some encoders can hang indefinitely (i.e. nvh264enc) if the pipeline is not stopped before it is destroyed (Observed on Debian bookworm). Signed-off-by: Dietmar Maurer --- include/system/system.h | 1 + include/ui/console.h| 1 + system/runstate.c | 2 ++ system/vl.c

Re: [PATCH v4 8/8] h264: stop gstreamer pipeline before destroying, cleanup on exit

2025-04-29 Thread Dietmar Maurer
> On 29.4.2025 07:17 CEST Marc-André Lureau wrote: > > > Hi > > On Mon, Apr 28, 2025 at 4:08 PM Dietmar Maurer wrote: > > > > > In file included from /home/elmarco/src/qemu/include/ui/console.h:4, > > > from ../system/runstate.c:5

Re: [PATCH v4 8/8] h264: stop gstreamer pipeline before destroying, cleanup on exit

2025-04-28 Thread Dietmar Maurer
> In file included from /home/elmarco/src/qemu/include/ui/console.h:4, > from ../system/runstate.c:54: > /home/elmarco/src/qemu/include/ui/qemu-pixman.h:10:10: fatal error: > pixman.h: No such file or directory >10 | #include > | ^~ Just noticed that af

[PATCH v4 6/8] h264: new vnc options to configure h264 at server side

2025-04-28 Thread Dietmar Maurer
h264: on/off (default is on) h264-encoders: A colon separated list of allowed gstreamer encoders. Select the first available encoder from that list (default is "x264enc:openh264enc"). Signed-off-by: Dietmar Maurer --- ui/vnc-enc-h264.c | 40 ++--

[PATCH v4 1/8] new configure option to enable gstreamer

2025-04-28 Thread Dietmar Maurer
GStreamer is required to implement H264 encoding for VNC. Please note that QEMU already depends on this library when you enable Spice. Signed-off-by: Dietmar Maurer --- meson.build | 10 ++ meson_options.txt | 2 ++ scripts/meson-buildoptions.sh | 3

[PATCH v4 3/8] add vnc h264 encoder

2025-04-28 Thread Dietmar Maurer
-> videoconvert -> x264enc -> appsink Note: videoconvert is required for RGBx to YUV420 conversion. The code still use the VNC server framebuffer change detection, and only encodes and sends video frames if there are changes. Signed-off-by: Dietmar Maurer --- ui/meson.build| 1 + u

[PATCH v4 7/8] h264: add hardware encoders

2025-04-28 Thread Dietmar Maurer
add most common hardware encoders: - nvh264enc: for NVidia hardware - vaapih264enc: for common AMD and Intel cards Signed-off-by: Dietmar Maurer --- ui/vnc-enc-h264.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c

[PATCH v4 5/8] h264: search for available h264 encoder

2025-04-28 Thread Dietmar Maurer
et reasonable/usable results. Signed-off-by: Dietmar Maurer --- ui/vnc-enc-h264.c | 82 ++- ui/vnc.h | 1 + 2 files changed, 68 insertions(+), 15 deletions(-) diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c index 26e8c19270..191e3aeb39 100644 ---

[PATCH v4 0/8] Add VNC Open H.264 Encoding

2025-04-28 Thread Dietmar Maurer
hanges in v2: - cleanup: h264: remove wrong libavcodec_ prefix from function names - search for available h264 encoder, and only enable h264 if a encoder is available - new vnc option to configure h264 at server side Dietmar Maurer (8): new configure option to enable gstreamer vnc:

[PATCH v4 2/8] vnc: initialize gst during argument processing

2025-04-28 Thread Dietmar Maurer
So that we can set --gst- options on the qemu command line. Signed-off-by: Dietmar Maurer --- system/vl.c | 8 1 file changed, 8 insertions(+) diff --git a/system/vl.c b/system/vl.c index ec93988a03..c7fff02da2 100644 --- a/system/vl.c +++ b/system/vl.c @@ -140,6 +140,10 @@ #include

[PATCH v4 8/8] h264: stop gstreamer pipeline before destroying, cleanup on exit

2025-04-28 Thread Dietmar Maurer
Some encoders can hang indefinitely (i.e. nvh264enc) if the pipeline is not stopped before it is destroyed (Observed on Debian bookworm). Signed-off-by: Dietmar Maurer --- include/ui/console.h | 1 + system/runstate.c| 2 ++ ui/vnc-enc-h264.c| 18 ++ ui/vnc.c

[PATCH v4 4/8] vnc: h264: send additional frames after the display is clean

2025-04-28 Thread Dietmar Maurer
flush the H264 encoder data. Signed-off-by: Dietmar Maurer --- ui/vnc.c | 13 - ui/vnc.h | 3 +++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/ui/vnc.c b/ui/vnc.c index c1f2c3ae15..0efdc9ec39 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3236,6 +3236,7 @@ static void vnc_re

Re: [PATCH v3 5/9] h264: new vnc option to configure h264 at server side

2025-04-25 Thread Dietmar Maurer
> Overloading one config option to both turn h264 on/off, > an choose encoding is not very desirable. > > IMHO there should be a "h264=" option to turn it > on/off, and a separate flag to choose the encoder. will do that. > Do we even need to give a list of encoders, as opposed > to just choosi

Re: [PATCH v3 2/9] add vnc h264 encoder

2025-04-24 Thread Dietmar Maurer
> > To be more specific, it is either a x264 encoder bug, or a web > > browser (VideoEncoder api) bug. You can reproduce it with noVNC. > > > > Form what I found out, newer versions of x264 do not use the problematic > > mode at all (but we want to support older versions). > > Do you have exampl

Re: [PATCH v3 2/9] add vnc h264 encoder

2025-04-24 Thread Dietmar Maurer
> > +g_object_set( > > +vs->h264->gst_encoder, > > +"tune", 4, /* zerolatency */ > > +/* > > + * fix for zerolatency with novnc (without, noVNC displays > > + * green stripes) > > + */ > > +"threads", 1, > > It seems a bit dubious for QEM

Re: [PATCH v3 2/9] add vnc h264 encoder

2025-04-24 Thread Dietmar Maurer
> > > > +void vnc_h264_clear(VncState *vs) > > > > +{ > > > > +if (!vs->h264) { > > > > +return; > > > > +} > > > > > > unnecessary > > > > This is required. For example if you disable h264, vs->h264 is > > always NULL, and we unconditionally call vnc_h264_clear(). > > > > Why do yo

Re: [PATCH v3 2/9] add vnc h264 encoder

2025-04-24 Thread Dietmar Maurer
> On 24.4.2025 09:32 CEST Dietmar Maurer wrote: > > > > > +gst_object_ref(vs->h264->source); > > > +if (!gst_bin_add(GST_BIN(vs->h264->pipeline), vs->h264->source)) { > > > +gst_object_unref(vs->h264->source); >

Re: [PATCH v3 2/9] add vnc h264 encoder

2025-04-24 Thread Dietmar Maurer
> > +gst_object_ref(vs->h264->source); > > +if (!gst_bin_add(GST_BIN(vs->h264->pipeline), vs->h264->source)) { > > +gst_object_unref(vs->h264->source); > > +VNC_DEBUG("Could not add source to gst pipeline\n"); > > +goto error; > > +} > > If you put the gst_obj

Re: [PATCH v3 2/9] add vnc h264 encoder

2025-04-23 Thread Dietmar Maurer
> > > > +VNC_DEBUG("Could not add source to gst pipeline\n"); > > > > +goto error; > > > > +} > > > > + > > > > +gst_object_ref(vs->h264->convert); > > > > +if (!gst_bin_add(GST_BIN(vs->h264->pipeline), vs->h264->convert)) { > > > > > > Can you use gst_bin_add_many() ? >

Re: [PATCH v3 2/9] add vnc h264 encoder

2025-04-23 Thread Dietmar Maurer
> On 19.4.2025 07:24 CEST Marc-André Lureau wrote: > > > Hi > > On Fri, Apr 18, 2025 at 3:41 PM Dietmar Maurer wrote: > > > > This patch implements H264 support for VNC. The RFB protocol > > extension is defined in: > > > > https://github.com/r

Re: [PATCH v3 9/9] h264: register shutdown notifiers, stop pipeline in destroy_encoder_context

2025-04-22 Thread Dietmar Maurer
> On 22.4.2025 09:07 CEST Marc-André Lureau wrote: > > > > > > Given that h264 code depends on VNC state, can you make VNC > > > close/clean the connection instead? > > > > For all open VNC connections? > > > > Yes, we should have a cleanup path instead of shutdown notifiers. I am finally conf

Re: [PATCH v3 9/9] h264: register shutdown notifiers, stop pipeline in destroy_encoder_context

2025-04-22 Thread Dietmar Maurer
> On 22.4.2025 08:39 CEST Marc-André Lureau wrote: > > > Hi > > On Tue, Apr 22, 2025 at 10:37 AM Dietmar Maurer wrote: > > > > > On Fri, Apr 18, 2025 at 3:30 PM Dietmar Maurer > > > wrote: > > > > > > > > Some encoders

Re: [PATCH v3 9/9] h264: register shutdown notifiers, stop pipeline in destroy_encoder_context

2025-04-21 Thread Dietmar Maurer
> On Fri, Apr 18, 2025 at 3:30 PM Dietmar Maurer wrote: > > > > Some encoders can hang indefinetly (i.e. nvh264enc) if > > indefinitely > > > the pipeline is not stopped before it is destroyed > > (Observed on Debian bookworm). > > but why do you need

[PATCH v3 2/9] add vnc h264 encoder

2025-04-18 Thread Dietmar Maurer
-> videoconvert -> x264enc -> appsink Note: videoconvert is required for RGBx to YUV420 conversion. The code still use the VNC server framebuffer change detection, and only encodes and sends video frames if there are changes. Signed-off-by: Dietmar Maurer --- ui/meson.build| 1 + u

[PATCH v3 7/9] h264: do not reduce vnc update speed while we have an active h264 stream

2025-04-18 Thread Dietmar Maurer
Signed-off-by: Dietmar Maurer --- ui/vnc.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ui/vnc.c b/ui/vnc.c index feab4c0043..6db03a1550 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3222,6 +3222,7 @@ static void vnc_refresh(DisplayChangeListener *dcl) VncDisplay *vd

[PATCH v3 4/9] h264: search for available h264 encoder

2025-04-18 Thread Dietmar Maurer
et reasonable/usable results. Signed-off-by: Dietmar Maurer --- ui/vnc-enc-h264.c | 89 +++ ui/vnc.h | 1 + 2 files changed, 75 insertions(+), 15 deletions(-) diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c index 3abe6a1528..047f4a3128 100644 ---

[PATCH v3 1/9] new configure option to enable gstreamer

2025-04-18 Thread Dietmar Maurer
GStreamer is required to implement H264 encoding for VNC. Please note that QEMU already depends on this library when you enable Spice. Signed-off-by: Dietmar Maurer --- meson.build | 10 ++ meson_options.txt | 2 ++ scripts/meson-buildoptions.sh | 5

[PATCH v3 0/9] Add VNC Open H.264 Encoding

2025-04-18 Thread Dietmar Maurer
- new vnc option to configure h264 at server side Dietmar Maurer (9): new configure option to enable gstreamer add vnc h264 encoder vnc: h264: send additional frames after the display is clean h264: search for available h264 encoder h264: new vnc option to configure h264 at server side

[PATCH v3 6/9] h264: add hardware encoders

2025-04-18 Thread Dietmar Maurer
Signed-off-by: Dietmar Maurer --- ui/vnc-enc-h264.c | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c index 0f89cafbf6..840674dbdb 100644 --- a/ui/vnc-enc-h264.c +++ b/ui/vnc-enc-h264.c @@ -29,15 +29,17 @@ static

[PATCH v3 5/9] h264: new vnc option to configure h264 at server side

2025-04-18 Thread Dietmar Maurer
Values can be 'on', 'off', or a space sparated list of allowed gstreamer encoders. - on: automatically select the encoder - off: disbale h264 - encoder-list: select first available encoder from that list. Signed-off-by: Dietmar Maurer --- ui

[PATCH v3 9/9] h264: register shutdown notifiers, stop pipeline in destroy_encoder_context

2025-04-18 Thread Dietmar Maurer
Some encoders can hang indefinetly (i.e. nvh264enc) if the pipeline is not stopped before it is destroyed (Observed on Debian bookworm). Signed-off-by: Dietmar Maurer --- ui/vnc-enc-h264.c | 50 ++- ui/vnc.h | 1 + 2 files changed, 42

[PATCH v3 3/9] vnc: h264: send additional frames after the display is clean

2025-04-18 Thread Dietmar Maurer
flush the H264 encoder data. Signed-off-by: Dietmar Maurer --- ui/vnc.c | 25 - ui/vnc.h | 3 +++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/ui/vnc.c b/ui/vnc.c index aed25b0183..badc7912c0 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3239,7 +3239,30 @@ s

[PATCH v3 8/9] vnc: initialize gst during argument processing

2025-04-18 Thread Dietmar Maurer
So that we can set --gst- options on the qemu command line. Signed-off-by: Dietmar Maurer --- system/vl.c | 8 ui/vnc.c| 4 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/system/vl.c b/system/vl.c index ec93988a03..c7fff02da2 100644 --- a/system/vl.c +++ b

[PATCH v2 3/6] vnc: h264: send additional frames after the display is clean

2025-04-10 Thread Dietmar Maurer
flush the H264 encoder data. Signed-off-by: Dietmar Maurer --- ui/vnc.c | 25 - ui/vnc.h | 3 +++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/ui/vnc.c b/ui/vnc.c index 2e60b55e47..4ba0b715fd 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3239,7 +3239,30 @@ s

[PATCH v2 5/6] h264: search for available h264 encoder

2025-04-10 Thread Dietmar Maurer
et reasonable/usable results. Signed-off-by: Dietmar Maurer --- ui/vnc-enc-h264.c | 76 +++ ui/vnc.h | 1 + 2 files changed, 65 insertions(+), 12 deletions(-) diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c index 9e01b8a548..3eabfc2cfe 100644 ---

[PATCH 3/3] vnc: h264: send additional frames after the display is clean

2025-04-10 Thread Dietmar Maurer
So that encoder can improve the picture quality. Signed-off-by: Dietmar Maurer --- ui/vnc.c | 25 - ui/vnc.h | 3 +++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/ui/vnc.c b/ui/vnc.c index 2e60b55e47..4ba0b715fd 100644 --- a/ui/vnc.c +++ b/ui/vnc.c

[PATCH v2 1/6] new configure option to enable gstreamer

2025-04-10 Thread Dietmar Maurer
GStreamer is required to implement H264 encoding for VNC. Please note that QEMU already depends on this library when you enable Spice. Signed-off-by: Dietmar Maurer --- meson.build | 10 ++ meson_options.txt | 2 ++ scripts/meson-buildoptions.sh | 5

[PATCH v2 0/6] Add VNC Open H.264 Encoding

2025-04-10 Thread Dietmar Maurer
g, because uncompressed audio need too much bandwidth. Changes in v2: - cleanup: h264: remove wrong libavcodec_ prefix from function names - search for available h264 encoder, and only enable h264 if a encoder is available - new vnc option to configure h264 at server side Dietmar Maurer (6): new config

[PATCH v2 6/6] h264: new vnc option to configure h264 at server side

2025-04-10 Thread Dietmar Maurer
Values can be 'on', 'off', or a space sparated list of allowed gstreamer encoders. - on: automatically select the encoder - off: disbale h264 - encoder-list: select first available encoder from that list. Signed-off-by: Dietmar Maurer --- ui/vnc-enc-h264.c | 28

[PATCH v2 4/6] h264: remove wrong libavcodec_ prefix from function names

2025-04-10 Thread Dietmar Maurer
Signed-off-by: Dietmar Maurer --- ui/vnc-enc-h264.c | 36 ++-- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/ui/vnc-enc-h264.c b/ui/vnc-enc-h264.c index ca8e206335..9e01b8a548 100644 --- a/ui/vnc-enc-h264.c +++ b/ui/vnc-enc-h264.c @@ -3,7 +3,7

[PATCH v2 2/6] add vnc h264 encoder

2025-04-10 Thread Dietmar Maurer
-> videoconvert -> x264enc -> appsink Note: videoconvert is required for RGBx to YUV420 conversion. The code still use the VNC server framebuffer change detection, and only encodes and sends video frames if there are changes. Signed-off-by: Dietmar Maurer --- ui/meson.build| 1 + u

[PATCH 2/3] add vnc h264 encoder

2025-04-10 Thread Dietmar Maurer
Signed-off-by: Dietmar Maurer --- ui/meson.build| 1 + ui/vnc-enc-h264.c | 269 ++ ui/vnc-jobs.c | 49 ++--- ui/vnc.c | 21 ui/vnc.h | 21 5 files changed, 346 insertions(+), 15 deletions(-) create mode

Re: [PATCH 2/3] add vnc h264 encoder

2025-04-08 Thread Dietmar Maurer
> > > > +#include > > > > + > > > > +static void libavcodec_destroy_encoder_context(VncState *vs) > > > > > > it's not libavcodec. > > > > I will fix that in v2. > > What about encodebin suggestion? I found no way to configure codec specific option (i.e. x264 zerolatency). I there a way? It is

Re: [PATCH 2/3] add vnc h264 encoder

2025-04-08 Thread Dietmar Maurer
> Please resend the series with a cover letter > (https://www.qemu.org/docs/master/devel/submitting-a-patch.html#use-git-format-patch) Ok, just resend this series with a cover letter and commit message. (patches unchanged) > > +#include > > + > > +static void libavcodec_destroy_encoder_context(V

[PATCH 1/3] new configure option to enable gstreamer

2025-04-08 Thread Dietmar Maurer
GStreamer is required to implement H264 encoding for VNC. Please note that QEMU already depends on this library when you enable Spice. Signed-off-by: Dietmar Maurer --- meson.build | 10 ++ meson_options.txt | 2 ++ scripts/meson-buildoptions.sh | 5

[PATCH 2/3] add vnc h264 encoder

2025-04-08 Thread Dietmar Maurer
-> videoconvert -> x264enc -> appsink Note: videoconvert is required for RGBx to YUV420 conversion. The code still use the VNC server framebuffer change detection, and only encodes and sends video frames if there are changes. Signed-off-by: Dietmar Maurer --- ui/meson.build| 1 + u

[PATCH 0/3] Add VNC Open H.264 Encoding

2025-04-08 Thread Dietmar Maurer
g, because uncompressed audio need too much bandwidth. Dietmar Maurer (3): new configure option to enable gstreamer add vnc h264 encoder vnc: h264: send additional frames after the display is clean meson.build | 10 ++ meson_options.txt | 2 + scripts/meson-buildoption

[PATCH 3/3] vnc: h264: send additional frames after the display is clean

2025-04-08 Thread Dietmar Maurer
flush the H264 encoder data. Signed-off-by: Dietmar Maurer --- ui/vnc.c | 25 - ui/vnc.h | 3 +++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/ui/vnc.c b/ui/vnc.c index 2e60b55e47..4ba0b715fd 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3239,7 +3239,30 @@ s

[PATCH 1/3] new configure option to enable gstreamer

2025-04-07 Thread Dietmar Maurer
Signed-off-by: Dietmar Maurer --- meson.build | 10 ++ meson_options.txt | 2 ++ scripts/meson-buildoptions.sh | 5 - 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 41f68d3806..28ca37855a 100644 --- a

Re: qemu_coroutine_yield switches thread?

2020-04-16 Thread Dietmar Maurer
> > quick question: Can a resume from a qemu_coroutine_yield happen in a > > different thread? > > > > Well, it can, since I'm seeing it happen, but is that okay or a bug? > > Yes, it can happen. At least for devices like IDE where a request is > started during a vmexit (MMIO or I/O port write),

Re: bdrv_drained_begin deadlock with io-threads

2020-04-03 Thread Dietmar Maurer
> On April 3, 2020 10:47 AM Kevin Wolf wrote: > > > Am 03.04.2020 um 10:26 hat Dietmar Maurer geschrieben: > > > With the following patch, it seems to survive for now. I'll give it some > > > more testing tomorrow (also qemu-iotests to check that I didn&#

Re: bdrv_drained_begin deadlock with io-threads

2020-04-03 Thread Dietmar Maurer
> With the following patch, it seems to survive for now. I'll give it some > more testing tomorrow (also qemu-iotests to check that I didn't > accidentally break something else.) Wow, that was fast! Seems your patch fixes the bug! I wonder what commit introduced that problem, maybe: https://gith

Re: bdrv_drained_begin deadlock with io-threads

2020-04-02 Thread Dietmar Maurer
> It does looks more like your case because I now have bs.in_flight == 0 > and the BlockBackend of the scsi-hd device has in_flight == 8. yes, this looks very familiar. > Of course, this still doesn't answer why it happens, and I'm not sure if we > can tell without adding some debug code. > > I

Re: bdrv_drained_begin deadlock with io-threads

2020-04-02 Thread Dietmar Maurer
> Can you reproduce the problem with my script, but pointing it to your > Debian image and running stress-ng instead of dd? yes > If so, how long does > it take to reproduce for you? I sometimes need up to 130 iterations ... Worse, I thought several times the bug is gone, but then it triggered

Re: bdrv_drained_begin deadlock with io-threads

2020-04-02 Thread Dietmar Maurer
> > Do you also run "stress-ng -d 5" indied the VM? > > I'm not using the exact same test case, but something that I thought > would be similar enough. Specifically, I run the script below, which > boots from a RHEL 8 CD and in the rescue shell, I'll do 'dd if=/dev/zero > of=/dev/sda' This test

Re: bdrv_drained_begin deadlock with io-threads

2020-04-02 Thread Dietmar Maurer
> It seems to fix it, yes. Now I don't get any hangs any more. I just tested using your configuration, and a recent centos8 image running dd loop inside it: # while dd if=/dev/urandom of=testfile.raw bs=1M count=100; do sync; done With that, I am unable to trigger the bug. Would you mind runni

Re: bdrv_drained_begin deadlock with io-threads

2020-04-01 Thread Dietmar Maurer
> > But, IMHO the commit is not the reason for (my) bug - It just makes > > it easier to trigger... I can see (my) bug sometimes with 4.1.1, although > > I have no easy way to reproduce it reliable. > > > > Also, Stefan sent some patches to the list to fix some of the problems. > > > > https://li

Re: bdrv_drained_begin deadlock with io-threads

2020-04-01 Thread Dietmar Maurer
> That's a pretty big change, and I'm not sure how it's related to > completed requests hanging in the thread pool instead of reentering the > file-posix coroutine. But I also tested it enough that I'm confident > it's really the first bad commit. > > Maybe you want to try if your problem starts a

Re: bdrv_drained_begin deadlock with io-threads

2020-04-01 Thread Dietmar Maurer
> > I really nobody else able to reproduce this (somebody already tried to > > reproduce)? > > I can get hangs, but that's for job_completed(), not for starting the > job. Also, my hangs have a non-empty bs->tracked_requests, so it looks > like a different case to me. Please can you post the com

Re: bdrv_drained_begin deadlock with io-threads

2020-04-01 Thread Dietmar Maurer
> On April 1, 2020 5:37 PM Dietmar Maurer wrote: > > > > > I really nobody else able to reproduce this (somebody already tried to > > > reproduce)? > > > > I can get hangs, but that's for job_completed(), not for starting the > > job. Al

Re: bdrv_drained_begin deadlock with io-threads

2020-03-31 Thread Dietmar Maurer
> On March 31, 2020 5:37 PM Kevin Wolf wrote: > > > Am 31.03.2020 um 17:24 hat Dietmar Maurer geschrieben: > > > > > > How can I see/debug those waiting request? > > > > > > Examine bs->tracked_requests list. > > > > >

Re: bdrv_drained_begin deadlock with io-threads

2020-03-31 Thread Dietmar Maurer
> > How can I see/debug those waiting request? > > Examine bs->tracked_requests list. > > BdrvTrackedRequest has "Coroutine *co" field. It's a pointer of coroutine of > this request. You may use qemu-gdb script to print request's coroutine > back-trace: I would, but there are no tracked requ

Re: bdrv_drained_begin deadlock with io-threads

2020-03-31 Thread Dietmar Maurer
> > After a few iteration the VM freeze inside bdrv_drained_begin(): > > > > Thread 1 (Thread 0x7fffe9291080 (LWP 30949)): > > #0 0x75cb3916 in __GI_ppoll (fds=0x7fff63d30c40, nfds=2, > > timeout=, timeout@entry=0x0, sigmask=sigmask@entry=0x0) at > > ../sysdeps/unix/sysv/linux/ppoll.c:3

Re: bdrv_drained_begin deadlock with io-threads

2020-03-31 Thread Dietmar Maurer
> Inside exec.c, there is a race: > > --- > static bool prepare_mmio_access(MemoryRegion *mr) > { > bool unlocked = !qemu_mutex_iothread_locked(); > bool release_lock = false; > > if (unlocked && mr->global_locking) { > qemu_mutex_lock_iothread(); > -- > > IMHO, check

Re: bdrv_drained_begin deadlock with io-threads

2020-03-31 Thread Dietmar Maurer
Inside exec.c, there is a race: --- static bool prepare_mmio_access(MemoryRegion *mr) { bool unlocked = !qemu_mutex_iothread_locked(); bool release_lock = false; if (unlocked && mr->global_locking) { qemu_mutex_lock_iothread(); -- IMHO, checking for unlocked that way

bdrv_drained_begin deadlock with io-threads

2020-03-31 Thread Dietmar Maurer
I can see and reproduce this error with latest code from today. But I also see it on stable 4.1.1 (sometimes). I guess this is a similar problem as reported earlier: https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg07363.html To reproduce, you need a VM using virtio-scsi-single drive usi

AIO_WAIT_WHILE questions

2020-03-27 Thread Dietmar Maurer
Hi all, I have a question about AIO_WAIT_WHILE. The docs inside the code say: * The caller's thread must be the IOThread that owns @ctx or the main loop * thread (with @ctx acquired exactly once). I wonder if that "with @ctx acquired exactly once" is always required? I have done a quick test

Re: [PATCH v2 0/3] Fix some AIO context locking in jobs

2020-03-27 Thread Dietmar Maurer
> I *think* the second patch also fixes the hangs on backup abort that I and > Dietmar noticed in v1, but I'm not sure, they we're somewhat intermittent > before too. After more test, I am 100% sure the bug (or another one) is still there. Here is how to trigger: 1. use latest qemu sources from

Re: [PATCH v2 0/3] Fix some AIO context locking in jobs

2020-03-27 Thread Dietmar Maurer
Wait - maybe this was a bug in my test setup - I am unable to reproduce now.. @Stefan Reiter: Are you able to trigger this? > > I *think* the second patch also fixes the hangs on backup abort that I and > > Dietmar noticed in v1, but I'm not sure, they we're somewhat intermittent > > before too.

Re: [PATCH v2 0/3] Fix some AIO context locking in jobs

2020-03-26 Thread Dietmar Maurer
But I need to mention that it takes some time to reproduce this. This time I run/aborted about 500 backup jobs until it triggers. > > I *think* the second patch also fixes the hangs on backup abort that I and > > Dietmar noticed in v1, but I'm not sure, they we're somewhat intermittent > > before

Re: [PATCH v2 0/3] Fix some AIO context locking in jobs

2020-03-26 Thread Dietmar Maurer
> I *think* the second patch also fixes the hangs on backup abort that I and > Dietmar noticed in v1, but I'm not sure, they we're somewhat intermittent > before too. No, I still get this freeze: 0 0x7f0aa4866916 in __GI_ppoll (fds=0x7f0a12935c40, nfds=2, timeout=, timeout@entry=0x0, sigm

Re: backup transaction with io-thread core dumps

2020-03-26 Thread Dietmar Maurer
> > > As mentioned earlier, even a totally simple/normal backup job fails when > > > using io-threads and the VM is under load. It results in a total > > > VM freeze! > > > > > > > This is definitely a different issue. I'll take a look at it today. > > Thanks. Stefan found a way to avoid that bu

Re: backup transaction with io-thread core dumps

2020-03-26 Thread Dietmar Maurer
> > > > So the solution is to disable backups when using io-threads? > > > > > > > > > > I meant forbidding transactions with completion-mode == grouped. It > > > would be still possible running transactions (and thus, backups) with > > > completion-mode == individual, which is the default. > >

Re: backup transaction with io-thread core dumps

2020-03-25 Thread Dietmar Maurer
> On March 25, 2020 1:39 PM Sergio Lopez wrote: > > > On Wed, Mar 25, 2020 at 01:29:48PM +0100, Dietmar Maurer wrote: > > > As expected, if both BDS are running on the same IOThread (and thus, > > > the same AioContext), the problem is not reproducible. > >

Re: backup transaction with io-thread core dumps

2020-03-25 Thread Dietmar Maurer
> As expected, if both BDS are running on the same IOThread (and thus, > the same AioContext), the problem is not reproducible. > > In a general sense, we could say that completion modes other than > "individual" are not supported for a transaction that may access > different AioContexts. I don't

Re: [PATCH-for-5.0] qga-posix: Avoid crashing process when failing to allocate memory

2020-03-24 Thread Dietmar Maurer
but error_setg() also calls malloc, so this does not help at all? > On March 24, 2020 8:48 PM Philippe Mathieu-Daudé wrote: > > > Similarly to commit 807e2b6fce0 for Windows, kindly return a > QMP error message instead of crashing the whole process. > > Cc: qemu-sta...@nongnu.org > Buglink: h

Re: backup transaction with io-thread core dumps

2020-03-24 Thread Dietmar Maurer
example. Is somebody able to reproduce that? And ideas whats wrong here? > On March 24, 2020 2:47 PM Max Reitz wrote: > > > Hi Dietmar, > > I assume this is with master and has popped up only recently? > > Maybe it has something to do with the recent mutex patches by

Re: backup transaction with io-thread core dumps

2020-03-24 Thread Dietmar Maurer
> I assume this is with master and has popped up only recently? sure > Maybe it has something to do with the recent mutex patches by Stefan, so > I’m Cc-ing him. Thanks.

Re: backup transaction with io-thread core dumps

2020-03-24 Thread Dietmar Maurer
> What version of QEMU are you running? Sergio contributed a lot of AIO > context (and iothread) fixes this release; this looks (at a glance) to > be similar and related. I test with latest code from git commit 09a98dd988c715157c0b80af16fa5baa80101eed Merge: f1e748d279 1583794b9b Author: Peter

Re: backup transaction with io-thread core dumps

2020-03-24 Thread Dietmar Maurer
spoke too soon - the error is still there, sigh > This is fixed with this patch: > > https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg07249.html > > thanks! > > > On March 24, 2020 12:13 PM Dietmar Maurer wrote: > > > > > > I get a core d

Re: backup transaction with io-thread core dumps

2020-03-24 Thread Dietmar Maurer
This is fixed with this patch: https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg07249.html thanks! > On March 24, 2020 12:13 PM Dietmar Maurer wrote: > > > I get a core dump with backup transactions when using io-threads. > > To reproduce, create and start a VM

backup transaction with io-thread core dumps

2020-03-24 Thread Dietmar Maurer
I get a core dump with backup transactions when using io-threads. To reproduce, create and start a VM with: # qemu-img create disk1.raw 100M # qemu-img create disk2.raw 100M #./x86_64-softmmu/qemu-system-x86_64 -chardev 'socket,id=qmp,path=/var/run/qemu-test.qmp,server,nowait' -mon 'chardev=qmp

Re: aio-context question

2020-03-23 Thread Dietmar Maurer
> If it doesn't have any effect because it just does what will be done > later anyway, it can be removed, but that doesn't buy us much. I think that code is simply unnecessary (no effect). But it is quite hard to read and understand, so I think removing that code helps to simplify things. > If it

aio-context question

2020-03-19 Thread Dietmar Maurer
I just saw commit 30dd65f307b647eef8156c4a33bd007823ef85cb, and noticed that a similar pattern in drive_backup_prepare() and blockdev_backup_prepare(). The calls to bdrv_try_set_aio_context() seems useless, because we already do that later in backup_job_create/brdv_attach. But I am not 100%& sur

Re: backup_calculate_cluster_size does not consider source

2019-11-06 Thread Dietmar Maurer
> On 6 November 2019 14:17 Max Reitz wrote: > > > On 06.11.19 14:09, Dietmar Maurer wrote: > >> Let me elaborate: Yes, a cluster size generally means that it is most > >> “efficient” to access the storage at that size. But there’s a tradeoff. > >>

Re: backup_calculate_cluster_size does not consider source

2019-11-06 Thread Dietmar Maurer
> Let me elaborate: Yes, a cluster size generally means that it is most > “efficient” to access the storage at that size. But there’s a tradeoff. > At some point, reading the data takes sufficiently long that reading a > bit of metadata doesn’t matter anymore (usually, that is). Any network stor

Re: backup_calculate_cluster_size does not consider source

2019-11-06 Thread Dietmar Maurer
> And if it issues a smaller request, there is no way for a guest device > to tell it “OK, here’s your data, but note we have a whole 4 MB chunk > around it, maybe you’d like to take that as well...?” > > I understand wanting to increase the backup buffer size, but I don’t > quite understand why w

Re: backup_calculate_cluster_size does not consider source

2019-11-06 Thread Dietmar Maurer
> The thing is, it just seems unnecessary to me to take the source cluster > size into account in general. It seems weird that a medium only allows > 4 MB reads, because, well, guests aren’t going to take that into account. Maybe it is strange, but it is quite obvious that there is an optimal clu

backup_calculate_cluster_size does not consider source

2019-11-05 Thread Dietmar Maurer
Example: Backup from ceph disk (rbd_cache=false) to local disk: backup_calculate_cluster_size returns 64K (correct for my local .raw image) Then the backup job starts to read 64K blocks from ceph. But ceph always reads 4M block, so this is incredibly slow and produces way too much network traffi

Re: [PATCH v3] yield_until_fd_readable: make it work with any AioContect

2019-10-23 Thread Dietmar Maurer
> > +aio_set_fd_handler(ctx, fd, false, (IOHandler *)qemu_coroutine_enter, > > + NULL, NULL, qemu_coroutine_self()); > > This cast is unsafe. If qemu_coroutine_enter()'s prototype is changed > there will be no compiler warning that the prototypes are now > incompatible.

[PATCH v4] yield_until_fd_readable: make it work with any AioContect

2019-10-23 Thread Dietmar Maurer
Simply use qemu_get_current_aio_context(). Signed-off-by: Dietmar Maurer --- Changelog for v4: - avoid unsafe cast and keep fd_coroutine_enter() Changelog for v3: - use (IOHandler *) instead of ((void (*)(void *)) - coding style: fix max line length Changelog for v2: - use correct read

[PATCH v3] yield_until_fd_readable: make it work with any AioContect

2019-10-21 Thread Dietmar Maurer
Simply use qemu_get_current_aio_context(). Signed-off-by: Dietmar Maurer --- Changelog for v3: - use (IOHandler *) instead of ((void (*)(void *)) - coding style: fix max line length Changelog for v2: - use correct read handler in aio_set_fd_handler (instead of write handler) util/qemu

[PATCH v2] yield_until_fd_readable: make it work with any AioContect

2019-10-20 Thread Dietmar Maurer
Simply use qemu_get_current_aio_context(). Signed-off-by: Dietmar Maurer --- Changelog for v2: - use correct read handler in aio_set_fd_handler (instead of write handler) util/qemu-coroutine-io.c | 20 +++- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/util

  1   2   3   4   >