Page MenuHomeFreeBSD
Differential D26542 Diff 78369 sbin/ggate/ggatec/ggatec.c

Changeset View

Standalone View

View Options

sbin/ggate/ggatec/ggatec.c

Show All 25 Lines
* SUCH DAMAGE. * SUCH DAMAGE.
* *
* $FreeBSD$ * $FreeBSD$
*/ */
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <stdint.h> #include <stdint.h>
#include <stdbool.h>
#include <fcntl.h> #include <fcntl.h>
#include <unistd.h> #include <unistd.h>
#include <string.h> #include <string.h>
#include <ctype.h> #include <ctype.h>
#include <inttypes.h> #include <inttypes.h>
#include <libgen.h> #include <libgen.h>
#include <pthread.h> #include <pthread.h>
#include <signal.h> #include <signal.h>
Show All 27 Lines
static unsigned port = G_GATE_PORT; static unsigned port = G_GATE_PORT;
static off_t mediasize; static off_t mediasize;
static unsigned sectorsize = 0; static unsigned sectorsize = 0;
static unsigned timeout = G_GATE_TIMEOUT; static unsigned timeout = G_GATE_TIMEOUT;
static int sendfd, recvfd; static int sendfd, recvfd;
static uint32_t token; static uint32_t token;
static pthread_t sendtd, recvtd; static pthread_t sendtd, recvtd;
static int reconnect; static int reconnect;
static int protocol_version = 2;
markjUnsubmitted
Done
Inline Actions

This is rather confusing. 2 apparently means that versions 0 and 1 are acceptable. It might make more sense to have a bitmask of permitted versions.

markj: This is rather confusing. 2 apparently means that versions 0 and 1 are acceptable. It might…
static bool g_gatec_connect_v0(void);
static bool g_gatec_connect_v1(void);
static void static void
usage(void) usage(void)
{ {
fprintf(stderr, "geom gate protocol version %d\n", G_GATE_VERSION);
fprintf(stderr, "usage: %s create [-nv] [-o <ro|wo|rw>] [-p port] " fprintf(stderr, "usage: %s create [-0|-1] [-nv] [-o <ro|wo|rw>] [-p port] "
"[-q queue_size] [-R rcvbuf] [-S sndbuf] [-s sectorsize] " "[-q queue_size] [-R rcvbuf] [-S sndbuf] [-s sectorsize] "
"[-t timeout] [-u unit] <host> <path>\n", getprogname()); "[-t timeout] [-u unit] <host> <path>\n", getprogname());
fprintf(stderr, " %s rescue [-nv] [-o <ro|wo|rw>] [-p port] " fprintf(stderr, " %s rescue [-0|-1] [-nv] [-o <ro|wo|rw>] [-p port] "
"[-R rcvbuf] [-S sndbuf] <-u unit> <host> <path>\n", getprogname()); "[-R rcvbuf] [-S sndbuf] <-u unit> <host> <path>\n", getprogname());
fprintf(stderr, " %s destroy [-f] <-u unit>\n", getprogname()); fprintf(stderr, " %s destroy [-f] <-u unit>\n", getprogname());
fprintf(stderr, " %s list [-v] [-u unit]\n", getprogname()); fprintf(stderr, " %s list [-v] [-u unit]\n", getprogname());
exit(EXIT_FAILURE); exit(EXIT_FAILURE);
} }
static void * static void *
send_thread(void *arg __unused) send_thread(void *arg __unused)
▲ Show 20 Lines • Show All 184 Lines • ▼ Show 20 Lines if (connect(sfd, (struct sockaddr *)&serv, sizeof(serv)) == -1) {
return (-1); return (-1);
} }
g_gate_log(LOG_INFO, "Connected to the server: %s:%d.", host, port); g_gate_log(LOG_INFO, "Connected to the server: %s:%d.", host, port);
/* /*
* Create and send version packet. * Create and send version packet.
*/ */
g_gate_log(LOG_DEBUG, "Sending version packet.");
assert(strlen(GGATE_MAGIC) == sizeof(ver.gv_magic)); assert(strlen(GGATE_MAGIC) == sizeof(ver.gv_magic));
bcopy(GGATE_MAGIC, ver.gv_magic, sizeof(ver.gv_magic)); bcopy(GGATE_MAGIC, ver.gv_magic, sizeof(ver.gv_magic));
ver.gv_version = GGATE_VERSION; ver.gv_version = dir ? GGATE_VERSION0 : GGATE_VERSION1;
g_gate_log(LOG_DEBUG, "Sending version %u packet.",
markjUnsubmitted
Done
Inline Actions

This can just be %u.

markj: This can just be %u.
ver.gv_version);
markjUnsubmitted
Done
Inline Actions

Extra whitespace after "?".

