[PATCH 2/2] testbot/TestAgent: Add the restart RPC.
Francois Gouget
fgouget at codeweavers.com
Tue Sep 24 04:06:43 CDT 2019
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 at 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);
}
--
2.20.1
More information about the wine-devel
mailing list