This is similar to the upgrade RPC but leaves out sending the new server binary and allows changing the server command line which makes it more flexible.
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/lib/WineTestBot/TestAgent.pm | 52 ++++++ testbot/scripts/TestAgent | 14 +- testbot/src/testagentd/platform.h | 2 +- testbot/src/testagentd/platform_unix.c | 45 +++-- testbot/src/testagentd/platform_windows.c | 199 ++++++++++++++++++++-- testbot/src/testagentd/testagentd.c | 72 +++++++- 6 files changed, 356 insertions(+), 28 deletions(-)
diff --git a/testbot/lib/WineTestBot/TestAgent.pm b/testbot/lib/WineTestBot/TestAgent.pm index 3cd72b503..a4d32ec27 100644 --- a/testbot/lib/WineTestBot/TestAgent.pm +++ b/testbot/lib/WineTestBot/TestAgent.pm @@ -40,6 +40,7 @@ my $RPC_UPGRADE = 9; my $RPC_RMCHILDPROC = 10; my $RPC_GETCWD = 11; my $RPC_SETPROPERTY = 12; +my $RPC_RESTART = 13;
my %RpcNames=( $RPC_PING => 'ping', @@ -55,6 +56,7 @@ my %RpcNames=( $RPC_RMCHILDPROC => 'rmchildproc', $RPC_GETCWD => 'getcwd', $RPC_SETPROPERTY => 'setproperty', + $RPC_RESTART => 'restart', );
my $Debug = 0; @@ -1482,6 +1484,56 @@ sub Upgrade($$) return $rc; }
+sub Restart($$) +{ + my ($self, $Argv) = @_; + + if (!$Argv || !@$Argv) + { + # Restart the server with the same parameters + my $Properties = $self->GetProperties(); + my $Argc = $Properties->{"server.argc"}; + if (!$Argc) + { + $self->_SetError($ERROR, "Could not get the server command line argument count"); + return undef; + } + for (my $i = 0; $i < $Argc; $i++) + { + my $Arg = $Properties->{sprintf("server.argv[%d]", $i)}; + if (!defined $Arg) + { + $self->_SetError($ERROR, "Server argument $i is undefined!"); + return undef; + } + push @$Argv, $Arg; + } + } + debug("Restart TestAgentd: '", join("' '", @$Argv), "'\n"); + + if (!$self->_StartRPC($RPC_RESTART) or + !$self->_SendListSize('ArgC', scalar(@$Argv))) + { + return undef; + } + my $i = 0; + foreach my $Arg (@$Argv) + { + return undef if (!$self->_SendString("Cmd$i", $Arg)); + $i++; + } + + # Get the reply + my $rc = $self->_RecvList(''); + + # The server has quit and thus the connection is no longer usable. + # So disconnect now to force the next RPC to reconnect, instead or letting it + # try to reuse the broken connection and fail. + $self->Disconnect(); + + return $rc; +} + sub RemoveChildProcess($$) { my ($self, $Pid) = @_; diff --git a/testbot/scripts/TestAgent b/testbot/scripts/TestAgent index b4ec582be..84bbf7e0c 100755 --- a/testbot/scripts/TestAgent +++ b/testbot/scripts/TestAgent @@ -53,7 +53,7 @@ sub error(@) }
my ($Cmd, $Hostname, $LocalFilename, $ServerFilename, $PropName, $PropValue, @Rm); -my (@Run, $RunIn, $RunOut, $RunErr, $WaitPid); +my (@Run, $RunIn, $RunOut, $RunErr, $WaitPid, @Restart); my $SendFlags = 0; my $RunFlags = 0; my ($Port, $ConnectOneTimeout, $ConnectMinAttempts, $ConnectMinTimeout, $Timeout); @@ -240,6 +240,12 @@ while (@ARGV) set_cmd($arg); $LocalFilename = check_opt_val($arg, $LocalFilename); } + elsif ($arg eq "restart") + { + set_cmd($arg); + @Restart = @ARGV; + last; + } else { error("unknown command '$arg'\n"); @@ -359,6 +365,8 @@ if (defined $Usage) print " ping Makes sure the server is still alive.\n"; print " upgrade Replaces the server executable with the specified file and\n"; print " restarts it.\n"; + print " restart Replaces and restarts the server from the specified server file\n"; + print " and arguments or from the last command line if omitted.\n"; print " <hostname> Is the hostname of the server.\n"; print " --port <port> Use the specified port number instead of the default one.\n"; print " --connect-one-timeout <time> Specifies the timeout for one connection\n"; @@ -477,6 +485,10 @@ elsif ($Cmd eq "upgrade") { $Result = $TA->Upgrade($LocalFilename); } +elsif ($Cmd eq "restart") +{ + $Result = $TA->Restart(@Restart); +} elsif ($Cmd eq "getcwd") { $Result = $TA->GetCwd(); diff --git a/testbot/src/testagentd/platform.h b/testbot/src/testagentd/platform.h index 06882dedc..eb67c7afb 100644 --- a/testbot/src/testagentd/platform.h +++ b/testbot/src/testagentd/platform.h @@ -100,7 +100,7 @@ int platform_settime(uint64_t epoch, uint32_t leeway); * Returns non-zero if successful, which will let the caller know to complete * the current RPC and cleanly exit. Returns 0 otherwise. */ -int platform_upgrade(const char* tmpserver, char** argv); +int platform_upgrade(char* current_argv0, int argc, char** argv);
/* Called when the message has been dismissed by the user. */ diff --git a/testbot/src/testagentd/platform_unix.c b/testbot/src/testagentd/platform_unix.c index 560d1210e..a64bd8edb 100644 --- a/testbot/src/testagentd/platform_unix.c +++ b/testbot/src/testagentd/platform_unix.c @@ -240,28 +240,48 @@ int platform_settime(uint64_t epoch, uint32_t leeway) }
-int platform_upgrade(const char* tmpserver, char** argv) +int platform_upgrade(char* current_argv0, int argc, char** argv) { - static const char* oldserver = "testagentd.old"; + static const char* OLDSERVER = "testagentd.old"; + const char* oldserver = NULL; + struct stat stat_current, stat_argv0; int pipefds[2]; pid_t pid;
- if (rename(argv[0], oldserver)) + if (stat(current_argv0, &stat_current)) { - set_status(ST_ERROR, "unable to move the current server file out of the way: %s", strerror(errno)); + set_status(ST_ERROR, "could not stat '%s': %s", current_argv0, strerror(errno)); return 0; }
- if (rename(tmpserver, argv[0])) + if (stat(argv[0], &stat_argv0)) { - set_status(ST_ERROR, "unable to move the currentnew server file into place: %s", strerror(errno)); - rename(oldserver, argv[0]); + set_status(ST_ERROR, "could not stat '%s': %s", argv[0], strerror(errno)); return 0; } + + if (stat_current.st_dev != stat_argv0.st_dev || + stat_current.st_ino != stat_argv0.st_ino) + { + oldserver = OLDSERVER; + if (rename(current_argv0, oldserver)) + { + set_status(ST_ERROR, "unable to move the current server file out of the way: %s", strerror(errno)); + return 0; + } + + if (rename(argv[0], current_argv0)) + { + set_status(ST_ERROR, "unable to move the currentnew server file into place: %s", strerror(errno)); + rename(oldserver, argv[0]); + return 0; + } + } if (pipe(pipefds)) { set_status(ST_ERROR, "could not synchronize with the new process: %s", strerror(errno)); - rename(oldserver, argv[0]); + if (oldserver) + rename(oldserver, current_argv0); return 0; }
@@ -271,11 +291,13 @@ int platform_upgrade(const char* tmpserver, char** argv) set_status(ST_ERROR, "unable to start the new server: %s", strerror(errno)); close(pipefds[0]); close(pipefds[1]); - rename(oldserver, argv[0]); + if (oldserver) + rename(oldserver, current_argv0); return 0; }
- unlink(oldserver); + if (oldserver) + unlink(oldserver); if (!pid) { /* The child process is responsible for cleanly closing the connection @@ -292,7 +314,8 @@ int platform_upgrade(const char* tmpserver, char** argv) read(pipefds[0], &pid, 1); close(pipefds[0]);
- execvp(argv[0], argv); + argv[0] = current_argv0; + execvp(current_argv0, argv); return 1; }
diff --git a/testbot/src/testagentd/platform_windows.c b/testbot/src/testagentd/platform_windows.c index 0ba7d2cdf..d6f9ba96d 100644 --- a/testbot/src/testagentd/platform_windows.c +++ b/testbot/src/testagentd/platform_windows.c @@ -298,40 +298,212 @@ char* get_server_filename(int old) return strdup(filename); }
-int platform_upgrade(const char* tmpserver, char** argv) +/*********************************************************************** + * build_command_line + * + * Adapted from the Wine source. + * + * Build the command line of a process from the argv array. + * + * Note that it does NOT necessarily include the file name. + * Sometimes we don't even have any command line options at all. + * + * We must quote and escape characters so that the argv array can be rebuilt + * from the command line: + * - spaces and tabs must be quoted + * 'a b' -> '"a b"' + * - quotes must be escaped + * '"' -> '"' + * - if ''s are followed by a '"', they must be doubled and followed by '"', + * resulting in an odd number of '' followed by a '"' + * '"' -> '\"' + * '\"' -> '\\"' + * - ''s are followed by the closing '"' must be doubled, + * resulting in an even number of '' followed by a '"' + * ' ' -> '" \"' + * ' \' -> '" \\"' + * - ''s that are not followed by a '"' can be left as is + * 'a\b' == 'a\b' + * 'a\b' == 'a\b' + */ +static char *build_command_line(char **argv) +{ + int len; + char **arg; + char *p; + char* cmdline; + + len = 0; + for (arg = argv; *arg; arg++) + { + BOOL has_space; + int bcount; + char* a; + + has_space=FALSE; + bcount=0; + a=*arg; + if( !*a ) has_space=TRUE; + while (*a!='\0') { + if (*a=='\') { + bcount++; + } else { + if (*a==' ' || *a=='\t') { + has_space=TRUE; + } else if (*a=='"') { + /* doubling of '' preceding a '"', + * plus escaping of said '"' + */ + len+=2*bcount+1; + } + bcount=0; + } + a++; + } + len+=(a-*arg)+1 /* for the separating space */; + if (has_space) + len+=2+bcount; /* for the quotes and doubling of '' preceding the closing quote */ + } + + cmdline = malloc(len); + if (!cmdline) + return NULL; + + p = cmdline; + for (arg = argv; *arg; arg++) + { + BOOL has_space,has_quote; + char* a; + int bcount; + + /* Check for quotes and spaces in this argument */ + has_space=has_quote=FALSE; + a=*arg; + if( !*a ) has_space=TRUE; + while (*a!='\0') { + if (*a==' ' || *a=='\t') { + has_space=TRUE; + if (has_quote) + break; + } else if (*a=='"') { + has_quote=TRUE; + if (has_space) + break; + } + a++; + } + + /* Now transfer it to the command line */ + if (has_space) + *p++='"'; + if (has_quote || has_space) { + bcount=0; + a=*arg; + while (*a!='\0') { + if (*a=='\') { + *p++=*a; + bcount++; + } else { + if (*a=='"') { + int i; + + /* Double all the '\' preceding this '"', plus one */ + for (i=0;i<=bcount;i++) + *p++='\'; + *p++='"'; + } else { + *p++=*a; + } + bcount=0; + } + a++; + } + } else { + char* x = *arg; + while ((*p=*x++)) p++; + } + if (has_space) { + int i; + + /* Double all the '' preceding the closing quote */ + for (i=0;i<bcount;i++) + *p++='\'; + *p++='"'; + } + *p++=' '; + } + if (p > cmdline) + p--; /* remove last space */ + *p = '\0'; + + return cmdline; +} + +int platform_upgrade(char* current_argv0, int argc, char** argv) { - char *testagentd = NULL, *oldtestagentd = NULL; + char *testagentd = NULL, *oldtestagentd = NULL, *cmdline = NULL; + char full_argv0[MAX_PATH]; + char *tmp_argv0; + DWORD rc; STARTUPINFO si; PROCESS_INFORMATION pi; int success = 0;
testagentd = get_server_filename(0); - oldtestagentd = get_server_filename(1); - if (!testagentd || !oldtestagentd) + if (!testagentd) { - set_status(ST_ERROR, "unable to get the process filenames (le=%lu)", GetLastError()); + set_status(ST_ERROR, "unable to get the process filename (le=%lu)", GetLastError()); goto done; }
- if (!MoveFile(testagentd, oldtestagentd)) + rc = GetFullPathNameA(argv[0], sizeof(full_argv0), full_argv0, NULL); + if (rc == 0 || rc > sizeof(full_argv0)) { - set_status(ST_ERROR, "unable to move the current server file out of the way (le=%lu)", GetLastError()); + set_status(ST_ERROR, "could not get the full path for '%s' (le=%lu)", argv[0], GetLastError()); goto done; } - if (!MoveFile(tmpserver, testagentd)) + + if (strcmp(testagentd, full_argv0)) + { + oldtestagentd = get_server_filename(1); + if (!oldtestagentd) + { + set_status(ST_ERROR, "unable to get the backup filename (le=%lu)", GetLastError()); + goto done; + } + if (!MoveFile(testagentd, oldtestagentd)) + { + set_status(ST_ERROR, "unable to move the current server file out of the way (le=%lu)", GetLastError()); + goto done; + } + if (!MoveFile(argv[0], testagentd)) + { + set_status(ST_ERROR, "unable to move the new server file in place (le=%lu)", GetLastError()); + MoveFile(oldtestagentd, testagentd); + goto done; + } + } + + tmp_argv0 = argv[0]; + argv[0] = testagentd; + cmdline = build_command_line(argv); + argv[0] = tmp_argv0; + if (!cmdline) { - set_status(ST_ERROR, "unable to move the new server file in place (le=%lu)", GetLastError()); - MoveFile(oldtestagentd, testagentd); + set_status(ST_ERROR, "unable to build the new command line"); + if (oldtestagentd) + MoveFile(oldtestagentd, testagentd); goto done; }
memset(&si, 0, sizeof(si)); si.cb = sizeof(si); - if (!CreateProcessA(testagentd, GetCommandLineA(), NULL, NULL, TRUE, + if (!CreateProcessA(testagentd, cmdline, NULL, NULL, TRUE, CREATE_NEW_CONSOLE, NULL, NULL, &si, &pi)) { - set_status(ST_ERROR, "could not run '%s': %lu", GetCommandLineA(), GetLastError()); - MoveFile(oldtestagentd, testagentd); + set_status(ST_ERROR, "could not run '%s': %lu", cmdline, GetLastError()); + if (oldtestagentd) + MoveFile(oldtestagentd, testagentd); goto done; }
@@ -341,6 +513,7 @@ int platform_upgrade(const char* tmpserver, char** argv) done: free(testagentd); free(oldtestagentd); + free(cmdline); return success; }
diff --git a/testbot/src/testagentd/testagentd.c b/testbot/src/testagentd/testagentd.c index 5fd136721..eb93aa203 100644 --- a/testbot/src/testagentd/testagentd.c +++ b/testbot/src/testagentd/testagentd.c @@ -39,8 +39,10 @@ * 1.5: Add support for upgrading the server. * 1.6: Add the rmchildproc and getcwd RPCs. * 1.7: Add --show-restarts and the setproperty RPC. + * 1.8: Add the restart RPC, server.arg* properties, fix upgrades if >1 + * server. */ -#define PROTOCOL_VERSION "testagentd 1.7" +#define PROTOCOL_VERSION "testagentd 1.8"
#define BLOCK_SIZE 65536
@@ -94,6 +96,7 @@ enum rpc_ids_t RPCID_RMCHILDPROC, RPCID_GETCWD, RPCID_SETPROPERTY, + RPCID_RESTART, };
/* This is the RPC currently being processed */ @@ -117,6 +120,7 @@ static const char* rpc_name(uint32_t id) "rmchildproc", "getcwd", "setproperty", + "restart", };
if (id < sizeof(names) / sizeof(*names)) @@ -1084,6 +1088,63 @@ static void do_setproperty(SOCKET client) } }
+static void do_restart(SOCKET client) +{ + uint32_t argc, i; + char** argv; + int success = 1; + + if (!recv_list_size(client, &argc)) + { + send_error(client); + return; + } + if (argc < 1) + { + set_status(ST_ERROR, "expected 1 argument or more (got %u)", argc); + send_error(client); + return; + } + /* Allocate an extra entry for the trailing NULL pointer */ + argv = malloc((argc + 1) * sizeof(*argv)); + if (argv) + { + memset(argv, 0, (argc + 1) * sizeof(*argv)); + for (i = 0; i < argc; i++) + { + if (!recv_string(client, &argv[i])) + { + success = 0; + break; + } + } + } + else + { + set_status(ST_ERROR, "malloc() failed: %s", strerror(errno)); + skip_entries(client, argc); + success = 0; + } + + if (success) + success = platform_upgrade(server_argv[0], argc, argv); + + /* Free all the memory */ + for (i = 0; i < argc; i++) + free(argv[i]); + free(argv); + + if (success) + { + send_list_size(client, 0); + /* Tell the main loop to quit */ + broken = 1; + quit = 1; + } + else + send_error(client); +} + static void do_upgrade(SOCKET client) { static const char *filename = "testagentd.tmp"; @@ -1112,7 +1173,12 @@ static void do_upgrade(SOCKET client) }
if (success) - success = platform_upgrade(filename, server_argv); + { + char* current_argv0 = server_argv[0]; + server_argv[0] = (char*)filename; + success = platform_upgrade(current_argv0, server_argc, server_argv); + server_argv[0] = current_argv0; + }
if (success) { @@ -1231,6 +1297,8 @@ static void process_rpc(SOCKET client) case RPCID_SETPROPERTY: do_setproperty(client); break; + case RPCID_RESTART: + do_restart(client); default: do_unknown(client, rpcid); }