Re: [kdevops PATCH] qemu-build: Add a configuration option for enabling qemu debug

Re: [kdevops PATCH] qemu-build: Add a configuration option for enabling qemu debug

[Luis Chamberlain]

On Thu, Aug 24, 2023 at 02:08:05PM -0700, fan.ni@gmx.us wrote:
> From: Fan Ni <fan.ni@gmx.us>
> 
> Currently, qemu will always be compiled without enabling debug, however,
> enabling qemu debug is important for debug purpose. A new Kconfig
> option is added to allow user to enable debug when configuring qemu. By
> default, it is disabled.
> 
> Signed-off-by: Fan Ni <fan.ni@samsung.com>
> ---

Thanks Fan, I've Cc'd kdevops@lists.linux.dev. Please use that
mailing list for future patches. My review below.

>  Makefile.build_qemu                          | 4 ++++
>  playbooks/roles/build_qemu/defaults/main.yml | 1 +
>  playbooks/roles/build_qemu/tasks/main.yml    | 2 +-
>  vagrant/Kconfig                              | 7 +++++++
>  4 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.build_qemu b/Makefile.build_qemu
> index 7addd2f0..3f50f949 100644
> --- a/Makefile.build_qemu
> +++ b/Makefile.build_qemu
> @@ -19,6 +19,10 @@ ifeq (y,$(CONFIG_TARGET_ARCH_PPC64LE))
>  QEMU_BUILD_SETUP_ARGS += qemu_target="ppc64-softmmu"
>  endif
> 
> +ifeq (y,$(CONFIG_QEMU_WITH_DEBUG_ENABLED))
> +QEMU_BUILD_SETUP_ARGS += qemu_debug="--enable-debug"
> +endif

This is a build debug thing.

