Re: [PATCH 2/3] qga: Add optional stream-output argument to guest-exec

l look into it. Daniel > > > On Mon, Sep 18, 2023 at 8:17 PM Daniel Xu wrote: >> Hi Daniel, >> >> On Mon, Sep 18, 2023 at 04:14:47PM +0100, Daniel P. Berrangé wrote: >> > On Mon, Sep 18, 2023 at 04:54:22AM -0600, Daniel Xu wrote: >> > > Currently, c

[PATCH 1/2] qga: Fix memory leak when output stream is unused

oesn't write to the stream. Fix by making free() unconditional. Reviewed-by: Konstantin Kostiuk Signed-off-by: Daniel Xu --- qga/commands.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qga/commands.c b/qga/commands.c index 09c683e263..ce172edd2d 100644 --- a/qga/comm

[PATCH 0/2] Small fixes for qga

These are two small fixes that fell out of [0]. Since we are not moving forward with that patchset, I thought it would be good to at least send the fixes that came out of it. See commits for more details. [0]: https://lore.kernel.org/qemu-devel/cover.1695034158.git@dxuuu.xyz/ Daniel Xu (2

[PATCH 2/2] qapi: qga: Clarify when out-data and err-data are populated

If output is being captured for a guest-exec invocation, the out-data and err-data fields of guest-exec-status are only populated after the process is reaped. This is somewhat counter intuitive and too late to change. Thus, it would be good to document the behavior. Signed-off-by: Daniel Xu

Re: [PATCH 2/3] qga: Add optional stream-output argument to guest-exec

Hi Daniel, On Mon, Sep 18, 2023 at 04:14:47PM +0100, Daniel P. Berrangé wrote: > On Mon, Sep 18, 2023 at 04:54:22AM -0600, Daniel Xu wrote: > > Currently, commands run through guest-exec are "silent" until they > > finish running. This is fine for short lived c

Re: [PATCH 2/3] qga: Add optional stream-output argument to guest-exec

On Mon, Sep 18, 2023 at 05:05:03PM +0200, Markus Armbruster wrote: > Daniel Xu writes: > > > Currently, commands run through guest-exec are "silent" until they > > finish running. This is fine for short lived commands. But for commands > > that take a wh

[PATCH 1/3] qga: Fix memory leak when output stream is unused

oesn't write to the stream. Fix by making free() unconditional. Signed-off-by: Daniel Xu --- qga/commands.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qga/commands.c b/qga/commands.c index 09c683e263..ce172edd2d 100644 --- a/qga/commands.c +++ b/qga/commands.c @@ -20

[PATCH 2/3] qga: Add optional stream-output argument to guest-exec

