If the check is called during normal work, tracking of the check must be
present in VM logs to have some clues if something going wrong with user's
data.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
block/parallels.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 9fa1f93973..d64e8007d5 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -42,6 +42,7 @@
#include "qemu/bswap.h"
#include "qemu/bitmap.h"
#include "qemu/memalign.h"
+#include "qemu/log-for-trace.h"
#include "migration/blocker.h"
#include "parallels.h"
@@ -436,8 +437,8 @@ static void parallels_check_unclean(BlockDriverState *bs,
return;
}
- fprintf(stderr, "%s image was not closed correctly\n",
- fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR");
+ qemu_log("%s image was not closed correctly\n",
+ fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR");
res->corruptions++;
if (fix & BDRV_FIX_ERRORS) {
/* parallels_close will do the job right */
@@ -464,8 +465,8 @@ parallels_check_outside_image(BlockDriverState *bs, BdrvCheckResult *res,
for (i = 0; i < s->bat_size; i++) {
off = bat2sect(s, i) << BDRV_SECTOR_BITS;
if (off + s->cluster_size > size) {
- fprintf(stderr, "%s cluster %u is outside image\n",
- fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
+ qemu_log("%s cluster %u is outside image\n",
+ fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
res->corruptions++;
if (fix & BDRV_FIX_ERRORS) {
parallels_set_bat_entry(s, i, 0);
@@ -551,8 +552,8 @@ parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res,
count = DIV_ROUND_UP(leak_size, s->cluster_size);
res->leaks += count;
- fprintf(stderr, "%s space leaked at the end of the image %" PRId64 "\n",
- fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR", leak_size);
+ qemu_log("%s space leaked at the end of the image %" PRId64 "\n",
+ fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR", leak_size);
if (fix & BDRV_FIX_LEAKS) {
res->leaks_fixed += count;
@@ -603,9 +604,8 @@ static int parallels_check_duplicate(BlockDriverState *bs,
cluster_index = host_cluster_index(s, off);
if (test_bit(cluster_index, bitmap)) {
/* this cluster duplicates another one */
- fprintf(stderr,
- "%s duplicate offset in BAT entry %u\n",
- *fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
+ qemu_log("%s duplicate offset in BAT entry %u\n",
+ *fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
res->corruptions++;
--
2.34.1On Mon, 29 May 2023 at 16:16, Alexander Ivanov
<alexander.ivanov@virtuozzo.com> wrote:
>
> If the check is called during normal work, tracking of the check must be
> present in VM logs to have some clues if something going wrong with user's
> data.
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> Reviewed-by: Denis V. Lunev <den@openvz.org>
> ---
> block/parallels.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 9fa1f93973..d64e8007d5 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -42,6 +42,7 @@
> #include "qemu/bswap.h"
> #include "qemu/bitmap.h"
> #include "qemu/memalign.h"
> +#include "qemu/log-for-trace.h"
> #include "migration/blocker.h"
> #include "parallels.h"
>
> @@ -436,8 +437,8 @@ static void parallels_check_unclean(BlockDriverState *bs,
> return;
> }
>
> - fprintf(stderr, "%s image was not closed correctly\n",
> - fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR");
> + qemu_log("%s image was not closed correctly\n",
> + fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR");
Generally speaking you shouldn't directly call qemu_log().
Instead call qemu_log_mask() and pass it the relevant
QEMU_LOG_* constant for whichever kind of -d debug logging
this is. The raw qemu_log() function is for the odd case
of expensive logging where you want to say
if (log category foo enabled) {
do expensive calculation;
qemu_log(result1);
qemu_log(result2);
}
But this doesn't look like the kind of logging we would usually
do with qemu_log(). Notably, the default for the qemu_log machinery
is to not output anything at all: it's only enabled if the user
explicitly turns on debug logging with a -d option.
You probably want warn_report() here.
thanks
-- PMM
On 29.05.23 17:15, Alexander Ivanov wrote: > If the check is called during normal work, tracking of the check must be > present in VM logs to have some clues if something going wrong with user's > data. I understand stderr counts as part of the VM log, doesn’t it? I thought stderr is generally logged, and naïvely, it seems like the better fit to me, because it conveys more urgency than the standard log (which, judging from its callers, looks mostly like a debug log). Hanna > Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> > Reviewed-by: Denis V. Lunev <den@openvz.org> > --- > block/parallels.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-)
On 6/2/23 16:48, Hanna Czenczek wrote: > On 29.05.23 17:15, Alexander Ivanov wrote: >> If the check is called during normal work, tracking of the check must be >> present in VM logs to have some clues if something going wrong with >> user's >> data. > > I understand stderr counts as part of the VM log, doesn’t it? I > thought stderr is generally logged, and naïvely, it seems like the > better fit to me, because it conveys more urgency than the standard > log (which, judging from its callers, looks mostly like a debug log). > > Hanna We want to add some image checks to parallels_open(). It means that it will be used not only in qemu-img and we may lose these messages if a log file is specified. Stderr is not duplicated to the log file. > >> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> >> Reviewed-by: Denis V. Lunev <den@openvz.org> >> --- >> block/parallels.c | 18 +++++++++--------- >> 1 file changed, 9 insertions(+), 9 deletions(-) > -- Best regards, Alexander Ivanov
© 2016 - 2024 Red Hat, Inc.