> +
>  qemu: $(KDEVOPS_EXTRA_VARS)
>  	@$(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \
>  		--inventory localhost, \
> diff --git a/playbooks/roles/build_qemu/defaults/main.yml b/playbooks/roles/build_qemu/defaults/main.yml
> index 006d669c..c8e12759 100644
> --- a/playbooks/roles/build_qemu/defaults/main.yml
> +++ b/playbooks/roles/build_qemu/defaults/main.yml
> @@ -12,3 +12,4 @@ qemu_git: "https://github.com/qemu/qemu.git"
>  qemu_version: "v7.2.0-rc4"
>  qemu_build_dir: "{{ qemu_data }}/build"
>  qemu_target: "x86_64-softmmu"
> +qemu_debug: False

this should be qemu_build_debug

> diff --git a/playbooks/roles/build_qemu/tasks/main.yml b/playbooks/roles/build_qemu/tasks/main.yml
> index 448714b2..a4b4bba9 100644
> --- a/playbooks/roles/build_qemu/tasks/main.yml
> +++ b/playbooks/roles/build_qemu/tasks/main.yml
> @@ -74,7 +74,7 @@
>      - build_qemu_now|bool
> 
>  - name: Run configure for QEMU
> -  command: "./configure --target-list={{ qemu_target }} --disable-download"
> +  command: "./configure --target-list={{ qemu_target }} --disable-download {{qemu_debug}}"
>    tags: [ 'qemu', 'configure' ]
>    args:
>      chdir: "{{ qemu_data }}"
> diff --git a/vagrant/Kconfig b/vagrant/Kconfig
> index 68b14a0b..68986295 100644
> --- a/vagrant/Kconfig
> +++ b/vagrant/Kconfig
> @@ -266,6 +266,13 @@ endif # !QEMU_BUILD
> 
>  if QEMU_BUILD
> 
> +config QEMU_WITH_DEBUG_ENABLED

And so this should be QEMU_BUILD_WITH_DEBUG_ENABLED

> +	bool "enable debug for qemu"
> +	default n
> +	help
> +	  Enable this will add "--enable-debug" option when configuring qemu. This
> +	  is useful for debugging and testing qemu code.

This should clarify when it's useful. Is it only a build thing?

Are there options for runtime debug? Is that also useful to you?
Could those be added dynamically, so that perhaps one could enable them
on the fly ? If so check DYNAMIC_RUNTIME_VARS on
workflows/fstests/Makefile for an exmaple of how one can for example for
example add specific arguments at the end of a make target to extend
variables dynamically. Likewise we have the use of ANSIBLE_VERBOSE which
might help for run time things too.

In so far as dynamic run time though, running qemu though is done
through vagrant and that just sets up and uses libvirt, but I suppose
the above might still be useful somehow if one perhaps wanted to somehow
extend debug dynamic parameters somewhere.

  Luis

Re: [kdevops PATCH] qemu-build: Add a configuration option for enabling qemu debug

[Fan Ni]

On Thu, Aug 24, 2023 at 02:29:04PM -0700, Luis Chamberlain wrote:
> On Thu, Aug 24, 2023 at 02:08:05PM -0700, fan.ni@gmx.us wrote:
> > From: Fan Ni <fan.ni@gmx.us>
> >
> > Currently, qemu will always be compiled without enabling debug, however,
> > enabling qemu debug is important for debug purpose. A new Kconfig
> > option is added to allow user to enable debug when configuring qemu. By
> > default, it is disabled.
> >
> > Signed-off-by: Fan Ni <fan.ni@samsung.com>
> > ---
>
> Thanks Fan, I've Cc'd kdevops@lists.linux.dev. Please use that
> mailing list for future patches. My review below.
>
Thaks Luis for the review. See below.

> >  Makefile.build_qemu                          | 4 ++++
> >  playbooks/roles/build_qemu/defaults/main.yml | 1 +
> >  playbooks/roles/build_qemu/tasks/main.yml    | 2 +-
> >  vagrant/Kconfig                              | 7 +++++++
> >  4 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile.build_qemu b/Makefile.build_qemu
> > index 7addd2f0..3f50f949 100644
> > --- a/Makefile.build_qemu
> > +++ b/Makefile.build_qemu
> > @@ -19,6 +19,10 @@ ifeq (y,$(CONFIG_TARGET_ARCH_PPC64LE))
> >  QEMU_BUILD_SETUP_ARGS += qemu_target="ppc64-softmmu"
> >  endif
> >
> > +ifeq (y,$(CONFIG_QEMU_WITH_DEBUG_ENABLED))
> > +QEMU_BUILD_SETUP_ARGS += qemu_debug="--enable-debug"
> > +endif
>
> This is a build debug thing.
Will fix in the next version.
>
> > +
> >  qemu: $(KDEVOPS_EXTRA_VARS)
> >  	@$(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \
> >  		--inventory localhost, \
> > diff --git a/playbooks/roles/build_qemu/defaults/main.yml b/playbooks/roles/build_qemu/defaults/main.yml
> > index 006d669c..c8e12759 100644
> > --- a/playbooks/roles/build_qemu/defaults/main.yml
> > +++ b/playbooks/roles/build_qemu/defaults/main.yml
> > @@ -12,3 +12,4 @@ qemu_git: "https://github.com/qemu/qemu.git"
> >  qemu_version: "v7.2.0-rc4"
> >  qemu_build_dir: "{{ qemu_data }}/build"
> >  qemu_target: "x86_64-softmmu"
> > +qemu_debug: False
>
> this should be qemu_build_debug
Will fix in the next version.
>
> > diff --git a/playbooks/roles/build_qemu/tasks/main.yml b/playbooks/roles/build_qemu/tasks/main.yml
> > index 448714b2..a4b4bba9 100644
> > --- a/playbooks/roles/build_qemu/tasks/main.yml
> > +++ b/playbooks/roles/build_qemu/tasks/main.yml
> > @@ -74,7 +74,7 @@
> >      - build_qemu_now|bool
> >
> >  - name: Run configure for QEMU
> > -  command: "./configure --target-list={{ qemu_target }} --disable-download"
> > +  command: "./configure --target-list={{ qemu_target }} --disable-download {{qemu_debug}}"
> >    tags: [ 'qemu', 'configure' ]
> >    args:
> >      chdir: "{{ qemu_data }}"
> > diff --git a/vagrant/Kconfig b/vagrant/Kconfig
> > index 68b14a0b..68986295 100644
> > --- a/vagrant/Kconfig
> > +++ b/vagrant/Kconfig
> > @@ -266,6 +266,13 @@ endif # !QEMU_BUILD
> >
> >  if QEMU_BUILD
> >
> > +config QEMU_WITH_DEBUG_ENABLED
>
> And so this should be QEMU_BUILD_WITH_DEBUG_ENABLED
Will fix in the next version.
>
> > +	bool "enable debug for qemu"
> > +	default n
> > +	help
> > +	  Enable this will add "--enable-debug" option when configuring qemu. This
> > +	  is useful for debugging and testing qemu code.
>
> This should clarify when it's useful. Is it only a build thing?
>
> Are there options for runtime debug? Is that also useful to you?
> Could those be added dynamically, so that perhaps one could enable them
> on the fly ? If so check DYNAMIC_RUNTIME_VARS on
> workflows/fstests/Makefile for an exmaple of how one can for example for
> example add specific arguments at the end of a make target to extend
> variables dynamically. Likewise we have the use of ANSIBLE_VERBOSE which
> might help for run time things too.
>
> In so far as dynamic run time though, running qemu though is done
> through vagrant and that just sets up and uses libvirt, but I suppose
> the above might still be useful somehow if one perhaps wanted to somehow
> extend debug dynamic parameters somewhere.
>
>   Luis

Yes, this is a build thing. Currently, qemu is built without debug,
which means we cannot use gdb for debugging the qemu code.
Once its compiled and installed, we cannot dynamically enable it.
I just use the change to debug qemu and locate a bug in qemu.

Fan