On 29.08.2013 18:36, Peter Krempa wrote:
--- cfg.mk | 2 +- po/POTFILES.in | 2 +- tools/Makefile.am | 2 +- tools/{console.c => virsh-console.c} | 73 ++++++++++++++++++++++-------------- tools/{console.h => virsh-console.h} | 4 +- tools/virsh-domain.c | 2 +- tools/virsh.c | 2 +- 7 files changed, 52 insertions(+), 35 deletions(-) rename tools/{console.c => virsh-console.c} (90%) rename tools/{console.h => virsh-console.h} (91%)
diff --git a/cfg.mk b/cfg.mk index 23564f1..5c3f3ab 100644 --- a/cfg.mk +++ b/cfg.mk @@ -907,7 +907,7 @@ exclude_file_name_regexp--sc_avoid_strcase = ^tools/virsh\.h$$ _src1=libvirt|fdstream|qemu/qemu_monitor|util/(vircommand|virfile)|xen/xend_internal|rpc/virnetsocket|lxc/lxc_controller|locking/lock_daemon _test1=shunloadtest|virnettlscontexttest|virnettlssessiontest|vircgroupmock exclude_file_name_regexp--sc_avoid_write = \ - ^(src/($(_src1))|daemon/libvirtd|tools/console|tests/($(_test1)))\.c$$ + ^(src/($(_src1))|daemon/libvirtd|tools/virsh-console|tests/($(_test1)))\.c$$
exclude_file_name_regexp--sc_bindtextdomain = ^(tests|examples)/
diff --git a/po/POTFILES.in b/po/POTFILES.in index 9a83069..aa604fd 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -213,10 +213,10 @@ src/xenapi/xenapi_driver.c src/xenapi/xenapi_utils.c src/xenxs/xen_sxpr.c src/xenxs/xen_xm.c -tools/console.c tools/libvirt-guests.sh.in tools/virsh.c tools/virsh.h +tools/virsh-console.c tools/virsh-domain-monitor.c tools/virsh-domain.c tools/virsh-edit.c diff --git a/tools/Makefile.am b/tools/Makefile.am index 74fae2d..cd4cb31 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -161,8 +161,8 @@ virt_login_shell_CFLAGS = \ $(COVERAGE_CFLAGS)
virsh_SOURCES = \ - console.c console.h \ virsh.c virsh.h \ + virsh-console.c virsh-console.h \ virsh-domain.c virsh-domain.h \ virsh-domain-monitor.c virsh-domain-monitor.h \ virsh-host.c virsh-host.h \ diff --git a/tools/console.c b/tools/virsh-console.c similarity index 90% rename from tools/console.c rename to tools/virsh-console.c index 6c24fcf..debf12c 100644 --- a/tools/console.c +++ b/tools/virsh-console.c @@ -1,7 +1,7 @@ /* - * console.c: A dumb serial console client + * virsh-console.c: A dumb serial console client * - * Copyright (C) 2007-2008, 2010-2012 Red Hat, Inc. + * Copyright (C) 2007-2008, 2010-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public
Not shown in the context, but there should be 'Authors:' line just above Dan's name.
@@ -37,7 +37,7 @@ # include <c-ctype.h>
# include "internal.h" -# include "console.h" +# include "virsh-console.h" # include "virlog.h" # include "virfile.h" # include "viralloc.h" @@ -58,6 +58,7 @@ struct virConsoleBuffer { char *data; };
+ typedef struct virConsole virConsole; typedef virConsole *virConsolePtr; struct virConsole { @@ -75,11 +76,15 @@ struct virConsole { char escapeChar; };
+ static int got_signal = 0; -static void do_signal(int sig ATTRIBUTE_UNUSED) { +static void +virConsoleHandleSignal(int sig ATTRIBUTE_UNUSED) +{ got_signal = 1; }
+ # ifndef HAVE_CFMAKERAW static void cfmakeraw(struct termios *attr) @@ -93,6 +98,7 @@ cfmakeraw(struct termios *attr) } # endif /* !HAVE_CFMAKERAW */
+ static void virConsoleShutdown(virConsolePtr con) { @@ -114,6 +120,21 @@ virConsoleShutdown(virConsolePtr con) virCondSignal(&con->cond); }
+ +static void +virConsoleFree(virConsolePtr con) +{ + if (!con) + return; + + if (con->st) + virStreamFree(con->st); + virMutexDestroy(&con->lock); + virCondDestroy(&con->cond); + VIR_FREE(con); +} + + static void virConsoleEventOnStream(virStreamPtr st, int events, void *opaque) @@ -171,9 +192,8 @@ virConsoleEventOnStream(virStreamPtr st,
avail = con->terminalToStream.length - con->terminalToStream.offset; if (avail > 1024) { - if (VIR_REALLOC_N(con->terminalToStream.data, - con->terminalToStream.offset + 1024) < 0) - {} + ignore_value(VIR_REALLOC_N(con->terminalToStream.data, + con->terminalToStream.offset + 1024)); con->terminalToStream.length = con->terminalToStream.offset + 1024;
I don't think this is quite right. If the VIR_REALLOC fails, why are we increasing the .lenght? I know this is pre-existing, but if we are touching this we should fix this.
} } @@ -187,6 +207,7 @@ virConsoleEventOnStream(virStreamPtr st, } }
+ static void virConsoleEventOnStdin(int watch ATTRIBUTE_UNUSED, int fd ATTRIBUTE_UNUSED, @@ -242,6 +263,7 @@ virConsoleEventOnStdin(int watch ATTRIBUTE_UNUSED, } }
+ static void virConsoleEventOnStdout(int watch ATTRIBUTE_UNUSED, int fd, @@ -270,9 +292,8 @@ virConsoleEventOnStdout(int watch ATTRIBUTE_UNUSED,
avail = con->streamToTerminal.length - con->streamToTerminal.offset; if (avail > 1024) { - if (VIR_REALLOC_N(con->streamToTerminal.data, - con->streamToTerminal.offset + 1024) < 0) - {} + ignore_value(VIR_REALLOC_N(con->streamToTerminal.data, + con->streamToTerminal.offset + 1024)); con->streamToTerminal.length = con->streamToTerminal.offset + 1024;
And again.
} } @@ -296,6 +317,7 @@ vshGetEscapeChar(const char *s) return *s; }
+ int vshMakeStdinRaw(struct termios *ttyattr, bool report_errors) { @@ -322,10 +344,12 @@ vshMakeStdinRaw(struct termios *ttyattr, bool report_errors) return 0;
-int vshRunConsole(virDomainPtr dom, - const char *dev_name, - const char *escape_seq, - unsigned int flags) + +int +vshRunConsole(virDomainPtr dom, + const char *dev_name, + const char *escape_seq, + unsigned int flags) { int ret = -1; struct termios ttyattr; @@ -348,11 +372,11 @@ int vshRunConsole(virDomainPtr dom, the original terminal settings on STDIN before the process exits - people don't like being left with a messed up terminal ! */ - old_sigquit = signal(SIGQUIT, do_signal); - old_sigterm = signal(SIGTERM, do_signal); - old_sigint = signal(SIGINT, do_signal); - old_sighup = signal(SIGHUP, do_signal); - old_sigpipe = signal(SIGPIPE, do_signal); + old_sigquit = signal(SIGQUIT, virConsoleHandleSignal); + old_sigterm = signal(SIGTERM, virConsoleHandleSignal); + old_sigint = signal(SIGINT, virConsoleHandleSignal); + old_sighup = signal(SIGHUP, virConsoleHandleSignal); + old_sigpipe = signal(SIGPIPE, virConsoleHandleSignal); got_signal = 0;
if (VIR_ALLOC(con) < 0) @@ -396,15 +420,8 @@ int vshRunConsole(virDomainPtr dom,
ret = 0;
- cleanup: - - if (con) { - if (con->st) - virStreamFree(con->st); - virMutexDestroy(&con->lock); - virCondDestroy(&con->cond); - VIR_FREE(con); - } +cleanup: + virConsoleFree(con);
/* Restore original signal handlers */ signal(SIGPIPE, old_sigpipe); diff --git a/tools/console.h b/tools/virsh-console.h similarity index 91% rename from tools/console.h rename to tools/virsh-console.h index 46255b8..96ef235 100644 --- a/tools/console.h +++ b/tools/virsh-console.h @@ -1,7 +1,7 @@ /* - * console.c: A dumb serial console client + * virsh-console.h: A dumb serial console client * - * Copyright (C) 2007, 2010, 2012 Red Hat, Inc. + * Copyright (C) 2007, 2010, 2012-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public
Not shown in the context, but there should be 'Authors:' line just above Dan's name.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c87cf6a..60eed51 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -41,7 +41,7 @@ #include "virbuffer.h" #include "c-ctype.h" #include "conf/domain_conf.h" -#include "console.h" +#include "virsh-console.h"
This should go a few lines down, just before the line: #include "virsh-domain-monitor.h"
#include "viralloc.h" #include "vircommand.h" #include "virfile.h" diff --git a/tools/virsh.c b/tools/virsh.c index 2f04e6a..0cc9bdd 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -57,7 +57,6 @@ #include "virerror.h" #include "base64.h" #include "virbuffer.h" -#include "console.h" #include "viralloc.h" #include "virxml.h" #include <libvirt/libvirt-qemu.h> @@ -73,6 +72,7 @@ #include "virtypedparam.h" #include "virstring.h"
+#include "virsh-console.h" #include "virsh-domain.h" #include "virsh-domain-monitor.h" #include "virsh-host.h"
I've pointed a few issues but I believe that you will fix them right so I don't need to see a v2. ACK Michal