Re: [PATCH v2 04/10] linux-user: Add '-one-insn-per-tb' option equivalent to '-singlestep'

2023-04-03 Thread Warner Losh
On Mon, Apr 3, 2023 at 12:35 PM Richard Henderson < richard.hender...@linaro.org> wrote: > On 4/3/23 07:46, Peter Maydell wrote: > > The '-singlestep' option is confusing, because it doesn't actually > > have anything to do with single-stepping the CPU. What it does do > > is force TCG emulation

Re: [PATCH v2 05/10] bsd-user: Add '-one-insn-per-tb' option equivalent to '-singlestep'

2023-04-03 Thread Warner Losh
On Mon, Apr 3, 2023 at 8:46 AM Peter Maydell wrote: > The '-singlestep' option is confusing, because it doesn't actually > have anything to do with single-stepping the CPU. What it does do > is force TCG emulation to put one guest instruction in each TB, > which can be useful in some situations.

Re: [PATCH v2 09/10] qapi/run-state.json: Fix missing newline at end of file

2023-04-03 Thread Richard Henderson
On 4/3/23 07:46, Peter Maydell wrote: The run-state.json file is missing a trailing newline; add it. Signed-off-by: Peter Maydell --- Noticed this because my editor wanted to add the newline when I touched the file for the following patch... --- qapi/run-state.json | 2 +- 1 file changed, 1

Re: [PATCH v2 08/10] hmp: Report 'one-insn-per-tb', not 'single step mode', in 'info status' output

2023-04-03 Thread Richard Henderson
On 4/3/23 07:46, Peter Maydell wrote: The HMP 'info status' output includes "(single step mode)" when we are running with TCG one-insn-per-tb enabled. Change this text to "(one insn per TB)" to match the new command line option names. We don't need to have a deprecation/transition plan for

Re: [PATCH v2 07/10] hmp: Add 'one-insn-per-tb' command equivalent to 'singlestep'

2023-04-03 Thread Richard Henderson
On 4/3/23 07:46, Peter Maydell wrote: The 'singlestep' HMP command is confusing, because it doesn't actually have anything to do with single-stepping the CPU. What it does do is force TCG emulation to put one guest instruction in each TB, which can be useful in some situations. Create a new

Re: [PATCH v2 06/10] Document that -singlestep command line option is deprecated

2023-04-03 Thread Richard Henderson
On 4/3/23 07:46, Peter Maydell wrote: Document that the -singlestep command line option is now deprecated, as it is replaced by either the TCG accelerator property 'one-insn-per-tb' for system emulation or the new '-one-insn-per-tb' option for usermode emulation, and remove the only use of the

Re: [PATCH v2 05/10] bsd-user: Add '-one-insn-per-tb' option equivalent to '-singlestep'

2023-04-03 Thread Richard Henderson
On 4/3/23 07:46, Peter Maydell wrote: The '-singlestep' option is confusing, because it doesn't actually have anything to do with single-stepping the CPU. What it does do is force TCG emulation to put one guest instruction in each TB, which can be useful in some situations. Create a new command

Re: [PATCH v2 04/10] linux-user: Add '-one-insn-per-tb' option equivalent to '-singlestep'

2023-04-03 Thread Richard Henderson
On 4/3/23 07:46, Peter Maydell wrote: The '-singlestep' option is confusing, because it doesn't actually have anything to do with single-stepping the CPU. What it does do is force TCG emulation to put one guest instruction in each TB, which can be useful in some situations. Create a new command

Re: [PATCH v2 03/10] tcg: Use one-insn-per-tb accelerator property in curr_cflags()

