On Sun, Jan 4, 2026 at 3:54 PM Jason Wang <[email protected]> wrote:
>
> Currently, filter-redirector does not implement the status_changed
> callback, which means the 'status' property cannot be used to
> dynamically enable/disable the filter at runtime. When status is
> set to 'off' via QMP/HMP, the filter still receives data from the
> indev chardev because the chardev handlers remain registered.
>
> This patch adds proper support for the 'status' property:
>
> 1. Implement filter_redirector_status_changed() callback:
> - When status changes to 'off': remove chardev read handlers
> - When status changes to 'on': re-register chardev handlers
> (only if chardev is already open)
>
> 2. Update filter_redirector_setup() to respect initial status:
> - If filter is created with status=off, do not register handlers
> - This allows creating disabled filters via command line or QMP
>
> 3. Handle chardev OPENED/CLOSED events to re-arm handlers on reconnect:
> - Keep the chr_event callback installed on CLOSE so a later OPENED
> can re-register the read handlers when nf->on
> - Use qemu_chr_fe_set_handlers_full(..., set_open=false, sync_state=false)
> instead of qemu_chr_fe_set_handlers() because the latter forces
> sync_state=true and may emit CHR_EVENT_OPENED for an already-open
> backend. Doing that from inside the chr_event callback would cause
> recursive/re-entrant OPENED handling.
>
> Signed-off-by: Jason Wang <[email protected]>
> ---
> net/filter-mirror.c | 38 +++++++++++++++++++++++++++++++++-----
> 1 file changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index 8e01e98f40..aa2ab452fd 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -179,9 +179,16 @@ static void redirector_chr_event(void *opaque,
> QEMUChrEvent event)
> MirrorState *s = FILTER_REDIRECTOR(nf);
>
> switch (event) {
> + case CHR_EVENT_OPENED:
> + if (nf->on) {
> + qemu_chr_fe_set_handlers_full(&s->chr_in,
> redirector_chr_can_read,
> + redirector_chr_read,
> redirector_chr_event,
> + NULL, nf, NULL, false, false);
> + }
> + break;
> case CHR_EVENT_CLOSED:
> - qemu_chr_fe_set_handlers(&s->chr_in, NULL, NULL, NULL,
> - NULL, NULL, NULL, true);
> + qemu_chr_fe_set_handlers_full(&s->chr_in, NULL, NULL,
> redirector_chr_event,
> + NULL, nf, NULL, false, false);
> break;
> default:
> break;
> @@ -306,9 +313,11 @@ static void filter_redirector_setup(NetFilterState *nf,
> Error **errp)
> return;
> }
>
> - qemu_chr_fe_set_handlers(&s->chr_in, redirector_chr_can_read,
> - redirector_chr_read, redirector_chr_event,
> - NULL, nf, NULL, true);
> + if (nf->on) {
> + qemu_chr_fe_set_handlers(&s->chr_in, redirector_chr_can_read,
> + redirector_chr_read,
> redirector_chr_event,
> + NULL, nf, NULL, true);
> + }
> }
>
> if (s->outdev) {
> @@ -324,6 +333,24 @@ static void filter_redirector_setup(NetFilterState *nf,
> Error **errp)
> }
> }
>
> +static void filter_redirector_status_changed(NetFilterState *nf, Error
> **errp)
> +{
> + MirrorState *s = FILTER_REDIRECTOR(nf);
> +
> + if (!s->indev) {
It's better to add a error here, for example:
error_setg(errp, "filter_redirector_status_changed failed for
the s->indev cleared" );
> + return;
> + }
> +
> + if (nf->on) {
> + qemu_chr_fe_set_handlers(&s->chr_in, redirector_chr_can_read,
> + redirector_chr_read, redirector_chr_event,
> + NULL, nf, NULL, true);
> + } else {
> + qemu_chr_fe_set_handlers(&s->chr_in, NULL, NULL, NULL,
> + NULL, NULL, NULL, true);
It seems like we need to keep the "redirector_chr_event" here?
And another thing, this series focus on indev, should we need to do
the same thing for outdev?
Thanks
Chen
> + }
> +}
> +
> static char *filter_redirector_get_indev(Object *obj, Error **errp)
> {
> MirrorState *s = FILTER_REDIRECTOR(obj);
> @@ -440,6 +467,7 @@ static void filter_redirector_class_init(ObjectClass *oc,
> const void *data)
> nfc->setup = filter_redirector_setup;
> nfc->cleanup = filter_redirector_cleanup;
> nfc->receive_iov = filter_redirector_receive_iov;
> + nfc->status_changed = filter_redirector_status_changed;
> }
>
> static void filter_mirror_init(Object *obj)
> --
> 2.34.1
>
