| Message ID | 0f5949d8ece522e30f990d25981f79965bf05bbf.1728232526.git.balaton@eik.bme.hu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Separate memory access logs from guest_errors | expand |
On Sun, 6 Oct 2024 at 17:49, BALATON Zoltan <balaton@eik.bme.hu> wrote: > > Rename guest_errors to guest_error to match the log constant I don't think this is a good reason to change the user-facing behaviour. Also, I don't think the existing names are so bad: -d guest_errors this is plural because we are asking to log all guest errors qemu_log_mask(LOG_GUEST_ERROR, ...) this is singular because we are logging a single error here. If we do want to change things for consistency, we should decide on what the user-facing option name ought to be (and I think plural is fine), and then change the internal define to match that, not vice versa. > and print > a warning for -d guest_errors to remind using guest_error,memaccess > instead but preserve previous behaviour for convenience. > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> thanks -- PMM
On Mon, 14 Oct 2024, Peter Maydell wrote: > On Sun, 6 Oct 2024 at 17:49, BALATON Zoltan <balaton@eik.bme.hu> wrote: >> >> Rename guest_errors to guest_error to match the log constant > > I don't think this is a good reason to change the user-facing > behaviour. Also, I don't think the existing names are so bad: > > -d guest_errors > this is plural because we are asking to log all guest errors > qemu_log_mask(LOG_GUEST_ERROR, ...) > this is singular because we are logging a single error here. > > If we do want to change things for consistency, we should decide > on what the user-facing option name ought to be (and I think > plural is fine), and then change the internal define to match > that, not vice versa. The point of this patch is not to match the define with the option. Originally I had a patch that just separated memory access logs and left guest_errors as it is but without the memory access logs. But I think Philippe said that he likes this option to log both. So to satisfy all needs I'v added this patch to preserve what guest_errors is doing now but let it be changed later after some time for people to get used to it (or leave it if that's what people like). Leaving guest_errors to log both is not an option as I want to have separate options for memory access and guest errors so they can be turned on or off independently. So if we have to keep guest_errors to log both then we need a new option for just guest errors for which one that matches the definen seemed like an easy way that is consistent with the other debug options. I'd be OK to leave guest_errors but without the memory access logs, then this patch is not needed but some people did not like that before. Regards, BALATON Zoltan >> and print >> a warning for -d guest_errors to remind using guest_error,memaccess >> instead but preserve previous behaviour for convenience. >> >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > > thanks > -- PMM > >
diff --git a/docs/devel/secure-coding-practices.rst b/docs/devel/secure-coding-practices.rst index 0454cc527e..b330b01956 100644 --- a/docs/devel/secure-coding-practices.rst +++ b/docs/devel/secure-coding-practices.rst @@ -85,7 +85,7 @@ request completes. Unexpected accesses must not cause memory corruption or leaks in QEMU. Invalid device register accesses can be reported with -``qemu_log_mask(LOG_GUEST_ERROR, ...)``. The ``-d guest_errors`` command-line +``qemu_log_mask(LOG_GUEST_ERROR, ...)``. The ``-d guest_error`` command-line option enables these log messages. Live Migration diff --git a/tests/avocado/smmu.py b/tests/avocado/smmu.py index 83fd79e922..7ed836c12d 100644 --- a/tests/avocado/smmu.py +++ b/tests/avocado/smmu.py @@ -46,7 +46,7 @@ def common_vm_setup(self, custom_kernel=False): self.vm.add_args("-accel", "kvm") self.vm.add_args("-cpu", "host") self.vm.add_args("-machine", "iommu=smmuv3") - self.vm.add_args("-d", "guest_errors") + self.vm.add_args("-d", "guest_error,memaccess") self.vm.add_args('-bios', os.path.join(BUILD_DIR, 'pc-bios', 'edk2-aarch64-code.fd')) self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0') diff --git a/tests/qtest/pnv-host-i2c-test.c b/tests/qtest/pnv-host-i2c-test.c index 7f64d597ac..63a2acf9de 100644 --- a/tests/qtest/pnv-host-i2c-test.c +++ b/tests/qtest/pnv-host-i2c-test.c @@ -418,7 +418,7 @@ static void test_host_i2c(const void *data) qts = qtest_initf("-M %s -smp %d,cores=1,threads=%d -nographic " "-nodefaults -serial mon:stdio -S " - "-d guest_errors", + "-d guest_error,memaccess", machine, SMT, SMT); /* Check the I2C master status registers after POR */ diff --git a/util/log.c b/util/log.c index 1aa7396277..279c2b5cfb 100644 --- a/util/log.c +++ b/util/log.c @@ -486,7 +486,7 @@ const QEMULogItem qemu_log_items[] = { "show CPU state before CPU resets" }, { LOG_UNIMP, "unimp", "log unimplemented functionality" }, - { LOG_GUEST_ERROR, "guest_errors", + { LOG_GUEST_ERROR, "guest_error", "log when the guest OS does something invalid (eg accessing a\n" "non-existent register)" }, { CPU_LOG_PAGE, "page", @@ -521,6 +521,10 @@ int qemu_str_to_log_mask(const char *str) for (item = qemu_log_items; item->mask != 0; item++) { mask |= item->mask; } + } else if (g_str_equal(*tmp, "guest_errors")) { + warn_report("Log option guest_errors is deprecated. " + "Use guest_error,memaccess instead."); + mask |= LOG_GUEST_ERROR | LOG_MEM_ACCESS; #ifdef CONFIG_TRACE_LOG } else if (g_str_has_prefix(*tmp, "trace:") && (*tmp)[6] != '\0') { trace_enable_events((*tmp) + 6);
Rename guest_errors to guest_error to match the log constant and print a warning for -d guest_errors to remind using guest_error,memaccess instead but preserve previous behaviour for convenience. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- As this is a debug switch I think no formal deprecation is needed but this is to allow people to change their commands and scripts then it may be removed eventually. docs/devel/secure-coding-practices.rst | 2 +- tests/avocado/smmu.py | 2 +- tests/qtest/pnv-host-i2c-test.c | 2 +- util/log.c | 6 +++++- 4 files changed, 8 insertions(+), 4 deletions(-)