Hi,
Just for reference, the output of spice-vdagentd -h before was:
| $ spice-vdagentd -h
| Usage: spice-vdagentd [OPTIONS]
|
| Spice guest agent daemon, version 0.17.0.
|
| Options:
| -h print this text
| -d log debug messages (use twice for extra info)
| -s <port> set virtio serial port [/dev/virtio-ports/com.redhat.spice.0]
| -S <filename> set udcs socket [/var/run/spice-vdagentd/spice-vdagent-sock]
| -u <dev> set uinput device [/dev/uinput]
| -f treat uinput device as fake; no ioctls
| -x don't daemonize
| -o Only handle one virtio serial session.
| -X Disable systemd-logind integration
and not it is:
| $ /usr/sbin/spice-vdagentd -h
| Usage:
| spice-vdagentd [OPTION?]
|
| Spice guest agent daemon, version 0.18.0
|
| Help Options:
| -h, --help Show help options
|
| Application Options:
| -d, --debug Log debug messages (use twice for extra info)
| -s, --virtio-serial-port-path Set virtio-serial path (/dev/virtio-ports/com.redhat.spice.0)
| -S, --vdagentd-socket Set spice-vdagentd socket (/var/run/spice-vdagentd/spice-vdagent-sock)
| -u, --uinput-device Set uinput device (/dev/uinput)
| -f, --fake-uinput Treat uinput device as fake; no ioctls
| -x, --foreground Do not daemonize the agent
| -o, --one-session Only handle one virtio serial session
| -X, --disable-session-integration Disable console kit and systemd-logind integration
I think that after "vdagentd: use GMainLoop" patch, we should
also include glib's command line options.
On Tue, Aug 14, 2018 at 08:53:35PM +0200, Jakub Janků wrote:
> All command line options now have long names
> as they are required by GLib.
I think that adding the following to the commit log as a quick
overview of current options and its long names might be helpful.
-d, --debug
-s, --virtio-serial-port-path
-S, --vdagentd-socket
-u, --uinput-device
-f, --fake-uinput
-x, --foreground
-o, --one-session
-X, --disable-session-integration
> Change types of some global variables that hold the options:
> - const char * --> gchar *
> - int --> gboolean
>
> Define DEFAULT_UINPUT_DEVICE as "/dev/uinput",
> since there's already DEFAULT_VIRTIO_PORT_PATH, VDAGENTD_SOCKET.
Would you mind adding Signed-off-by: Name <email> ?
(more below)
> ---
> src/vdagentd/vdagentd.c | 149 ++++++++++++++++++++++------------------
> 1 file changed, 82 insertions(+), 67 deletions(-)
>
> diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
> index 682761a..d88bbc7 100644
> --- a/src/vdagentd/vdagentd.c
> +++ b/src/vdagentd/vdagentd.c
> @@ -48,6 +48,8 @@
> #include "virtio-port.h"
> #include "session-info.h"
>
> +#define DEFAULT_UINPUT_DEVICE "/dev/uinput"
> +
> struct agent_data {
> char *session;
> int width;
> @@ -58,12 +60,16 @@ struct agent_data {
>
> /* variables */
> static const char *pidfilename = "/var/run/spice-vdagentd/spice-vdagentd.pid";
> -static const char *portdev = DEFAULT_VIRTIO_PORT_PATH;
> -static const char *vdagentd_socket = VDAGENTD_SOCKET;
> -static const char *uinput_device = "/dev/uinput";
> +
> +static gchar *portdev = NULL;
> +static gchar *vdagentd_socket = NULL;
> +static gchar *uinput_device = NULL;
> static int debug = 0;
> -static int uinput_fake = 0;
> -static int only_once = 0;
> +static gboolean uinput_fake = FALSE;
> +static gboolean only_once = FALSE;
> +static gboolean do_daemonize = TRUE;
> +static gboolean want_session_info = TRUE;
> +
> static struct udscs_server *server = NULL;
> static struct vdagent_virtio_port *virtio_port = NULL;
> static GHashTable *active_xfers = NULL;
> @@ -960,29 +966,6 @@ static void agent_read_complete(struct udscs_connection **connp,
>
> /* main */
>
> -static void usage(FILE *fp)
> -{
> - fprintf(fp,
> - "Usage: spice-vdagentd [OPTIONS]\n\n"
> - "Spice guest agent daemon, version %s.\n\n"
> - "Options:\n"
> - " -h print this text\n"
> - " -d log debug messages (use twice for extra info)\n"
> - " -s <port> set virtio serial port [%s]\n"
> - " -S <filename> set vdagent Unix domain socket [%s]\n"
> - " -u <dev> set uinput device [%s]\n"
> - " -f treat uinput device as fake; no ioctls\n"
> - " -x don't daemonize\n"
> - " -o only handle one virtio serial session\n"
> -#ifdef HAVE_CONSOLE_KIT
> - " -X disable console kit integration\n"
> -#endif
> -#ifdef HAVE_LIBSYSTEMD_LOGIN
> - " -X disable systemd-logind integration\n"
> -#endif
> - ,VERSION, portdev, vdagentd_socket, uinput_device);
> -}
> -
> static void daemonize(void)
> {
> int x;
> @@ -1081,52 +1064,80 @@ static void quit_handler(int sig)
> quit = 1;
> }
>
> +static gboolean parse_debug_level_cb(const gchar *option_name,
> + const gchar *value,
> + gpointer data,
> + GError **error)
> +{
> + debug++;
> + return TRUE;
> +}
It is okay to leave this for now as this patches are touching the
command line parsing part but within the context of glib
integration, we might consider removing this in the future and
follow what might be dictated by Glib's logging system.
> +static GOptionEntry cmd_entries[] = {
> + { "debug", 'd', G_OPTION_FLAG_NO_ARG,
> + G_OPTION_ARG_CALLBACK, parse_debug_level_cb,
> + "Log debug messages (use twice for extra info)", NULL },
> +
> + { "virtio-serial-port-path", 's', 0,
> + G_OPTION_ARG_STRING, &portdev,
> + "Set virtio-serial path (" DEFAULT_VIRTIO_PORT_PATH ")", NULL },
> +
> + { "vdagentd-socket", 'S', 0,
> + G_OPTION_ARG_STRING, &vdagentd_socket,
> + "Set spice-vdagentd socket (" VDAGENTD_SOCKET ")", NULL },
> +
> + { "uinput-device", 'u', 0,
> + G_OPTION_ARG_STRING, &uinput_device,
> + "Set uinput device (" DEFAULT_UINPUT_DEVICE ")", NULL },
> +
> + { "fake-uinput", 'f', 0,
> + G_OPTION_ARG_NONE, &uinput_fake,
> + "Treat uinput device as fake; no ioctls", NULL },
> +
> + { "foreground", 'x', G_OPTION_FLAG_REVERSE,
> + G_OPTION_ARG_NONE, &do_daemonize,
> + "Do not daemonize the agent", NULL},
> +
> + { "one-session", 'o', 0,
> + G_OPTION_ARG_NONE, &only_once,
> + "Only handle one virtio serial session", NULL },
> +
> +#if defined(HAVE_CONSOLE_KIT) || defined (HAVE_LIBSYSTEMD_LOGIN)
> + { "disable-session-integration", 'X', G_OPTION_FLAG_REVERSE,
> + G_OPTION_ARG_NONE, &want_session_info,
> + "Disable console kit and systemd-logind integration", NULL },
> +#endif
> +
> + { NULL }
> +};
> +
> int main(int argc, char *argv[])
> {
> - int c;
> - int do_daemonize = 1;
> - int want_session_info = 1;
> + GOptionContext *context;
> + GError *err = NULL;
> struct sigaction act;
> gboolean own_socket = TRUE;
>
> - for (;;) {
> - if (-1 == (c = getopt(argc, argv, "-dhxXfos:u:S:")))
> - break;
> - switch (c) {
> - case 'd':
> - debug++;
> - break;
> - case 's':
> - portdev = optarg;
> - break;
> - case 'S':
> - vdagentd_socket = optarg;
> - break;
> - case 'u':
> - uinput_device = optarg;
> - break;
> - case 'f':
> - uinput_fake = 1;
> - break;
> - case 'o':
> - only_once = 1;
> - break;
> - case 'x':
> - do_daemonize = 0;
> - break;
> - case 'X':
> - want_session_info = 0;
> - break;
> - case 'h':
> - usage(stdout);
> - return 0;
> - default:
> - fputs("\n", stderr);
> - usage(stderr);
> - return 1;
> - }
> + context = g_option_context_new(NULL);
> + g_option_context_add_main_entries(context, cmd_entries, NULL);
> + g_option_context_set_summary(context,
> + "Spice guest agent daemon, version " VERSION);
> + g_option_context_parse(context, &argc, &argv, &err);
> + g_option_context_free(context);
> +
> + if (err) {
> + g_printerr("Invalid arguments, %s\n", err->message);
We don't use g_printerr() or any g_log() in this code yet.
I hope we can move to that whenever we bump glib to 2.50 or above
as it writes to systemd's journal by default for daemons (AFAIK).
Reviewed-by: Victor Toso <victortoso@xxxxxxxxxx>
Feel free to submit a new version with fixes as standalone patch
with me in cc, to reduce the backlog of this series ;)
Cheers,
Victor
> + g_error_free(err);
> + return 1;
> }
>
> + if (portdev == NULL)
> + portdev = g_strdup(DEFAULT_VIRTIO_PORT_PATH);
> + if (vdagentd_socket == NULL)
> + vdagentd_socket = g_strdup(VDAGENTD_SOCKET);
> + if (uinput_device == NULL)
> + uinput_device = g_strdup(DEFAULT_UINPUT_DEVICE);
> +
> memset(&act, 0, sizeof(act));
> act.sa_flags = SA_RESTART;
> act.sa_handler = quit_handler;
> @@ -1223,5 +1234,9 @@ int main(int argc, char *argv[])
> if (do_daemonize)
> unlink(pidfilename);
>
> + g_free(portdev);
> + g_free(vdagentd_socket);
> + g_free(uinput_device);
> +
> return retval;
> }
> --
> 2.17.1
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel