I'm keeping the EACCES because callers expect to be able to look at
errno.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Remove fds right away instead of setting the ->removed flag. We don't
need the extra complexity of having a cleanup function reap the
removed entries at a later time.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
monitor_fdsets_cleanup() currently has three responsibilities:
1- Remove the fds that have been marked for removal(->removed=true) by
qmp_remove_fd(). This is overly complicated, but ok.
2- Remove any file descriptors that have been passed into QEMU and
never duplicated[1,2]. A file descriptor without duplicates
indicates that no part of QEMU has made use of it. This is
problematic because the current implementation does it only if the
guest is not running and the monitor is closed.
3- Remove/free fdsets that have become empty due to the above
removals. This is ok.
The scenario described in (2) is starting to show some cracks now that
we're trying to consume fds from the migration code:
- Doing cleanup every time the last monitor connection closes works to
reap unused fds, but also has the side effect of forcing the
management layer to pass the file descriptors again in case of a
disconnect/re-connect, if that happened to be the only monitor
connection.
Another side effect is that removing an fd with qmp_remove_fd() is
effectively delayed until the last monitor connection closes.
The usage of mon_refcount is also problematic because it's racy.
- Checking runstate_is_running() skips the cleanup unless the VM is
running and avoids premature cleanup of the fds, but also has the
side effect of blocking the legitimate removal of an fd via
qmp_remove_fd() if the VM happens to be in another state.
This affects qmp_remove_fd() and qmp_query_fdsets() in particular
because requesting a removal at a bad time (guest stopped) might
cause an fd to never be removed, or to be removed at a much later
point in time, causing the query command to continue showing the
supposedly removed fd/fdset.
Note that file descriptors that *have* been duplicated are owned by
the code that uses them and will be removed after qemu_close() is
called. Therefore we've decided that the best course of action to
avoid the undesired side-effects is to stop managing non-duplicated
file descriptors.
1- efb87c1697 ("monitor: Clean up fd sets on monitor disconnect")
2- ebe52b592d ("monitor: Prevent removing fd from set during init")
Reviewed-by: Peter Xu <peterx@redhat.com>
[fix logic mistake: s/fdset_free/fdset_free_if_empty]
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Introduce new functions to remove and free no longer used fds and
fdsets.
We need those to decouple the remove/free routines from
monitor_fdset_cleanup() which will go away in the next patches.
The new functions:
- monitor_fdset_free/_if_empty() will be used when a monitor
connection closes and when an fd is removed to cleanup any fdset
that is now empty.
- monitor_fdset_fd_free() will be used to remove one or more fds that
have been explicitly targeted by qmp_remove_fd().
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Those functions are not needed, one remove function should already
work. Clean it up.
Here the code doesn't really care about whether we need to keep that dupfd
around if close() failed: when that happens something got very wrong,
keeping the dup_fd around the fdsets may not help that situation so far.
Cc: Dr. David Alan Gilbert <dave@treblig.org>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
[add missing return statement, removal during traversal is not safe]
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Currently, the function will simply fail if ancillary fds are not
provided, for ex on unsupported platforms.
This changes the failure from:
{"error": {"class": "GenericError", "desc": "No file descriptor
supplied via SCM_RIGHTS"}}
to:
{"error": {"class": "CommandNotFound", "desc": "The command getfd
has not been found"}}
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
A process with enough capabilities can duplicate a socket to QEMU. Add a
QMP command to import it and add it to the monitor fd list, so it can be
later used by other commands.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20230306122751.2355515-9-marcandre.lureau@redhat.com>
As per comment, presumably to avoid syscall in critical section.
Fixes: 0210c3b39b ("monitor: Use LOCK_GUARD macros")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20230306122751.2355515-7-marcandre.lureau@redhat.com>