do. And it is a more reliable design than having QGA internally keep track of position -- for the cases that the caller "loses" a response. Signed-off-by: Daniel Xu --- qga/commands.c | 12 qga/qapi-schema.json | 7 ++- 2 files changed, 18 insertions(+), 1 deletion(-

[PATCH 0/3] qga: Add optional stream-output argument to guest-exec

red output. This allows downstream applications to be able to relay "status" to the end user. I also uncovered a latent memory leak bug with the added unit test. The fix is in commit 1. Daniel Xu (3): qga: Fix memory leak when output stream is unused qga: Add optional stream-output argument

[PATCH 3/3] qga: test: Add test for guest-exec stream-output

Add a test that simulates a long running process (by using a named pipe to synchronize). This test ensures that full output is returned with each call to guest-exec-status. Signed-off-by: Daniel Xu --- tests/unit/test-qga.c | 77 +++ 1 file changed, 77

Re: [PATCH v6 2/3] qga: Add `merged` variant to GuestExecCaptureOutputMode

Hi Konstantin, On Mon, Apr 3, 2023, at 8:56 AM, Konstantin Kostiuk wrote: > Hi Daniel, > > I will merge this series after the 8.0 release. > > Best Regards, > Konstantin Kostiuk. > Sorry to bug again, but 8.0 is out now right? Does this need a rebase or is it good to go? Thanks, Daniel [...]

Re: [PATCH v6 2/3] qga: Add `merged` variant to GuestExecCaptureOutputMode

Hi Daniel, On Thu, Mar 23, 2023, at 3:26 AM, Daniel P. Berrangé wrote: > On Wed, Mar 22, 2023 at 06:19:27PM -0600, Daniel Xu wrote: >> Currently, any captured output (via `capture-output`) is segregated into >> separate GuestExecStatus fields (`out-data` and `err-dat

[PATCH v6 3/3] qga: test: Add tests for `merged` flag

This commit adds a test to ensure `merged` functions as expected. We also add a negative test to ensure we haven't regressed previous functionality. Reviewed-by: Daniel P. Berrangé Signed-off-by: Daniel Xu --- tests/unit/test-qga.c | 158 +- 1 file

[PATCH v6 2/3] qga: Add `merged` variant to GuestExecCaptureOutputMode

a pristine view of the original command output. Signed-off-by: Daniel Xu --- qga/commands.c | 25 +++-- qga/qapi-schema.json | 5 - 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/qga/commands.c b/qga/commands.c index 01f68b45ab..09c683e263 100644

[PATCH v6 1/3] qga: Refactor guest-exec capture-output to take enum

mode instead while preserving backwards compatibility. Suggested-by: Daniel P. Berrangé Reviewed-by: Daniel P. Berrangé Signed-off-by: Daniel Xu --- qga/commands.c | 37 ++--- qga/qapi-schema.json | 33 - 2 files changed, 66

[PATCH v6 0/3] qga: Support merging output streams in guest-exec

guests * Fix a UAF in tests Daniel Xu (3): qga: Refactor guest-exec capture-output to take enum qga: Add `merged` variant to GuestExecCaptureOutputMode qga: test: Add tests for `merged` flag qga/commands.c| 62 +++-- qga/qapi-schema.json | 36 +- tests/uni

Re: [PATCH v5 2/3] qga: Add `merged` variant to GuestExecCaptureOutputMode

Hi Daniel, Sorry about the delay -- was out of town the past week. On Fri, Mar 10, 2023, at 2:24 AM, Daniel P. Berrangé wrote: > On Thu, Mar 09, 2023 at 03:40:57PM -0700, Daniel Xu wrote: >> Currently, any captured output (via `capture-output`) is segregated into >> separate

[PATCH v5 2/3] qga: Add `merged` variant to GuestExecCaptureOutputMode

a pristine view of the original command output. Signed-off-by: Daniel Xu --- qga/commands.c | 31 +-- qga/qapi-schema.json | 4 +++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/qga/commands.c b/qga/commands.c index 01f68b45ab..c347d434ed

[PATCH v5 0/3] qga: Support merging output streams in guest-exec

output` on windows guests * Add FIXMEs for when glib is updated * Fix memory leaks in qemu-keymap Changes from v1: * Drop invalid test fix * Do not support `merge-output` on windows guests * Fix a UAF in tests Daniel Xu (3): qga: Refactor guest-exec capture-output to take enum qga: Add `merged` v

[PATCH v5 1/3] qga: Refactor guest-exec capture-output to take enum

mode instead while preserving backwards compatibility. Suggested-by: Daniel P. Berrangé Signed-off-by: Daniel Xu --- qga/commands.c | 37 ++--- qga/qapi-schema.json | 33 - 2 files changed, 66 insertions(+), 4 deletions

[PATCH v5 3/3] qga: test: Add tests for `merged` flag

This commit adds a test to ensure `merged` functions as expected. We also add a negative test to ensure we haven't regressed previous functionality. Signed-off-by: Daniel Xu --- tests/unit/test-qga.c | 158 +- 1 file changed, 141 insertions(+), 17

Re: [PATCH v4 1/3] qga: Refactor guest-exec capture-output to take enum

Hi Daniel, On Thu, Mar 09, 2023 at 09:29:34AM +, Daniel P. Berrangé wrote: > On Wed, Mar 08, 2023 at 03:49:39PM -0700, Daniel Xu wrote: > > Previously capture-output was an optional boolean flag that either > > captured all output or captured none. While this is OK in most case

[PATCH v4 3/3] qga: test: Add tests for `all-merge` flag

This commit adds a test to ensure `all-merge` functions as expected. We also add a negative test to ensure we haven't regressed previous functionality. Signed-off-by: Daniel Xu --- tests/unit/test-qga.c | 158 +- 1 file changed, 141 insertions(+), 17

[PATCH v4 1/3] qga: Refactor guest-exec capture-output to take enum

mode instead while preserving backwards compatibility. Suggested-by: Daniel P. Berrangé Signed-off-by: Daniel Xu --- qga/commands.c | 37 ++--- qga/qapi-schema.json | 32 +++- 2 files changed, 65 insertions(+), 4 deletions

[PATCH v4 2/3] qga: Add `all-merge` variant to GuestExecCaptureOutputMode

a pristine view of the original command output. Signed-off-by: Daniel Xu --- qga/commands.c | 31 +-- qga/qapi-schema.json | 4 +++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/qga/commands.c b/qga/commands.c index 5504fc5b8c..7d458d0aca

[PATCH v4 0/3] qga: Support merging output streams in guest-exec

-keymap Changes from v1: * Drop invalid test fix * Do not support `merge-output` on windows guests * Fix a UAF in tests Daniel Xu (3): qga: Refactor guest-exec capture-output to take enum qga: Add `all-merge` variant to GuestExecCaptureOutputMode qga: test: Add tests for `all-merge` flag

[PATCH 1/2] crypto/luks: Initialize stack variable to silence warning

by checking `splitkey` nullness. But the compiler is not smart enough to realize that. Fix warning by initializing the variable. Signed-off-by: Daniel Xu Reviewed-by: Daniel P. Berrangé --- crypto/block-luks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/block-luks.c b

[PATCH 2/2] qemu-keymap: Fix memory leaks

0x7f32635db73e (/usr/lib/libxkbcommon.so.0+0x2273e) Fix leaks by correctly decrementing refcounts on xkb structs. Signed-off-by: Daniel Xu Reviewed-by: Marc-André Lureau --- qemu-keymap.c | 5 + 1 file changed, 5 insertions(+) diff --git a/qemu-keymap.c b/qemu-keymap.c index 229866e004

[PATCH 0/2] build: Fix --enable-sanitizers build errors

I hit two small build errors when building with --enable-sanitizers. These two fixes are split out from a previous series [0]. [0]: https://lore.kernel.org/qemu-devel/cover.1677617035.git@dxuuu.xyz/ Daniel Xu (2): crypto/luks: Initialize stack variable to silence warning qemu-keymap: Fix

Re: [PATCH v3 3/4] qga: Add optional `merge-output` flag to guest-exec qapi

Hi Daniel, On Wed, Mar 01, 2023 at 09:03:53AM +, Daniel P. Berrangé wrote: > On Tue, Feb 28, 2023 at 01:48:03PM -0700, Daniel Xu wrote: > > Currently, the captured output (via `capture-output`) is segregated into > > separate GuestExecStatus fields (`out-data` and `err-dat

[PATCH v3 0/4] qga: Add optional `merge-output` flag to guest-exec QAPI

-output` on windows guests * Add FIXMEs for when glib is updated * Fix memory leaks in qemu-keymap Changes from v1: * Drop invalid test fix * Do not support `merge-output` on windows guests * Fix a UAF in tests Daniel Xu (4): crypto/luks: Initialize stack variable to silence warning qemu

[PATCH v3 3/4] qga: Add optional `merge-output` flag to guest-exec qapi

of the original command output. Signed-off-by: Daniel Xu --- qga/commands.c | 28 ++-- qga/qapi-schema.json | 6 +- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/qga/commands.c b/qga/commands.c index 172826f8f8..cfce13d034 100644 --- a/qga

[PATCH v3 1/4] crypto/luks: Initialize stack variable to silence warning

by checking `splitkey` nullness. But the compiler is not smart enough to realize that. Fix warning by initializing the variable. Signed-off-by: Daniel Xu --- crypto/block-luks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index

[PATCH v3 2/4] qemu-keymap: Fix memory leaks

0x7f32635db73e (/usr/lib/libxkbcommon.so.0+0x2273e) Fix leaks by correctly decrementing refcounts on xkb structs. Signed-off-by: Daniel Xu --- qemu-keymap.c | 5 + 1 file changed, 5 insertions(+) diff --git a/qemu-keymap.c b/qemu-keymap.c index 229866e004..ed8cee3467 100644 --- a/qemu

[PATCH v3 4/4] qga: test: Add tests for `merge-output` flag

This commit adds a test to ensure `merge-output` functions as expected. We also add a negative test to ensure we haven't regressed previous functionality. Signed-off-by: Daniel Xu --- tests/unit/test-qga.c | 157 +- 1 file changed, 140 insertions(+), 17

Re: [PATCH v2 2/3] qga: Add optional `merge-output` flag to guest-exec qapi

023 at 10:51 PM Daniel Xu wrote: > > > > Currently, the captured output (via `capture-output`) is segregated into > > separate GuestExecStatus fields (`out-data` and `err-data`). This means > > that downstream consumers have no way to reassemble the captured data

[PATCH v2 3/3] qga: test: Add tests for `merge-output` flag

This commit adds a test to ensure `merge-output` functions as expected. We also add a negative test to ensure we haven't regressed previous functionality. Signed-off-by: Daniel Xu --- tests/unit/test-qga.c | 133 -- 1 file changed, 116 insertions(+), 17

[PATCH v2 1/3] crypto/luks: Initialize stack variable to silence warning

by checking `splitkey` nullness. But the compiler is not smart enough to realize that. Fix warning by initializing the variable. Signed-off-by: Daniel Xu --- crypto/block-luks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/block-luks.c b/crypto/block-luks.c index

[PATCH v2 2/3] qga: Add optional `merge-output` flag to guest-exec qapi

of the original command output. Signed-off-by: Daniel Xu --- qga/commands.c | 14 -- qga/qapi-schema.json | 6 +- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/qga/commands.c b/qga/commands.c index 172826f8f8..e13a8e86df 100644 --- a/qga/commands.c +++ b

[PATCH v2 0/3] qga: Add optional `merge-output` flag to guest-exec QAPI

fix * Do not support `merge-output` on windows guests * Fix a UAF in tests Daniel Xu (3): crypto/luks: Initialize stack variable to silence warning qga: Add optional `merge-output` flag to guest-exec qapi qga: test: Add tests for `merge-output` flag crypto/block-luks.c | 2 +- qga

Re: [PATCH 3/3] qga: test: Add tests for `merge-output` flag

Hi, On Mon, Feb 27, 2023, at 1:40 AM, Marc-André Lureau wrote: > Hi > > On Fri, Feb 24, 2023 at 8:31 AM Daniel Xu wrote: >> >> This commit adds a test to ensure `merge-output` functions as expected. >> We also add a negative test to ensure we haven't regresse

Re: [PATCH 2/3] qga: Add optional `merge-output` flag to guest-exec qapi

Hi, On Mon, Feb 27, 2023, at 1:22 AM, Marc-André Lureau wrote: > Hi > > On Fri, Feb 24, 2023 at 8:31 AM Daniel Xu wrote: >> >> Currently, the captured output (via `capture-output`) is segregated into >> separate GuestExecStatus fields (`out-data` and `err-data`). Th

Re: [PATCH 1/3] qga: test: Use absolute path to test data

Hi Marc-André, Thanks for reviewing the series. On Mon, Feb 27, 2023, at 1:16 AM, Marc-André Lureau wrote: > Hi > > On Fri, Feb 24, 2023 at 8:31 AM Daniel Xu wrote: >> >> It looks like qga's working directory is in a tempdir. So the relative >> path that the

Re: [PATCH 0/3] qga: Add optional `merge-output` flag to guest-exec QAPI

On Thu, Feb 23, 2023, at 7:05 PM, Daniel Xu wrote: > Currently, the captured output (via `capture-output`) is segregated into > separate GuestExecStatus fields (`out-data` and `err-data`). This means > that downstream consumers have no way to reassemble the captured data > back into

[PATCH 3/3] qga: test: Add tests for `merge-output` flag

This commit adds a test to ensure `merge-output` functions as expected. We also add a negative test to ensure we haven't regressed previous functionality. Signed-off-by: Daniel Xu --- tests/unit/test-qga.c | 128 -- 1 file changed, 111 insertions(+), 17

[PATCH 2/3] qga: Add optional `merge-output` flag to guest-exec qapi

of the original command output. Signed-off-by: Daniel Xu --- qga/commands.c | 13 - qga/qapi-schema.json | 6 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/qga/commands.c b/qga/commands.c index 360077364e..14b970e768 100644 --- a/qga/commands.c +++ b

[PATCH 0/3] qga: Add optional `merge-output` flag to guest-exec QAPI

(ie. read only) CLI tools. Such tools may deliberately interleave stdout and stderr for visual effect. If segregated, the output becomes harder to visually understand. This patchset adds support for merging stderr and stdout output streams via a new QAPI flag. Daniel Xu (3): qga: test: Use

[PATCH 1/3] qga: test: Use absolute path to test data

that helper was only introduced in glib 2.58 and the current GLIB_VERSION_MAX_ALLOWED is pinned to 2.56. Signed-off-by: Daniel Xu --- tests/unit/test-qga.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c index b4e0a14573