[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 02/12] monitor: Use getter/setter functions for cur_mon
From: |
Kevin Wolf |
Subject: |
Re: [PATCH v6 02/12] monitor: Use getter/setter functions for cur_mon |
Date: |
Tue, 2 Jun 2020 15:36:29 +0200 |
Am 28.05.2020 um 20:31 hat Eric Blake geschrieben:
> On 5/28/20 10:37 AM, Kevin Wolf wrote:
> > cur_mon really needs to be coroutine-local as soon as we move monitor
> > command handlers to coroutines and let them yield. As a first step, just
> > remove all direct accesses to cur_mon so that we can implement this in
> > the getter function later.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
>
>
> > +++ b/audio/wavcapture.c
> > @@ -1,5 +1,5 @@
> > #include "qemu/osdep.h"
> > -#include "monitor/monitor.h"
> > +#include "qemu/qemu-print.h"
> > #include "qapi/error.h"
> > #include "qemu/error-report.h"
> > #include "audio.h"
> > @@ -94,9 +94,9 @@ static void wav_capture_info (void *opaque)
> > WAVState *wav = opaque;
> > char *path = wav->path;
> > - monitor_printf (cur_mon, "Capturing audio(%d,%d,%d) to %s: %d bytes\n",
> > - wav->freq, wav->bits, wav->nchannels,
> > - path ? path : "<not available>", wav->bytes);
> > + qemu_printf("Capturing audio(%d,%d,%d) to %s: %d bytes\n",
> > + wav->freq, wav->bits, wav->nchannels,
> > + path ? path : "<not available>", wav->bytes);
>
> Why the change to qemu_printf() here instead of a call to
> monitor_cur() as is done in the rest of the patch?
There was actually only one other monitor_printf() call for cur_mon, and
that was in the monitor core, so I think there's no reason to switch
that one.
If you prefer, I can call monitor_cur() directly here, but I thought not
calling it from everywhere when there is already a wrapper like
qemu_printf() would be a good thing.
Maybe wav_capture_info() should actually be passed a Monitor object so
we don't need either.
> > +++ b/stubs/monitor-core.c
> > @@ -3,7 +3,10 @@
> > #include "qemu-common.h"
> > #include "qapi/qapi-emit-events.h"
> > -__thread Monitor *cur_mon;
> > +Monitor *monitor_cur(void)
> > +{
> > + return NULL;
> > +}
>
> monitor_set_cur() didn't need a stub? Odd, but I guess it's okay.
Not that odd, only the code to handle monitor commands ever sets the
current monitor. But if you use the stubs, you don't have a monitor.
Kevin
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH v6 02/12] monitor: Use getter/setter functions for cur_mon,
Kevin Wolf <=