markj: Extra whitespace after "?".
ver.gv_error = 0; ver.gv_error = 0;
g_gate_swap2n_version(&ver); g_gate_swap2n_version(&ver);
if (g_gate_send(sfd, &ver, sizeof(ver), MSG_NOSIGNAL) == -1) { if (g_gate_send(sfd, &ver, sizeof(ver), MSG_NOSIGNAL) == -1) {
g_gate_log(LOG_DEBUG, "Error while sending version packet: %s.", g_gate_log(LOG_DEBUG, "Error while sending version packet: %s.",
strerror(errno)); strerror(errno));
close(sfd); close(sfd);
return (-1); return (-1);
} }
▲ Show 20 Lines • Show All 67 Lines • ▼ Show 20 Lines if (g_gate_verbose > 0)
return; return;
if (daemon(0, 0) == 0) if (daemon(0, 0) == 0)
return; return;
if (action == CREATE) if (action == CREATE)
g_gate_destroy(unit, 1); g_gate_destroy(unit, 1);
err(EXIT_FAILURE, "Cannot daemonize"); err(EXIT_FAILURE, "Cannot daemonize");
} }
static int static bool
g_gatec_connect(void) g_gatec_connect_v0(void)
{ {
token = arc4random(); token = arc4random();
/* /*
* Our receive descriptor is connected to the send descriptor on the * Our receive descriptor is connected to the send descriptor on the
* server side. * server side.
*/ */
recvfd = handshake(GGATE_FLAG_SEND); recvfd = handshake(GGATE_FLAG_SEND);
if (recvfd == -1) if (recvfd == -1)
return (0); return (false);
/* /*
* Our send descriptor is connected to the receive descriptor on the * Our send descriptor is connected to the receive descriptor on the
* server side. * server side.
*/ */
sendfd = handshake(GGATE_FLAG_RECV); sendfd = handshake(GGATE_FLAG_RECV);
if (sendfd == -1) if (sendfd == -1)
return (0); return (false);
return (1); return (true);
} }
static bool
g_gatec_connect_v1(void)
{
token = arc4random();
recvfd = sendfd = handshake( 0 );
markjUnsubmitted
Done
Inline Actions

Extra whitespace between the parentheses.

markj: Extra whitespace between the parentheses.
if (sendfd == -1)
return (false);
return (true);
}
static bool
g_gatec_connect(void)
{
if (protocol_version >= 1) {
if (g_gatec_connect_v1()) {
protocol_version = 1;
peterjUnsubmitted
Not Done
Inline Actions

The 3 try_version assignments in g_gatec_connect() break the automatic link recovery: If ggatec detects a synchronisation error with, or loses connection to, its peer ggated process then it will close the existing socket(s) and call g_gatec_connect() to establish a new connection. The two "try_version = 1 << GGATE_VERSION_n" assignments mean that it will never try a different protocol - which could be necessary if ggated restarted. Worse, if both connection attempts fail then try_version is set to 0 and ggatec can never recover.

try_version is never read outside this function so the assignments serve no purpose.

peterj: The 3 try_version assignments in g_gatec_connect() break the automatic link recovery: If ggatec…
return (true);
markjUnsubmitted
Done
Inline Actions

Can just be if (g_gatec_connect_v1()) { ...

markj: Can just be `if (g_gatec_connect_v1()) { ...`
}
else if (protocol_version == 1)
protocol_version = -1;
}
markjUnsubmitted
Done
Inline Actions

The style is wrong here, see style(9).

markj: The style is wrong here, see style(9).
if (protocol_version >= 0) {
if (g_gatec_connect_v0()) {
protocol_version = 0;
return (true);
}
}
protocol_version = -1;
return (false);
}
static void static void
g_gatec_start(void) g_gatec_start(void)
{ {
int error; int error;
reconnect = 0; reconnect = 0;
error = pthread_create(&recvtd, NULL, recv_thread, NULL); error = pthread_create(&recvtd, NULL, recv_thread, NULL);
if (error != 0) { if (error != 0) {
g_gate_destroy(unit, 1); g_gate_destroy(unit, 1);
g_gate_xlog("pthread_create(recv_thread): %s.", g_gate_xlog("pthread_create(recv_thread): %s.",
strerror(error)); strerror(error));
} }
sendtd = pthread_self(); sendtd = pthread_self();
send_thread(NULL); send_thread(NULL);
/* Disconnected. */ /* Disconnected. */
close(sendfd); close(sendfd);
if (sendfd != recvfd)
close(recvfd); close(recvfd);
} }
static void static void
signop(int sig __unused) signop(int sig __unused)
{ {
/* Do nothing. */ /* Do nothing. */
} }
▲ Show 20 Lines • Show All 85 Lines • ▼ Show 20 Lines else if (strcasecmp(argv[1], "rescue") == 0)
action = RESCUE; action = RESCUE;
else else
usage(); usage();
argc -= 1; argc -= 1;
argv += 1; argv += 1;
for (;;) { for (;;) {
int ch; int ch;
ch = getopt(argc, argv, "fno:p:q:R:S:s:t:u:v"); ch = getopt(argc, argv, "01fno:p:q:R:S:s:t:u:v");
if (ch == -1) if (ch == -1)
break; break;
switch (ch) { switch (ch) {
case '0':
case '1':
protocol_version = ch - '0';
markjUnsubmitted
Done
Inline Actions

This looks wrong. Shouldn't it be protocol_version = ch - '0'?

markj: This looks wrong. Shouldn't it be `protocol_version = ch - '0'`?
break;
case 'f': case 'f':
if (action != DESTROY) if (action != DESTROY)
usage(); usage();
force = 1; force = 1;
break; break;
case 'n': case 'n':
if (action != CREATE && action != RESCUE) if (action != CREATE && action != RESCUE)
usage(); usage();
▲ Show 20 Lines • Show All 123 Lines • Show Last 20 Lines