2023-04-03 Thread Richard Henderson
On 4/3/23 07:46, Peter Maydell wrote: uint32_t curr_cflags(CPUState *cpu) { uint32_t cflags = cpu->tcg_cflags; +TCGState *tcgstate = TCG_STATE(current_accel()); As mentioned against the cover, this is a very hot path. We should try for something less expensive. Perhaps as

Re: [PATCH v2 02/10] softmmu: Don't use 'singlestep' global in QMP and HMP commands

2023-04-03 Thread Richard Henderson
On 4/3/23 07:46, Peter Maydell wrote: The HMP 'singlestep' command, the QMP 'query-status' command and the HMP 'info status' command (which is just wrapping the QMP command implementation) look at the 'singlestep' global variable. Make them access the new TCG accelerator 'one-insn-per-tb'

Re: [PATCH v2 01/10] make one-insn-per-tb an accel option

2023-04-03 Thread Richard Henderson
On 4/3/23 07:46, Peter Maydell wrote: This commit adds 'one-insn-per-tb' as a property on the TCG accelerator object, so you can enable it with -accel tcg,one-insn-per-tb=on It has the same behaviour as the existing '-singlestep' command line option. We use a different name because

Re: [PATCH v2 07/10] hmp: Add 'one-insn-per-tb' command equivalent to 'singlestep'

2023-04-03 Thread Dr. David Alan Gilbert
* Peter Maydell (peter.mayd...@linaro.org) wrote: > The 'singlestep' HMP command is confusing, because it doesn't > actually have anything to do with single-stepping the CPU. What it > does do is force TCG emulation to put one guest instruction in each > TB, which can be useful in some

[PATCH] Conf: Move validation of Graphics Transport and StorageNetwork Socket out of parser

2023-04-03 Thread skran
I have implemented your corrections successfully. Thank you. Signed-off-by: K Shiva --- src/conf/domain_conf.c | 42 +++- src/conf/domain_validate.c | 56 ++ src/conf/domain_validate.h | 7 + 3 files changed, 67 insertions(+),

Re: [PATCH v2 00/10] Deprecate/rename singlestep command line option, monitor interfaces

2023-04-03 Thread Richard Henderson
On 4/3/23 07:46, Peter Maydell wrote: * I have written patch 3 on the assumption that curr_cflags() is not such a hot codepath that we can't afford to have a QOM cast macro in it; the alternative would be to keep it using a global variable but make the global be restricted to

Re: (GSoC) Contributor Application

2023-04-03 Thread Shiva
On 4/3/23 19:40, Martin Kletzander wrote: On Mon, Apr 03, 2023 at 09:16:22AM +0200, Peter Krempa wrote: On Mon, Apr 03, 2023 at 12:24:09 +0530, Shiva wrote: Greetings libvirt developers! My name is Shiva and I am interested in contributing to the project titled *"Metadata support for all

[PATCH v2 09/10] qapi/run-state.json: Fix missing newline at end of file

2023-04-03 Thread Peter Maydell
The run-state.json file is missing a trailing newline; add it. Signed-off-by: Peter Maydell --- Noticed this because my editor wanted to add the newline when I touched the file for the following patch... --- qapi/run-state.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git

[PATCH v2 00/10] Deprecate/rename singlestep command line option, monitor interfaces

2023-04-03 Thread Peter Maydell
The command line option '-singlestep' and its HMP equivalent the 'singlestep' command are very confusingly named, because they have nothing to do with single-stepping the guest (either via the gdb stub or by emulation of guest CPU architectural debug facilities). What they actually do is put TCG

[PATCH v2 08/10] hmp: Report 'one-insn-per-tb', not 'single step mode', in 'info status' output

2023-04-03 Thread Peter Maydell
The HMP 'info status' output includes "(single step mode)" when we are running with TCG one-insn-per-tb enabled. Change this text to "(one insn per TB)" to match the new command line option names. We don't need to have a deprecation/transition plan for this, because we make no guarantees about

[PATCH v2 10/10] hmp: Deprecate 'singlestep' member of StatusInfo

2023-04-03 Thread Peter Maydell
The 'singlestep' member of StatusInfo has never done what the QMP documentation claims it does. What it actually reports is whether TCG is working in "one guest instruction per translation block" mode. Create a new 'one-insn-per-tb' member whose name matches what the field actually does and the

[PATCH v2 03/10] tcg: Use one-insn-per-tb accelerator property in curr_cflags()

2023-04-03 Thread Peter Maydell
Change curr_cflags() to look at the one-insn-per-tb accelerator property instead of the old singlestep global variable. Since this is the final remaining use of the global, we can delete it entirely. Signed-off-by: Peter Maydell --- This is the "clean" way of doing it. I dunno how much of a hot

[PATCH v2 05/10] bsd-user: Add '-one-insn-per-tb' option equivalent to '-singlestep'

2023-04-03 Thread Peter Maydell
The '-singlestep' option is confusing, because it doesn't actually have anything to do with single-stepping the CPU. What it does do is force TCG emulation to put one guest instruction in each TB, which can be useful in some situations. Create a new command line argument -one-insn-per-tb, so we

[PATCH v2 07/10] hmp: Add 'one-insn-per-tb' command equivalent to 'singlestep'

2023-04-03 Thread Peter Maydell
The 'singlestep' HMP command is confusing, because it doesn't actually have anything to do with single-stepping the CPU. What it does do is force TCG emulation to put one guest instruction in each TB, which can be useful in some situations. Create a new HMP command 'one-insn-per-tb', so we can

[PATCH v2 04/10] linux-user: Add '-one-insn-per-tb' option equivalent to '-singlestep'

2023-04-03 Thread Peter Maydell
The '-singlestep' option is confusing, because it doesn't actually have anything to do with single-stepping the CPU. What it does do is force TCG emulation to put one guest instruction in each TB, which can be useful in some situations. Create a new command line argument -one-insn-per-tb, so we

[PATCH v2 06/10] Document that -singlestep command line option is deprecated

2023-04-03 Thread Peter Maydell
Document that the -singlestep command line option is now deprecated, as it is replaced by either the TCG accelerator property 'one-insn-per-tb' for system emulation or the new '-one-insn-per-tb' option for usermode emulation, and remove the only use of the deprecated syntax from a README.

[PATCH v2 02/10] softmmu: Don't use 'singlestep' global in QMP and HMP commands

2023-04-03 Thread Peter Maydell
The HMP 'singlestep' command, the QMP 'query-status' command and the HMP 'info status' command (which is just wrapping the QMP command implementation) look at the 'singlestep' global variable. Make them access the new TCG accelerator 'one-insn-per-tb' property instead. This leaves the HMP and QMP

[PATCH v2 01/10] make one-insn-per-tb an accel option

2023-04-03 Thread Peter Maydell
This commit adds 'one-insn-per-tb' as a property on the TCG accelerator object, so you can enable it with -accel tcg,one-insn-per-tb=on It has the same behaviour as the existing '-singlestep' command line option. We use a different name because 'singlestep' has always been a confusing choice,

Re: (GSoC) Contributor Application

2023-04-03 Thread Martin Kletzander
On Mon, Apr 03, 2023 at 09:16:22AM +0200, Peter Krempa wrote: On Mon, Apr 03, 2023 at 12:24:09 +0530, Shiva wrote: Greetings libvirt developers! My name is Shiva and I am interested in contributing to the project titled *"Metadata support for all object schemas". * My apologies for the delay

Re: [libvirt PATCH 05/20] qemu_snapshot: introduce qemuSnapshotDomainDefUpdateDisk

2023-04-03 Thread Peter Krempa
On Mon, Mar 13, 2023 at 16:42:06 +0100, Pavel Hrdina wrote: > Extract the code that updates disks in domain definition while creating > external snapshots. We will use it later in the external snapshot revert > code. > > Signed-off-by: Pavel Hrdina > --- > src/qemu/qemu_snapshot.c | 63

Re: [libvirt PATCH 04/20] snapshot_conf: introduce metadata element

2023-04-03 Thread Peter Krempa
On Mon, Mar 13, 2023 at 16:42:05 +0100, Pavel Hrdina wrote: > This new element will hold the new disk overlay created when reverting > to non-leaf snapshot in order to remember the files libvirt created. > > Signed-off-by: Pavel Hrdina > --- > src/conf/schemas/domainsnapshot.rng | 7 +++ >

Re: [libvirt PATCH 03/20] snapshot_conf: use alternate domain definition in virDomainSnapshotDefAssignExternalNames

2023-04-03 Thread Peter Krempa
On Mon, Mar 13, 2023 at 16:42:04 +0100, Pavel Hrdina wrote: > Commit introduced new > argument for virDomainSnapshotAlignDisks() that allows passing alternate > domain definition in case the snapshot parent.dom is NULL. > > In case of redefining snapshot it will not hit the part of code that >

Re: [libvirt PATCH 02/20] snapshot_conf: export virDomainSnapshotDiskDefClear

2023-04-03 Thread Peter Krempa
On Mon, Mar 13, 2023 at 16:42:03 +0100, Pavel Hrdina wrote: > We will need to call this function from qemu_snapshot when introducing > external snapshot revert support. > > Signed-off-by: Pavel Hrdina > --- > src/conf/snapshot_conf.c | 2 +- > src/conf/snapshot_conf.h | 3 +++ >

Re: [libvirt PATCH 01/20] libvirt_private: list virDomainMomentDefPostParse

2023-04-03 Thread Peter Krempa
On Mon, Mar 13, 2023 at 16:42:02 +0100, Pavel Hrdina wrote: > We will need to call this function from qemu_snapshot when introducing > external snapshot revert support. > > Signed-off-by: Pavel Hrdina > --- > src/libvirt_private.syms | 4 > 1 file changed, 4 insertions(+) Reviewed-by:

[PATCHv2] storage_file_probe: change maximum len value in vmdk4GetBackingStore

2023-04-03 Thread Anastasia Belova
desc length should be always less than VIR_STORAGE_MAX_HEADER. If len = VIR_STORAGE_MAX_HEADER, desc may be out of bounds. Fixes: 348b4e254b ("storage: always probe type with buffer") Signed-off-by: Anastasia Belova --- v2: change Fixes: line src/storage_file/storage_file_probe.c | 4 ++-- 1

Re: [PATCH 1/1] Partial fixes for Issue #7 titled 'Move validation checks out of domain XML parsing'

2023-04-03 Thread Michal Prívozník
On 4/3/23 11:34, Peter Krempa wrote: > In the summary line please shortly describe the change itself e.g.: > > conf: Move validation of graphics transport out of parser > > Any further description can be in the body, including mentioning issues > and other less-relevant information. > >

Re: [PATCH 1/1] Partial fixes for Issue #7 titled 'Move validation checks out of domain XML parsing'

2023-04-03 Thread Peter Krempa
In the summary line please shortly describe the change itself e.g.: conf: Move validation of graphics transport out of parser Any further description can be in the body, including mentioning issues and other less-relevant information. Additionally in my reply to your application where I've

[PATCH (pushed)] domaincapstest: Skip unknown variants instead of the default variant

2023-04-03 Thread Peter Krempa
Fix the logic selecting when to run the tests to skip unknown variants rather than the default variant. Fixes: 738c5bae888 Signed-off-by: Peter Krempa --- Pushed as trivial. I've messed up when applying review feedback. tests/domaincapstest.c | 2 +- 1 file changed, 1 insertion(+), 1

[PATCH 1/1] Partial fixes for Issue #7 titled 'Move validation checks out of domain XML parsing'

2023-04-03 Thread skran
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3ab7cd2666..2a94566047 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5840,14 +5840,14 @@ virDomainStorageNetworkParseHost(xmlNodePtr hostnode, host->socket = virXMLPropString(hostnode, "socket");

Re: (GSoC) Contributor Application

2023-04-03 Thread Peter Krempa
On Mon, Apr 03, 2023 at 12:24:09 +0530, Shiva wrote: > Greetings libvirt developers! > > My name is Shiva and I am interested in contributing to the project titled > *"Metadata support for all object schemas". > * > > My apologies for the delay in contact as this is my first time at GSoC and I >

(GSoC) Contributor Application

2023-04-03 Thread Shiva
Greetings libvirt developers! My name is Shiva and I am interested in contributing to the project titled *"Metadata support for all object schemas". * My apologies for the delay in contact as this is my first time at GSoC and I have missed the dates. Is this the correct mailing list for

Re: [libvirt PATCH] Remove trailing spaces from translatable strings

2023-04-03 Thread Peter Krempa
On Sun, Apr 02, 2023 at 23:14:29 +0200, Ján Tomko wrote: > Signed-off-by: Ján Tomko > --- Reviewed-by: Peter Krempa