Skip to content

Commit

Permalink
sockets: Limit SocketAddressLegacy to external interfaces
Browse files Browse the repository at this point in the history
SocketAddressLegacy is a simple union, and simple unions are awkward:
they have their variant members wrapped in a "data" object on the
wire, and require additional indirections in C.  SocketAddress is the
equivalent flat union.  Convert all users of SocketAddressLegacy to
SocketAddress, except for existing external interfaces.

See also commit fce5d53..9445673 and 85a82e8..c5f1ae3.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1493192202-3184-7-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Minor editing accident fixed, commit message and a comment tweaked]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
  • Loading branch information
Markus Armbruster committed May 9, 2017
1 parent 62cf396 commit bd269eb
Show file tree
Hide file tree
Showing 24 changed files with 440 additions and 406 deletions.
4 changes: 1 addition & 3 deletions block/nbd.c
Original file line number Diff line number Diff line change
Expand Up @@ -306,10 +306,9 @@ NBDClientSession *nbd_get_client_session(BlockDriverState *bs)
return &s->client;
}

static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr_flat,
static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr,
Error **errp)
{
SocketAddressLegacy *saddr = socket_address_crumple(saddr_flat);
QIOChannelSocket *sioc;
Error *local_err = NULL;

Expand All @@ -319,7 +318,6 @@ static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr_flat,
qio_channel_socket_connect_sync(sioc,
saddr,
&local_err);
qapi_free_SocketAddressLegacy(saddr);
if (local_err) {
object_unref(OBJECT(sioc));
error_propagate(errp, local_err);
Expand Down
32 changes: 13 additions & 19 deletions block/sheepdog.c
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ struct BDRVSheepdogState {
uint32_t cache_flags;
bool discard_supported;

SocketAddressLegacy *addr;
SocketAddress *addr;
int fd;

CoMutex lock;
Expand Down Expand Up @@ -530,32 +530,29 @@ static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
QLIST_INSERT_HEAD(&s->inflight_aiocb_head, acb, aiocb_siblings);
}

static SocketAddressLegacy *sd_socket_address(const char *path,
static SocketAddress *sd_socket_address(const char *path,
const char *host, const char *port)
{
SocketAddressLegacy *addr = g_new0(SocketAddressLegacy, 1);
SocketAddress *addr = g_new0(SocketAddress, 1);

if (path) {
addr->type = SOCKET_ADDRESS_LEGACY_KIND_UNIX;
addr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
addr->u.q_unix.data->path = g_strdup(path);
addr->type = SOCKET_ADDRESS_TYPE_UNIX;
addr->u.q_unix.path = g_strdup(path);
} else {
addr->type = SOCKET_ADDRESS_LEGACY_KIND_INET;
addr->u.inet.data = g_new0(InetSocketAddress, 1);
addr->u.inet.data->host = g_strdup(host ?: SD_DEFAULT_ADDR);
addr->u.inet.data->port = g_strdup(port ?: stringify(SD_DEFAULT_PORT));
addr->type = SOCKET_ADDRESS_TYPE_INET;
addr->u.inet.host = g_strdup(host ?: SD_DEFAULT_ADDR);
addr->u.inet.port = g_strdup(port ?: stringify(SD_DEFAULT_PORT));
}

return addr;
}

static SocketAddressLegacy *sd_server_config(QDict *options, Error **errp)
static SocketAddress *sd_server_config(QDict *options, Error **errp)
{
QDict *server = NULL;
QObject *crumpled_server = NULL;
Visitor *iv = NULL;
SocketAddress *saddr_flat = NULL;
SocketAddressLegacy *saddr = NULL;
SocketAddress *saddr = NULL;
Error *local_err = NULL;

qdict_extract_subqdict(options, &server, "server.");
Expand All @@ -574,16 +571,13 @@ static SocketAddressLegacy *sd_server_config(QDict *options, Error **errp)
* visitor expects the former.
*/
iv = qobject_input_visitor_new(crumpled_server);
visit_type_SocketAddress(iv, NULL, &saddr_flat, &local_err);
visit_type_SocketAddress(iv, NULL, &saddr, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto done;
}

saddr = socket_address_crumple(saddr_flat);

done:
qapi_free_SocketAddress(saddr_flat);
visit_free(iv);
qobject_decref(crumpled_server);
QDECREF(server);
Expand All @@ -597,7 +591,7 @@ static int connect_to_sdog(BDRVSheepdogState *s, Error **errp)

fd = socket_connect(s->addr, NULL, NULL, errp);

if (s->addr->type == SOCKET_ADDRESS_LEGACY_KIND_INET && fd >= 0) {
if (s->addr->type == SOCKET_ADDRESS_TYPE_INET && fd >= 0) {
int ret = socket_set_nodelay(fd);
if (ret < 0) {
error_report("%s", strerror(errno));
Expand Down Expand Up @@ -2149,7 +2143,7 @@ static void sd_close(BlockDriverState *bs)
aio_set_fd_handler(bdrv_get_aio_context(bs), s->fd,
false, NULL, NULL, NULL, NULL);
closesocket(s->fd);
qapi_free_SocketAddressLegacy(s->addr);
qapi_free_SocketAddress(s->addr);
}

static int64_t sd_getlength(BlockDriverState *bs)
Expand Down
21 changes: 15 additions & 6 deletions blockdev-nbd.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,8 @@ static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp)
}


void qmp_nbd_server_start(SocketAddressLegacy *addr,
bool has_tls_creds, const char *tls_creds,
Error **errp)
void nbd_server_start(SocketAddress *addr, const char *tls_creds,
Error **errp)
{
if (nbd_server) {
error_setg(errp, "NBD server already running");
Expand All @@ -118,14 +117,14 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
goto error;
}

if (has_tls_creds) {
if (tls_creds) {
nbd_server->tlscreds = nbd_get_tls_creds(tls_creds, errp);
if (!nbd_server->tlscreds) {
goto error;
}

/* TODO SOCKET_ADDRESS_LEGACY_KIND_FD where fd has AF_INET or AF_INET6 */
if (addr->type != SOCKET_ADDRESS_LEGACY_KIND_INET) {
/* TODO SOCKET_ADDRESS_TYPE_FD where fd has AF_INET or AF_INET6 */
if (addr->type != SOCKET_ADDRESS_TYPE_INET) {
error_setg(errp, "TLS is only supported with IPv4/IPv6");
goto error;
}
Expand All @@ -145,6 +144,16 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
nbd_server = NULL;
}

void qmp_nbd_server_start(SocketAddressLegacy *addr,
bool has_tls_creds, const char *tls_creds,
Error **errp)
{
SocketAddress *addr_flat = socket_address_flatten(addr);

nbd_server_start(addr_flat, tls_creds, errp);
qapi_free_SocketAddress(addr_flat);
}

void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
Error **errp)
{
Expand Down
40 changes: 20 additions & 20 deletions chardev/char-socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ typedef struct {
int *write_msgfds;
size_t write_msgfds_num;

SocketAddressLegacy *addr;
SocketAddress *addr;
bool is_listen;
bool is_telnet;
bool is_tn3270;
Expand Down Expand Up @@ -356,30 +356,30 @@ static void tcp_chr_free_connection(Chardev *chr)
s->connected = 0;
}

static char *SocketAddress_to_str(const char *prefix, SocketAddressLegacy *addr,
static char *SocketAddress_to_str(const char *prefix, SocketAddress *addr,
bool is_listen, bool is_telnet)
{
switch (addr->type) {
case SOCKET_ADDRESS_LEGACY_KIND_INET:
case SOCKET_ADDRESS_TYPE_INET:
return g_strdup_printf("%s%s:%s:%s%s", prefix,
is_telnet ? "telnet" : "tcp",
addr->u.inet.data->host,
addr->u.inet.data->port,
addr->u.inet.host,
addr->u.inet.port,
is_listen ? ",server" : "");
break;
case SOCKET_ADDRESS_LEGACY_KIND_UNIX:
case SOCKET_ADDRESS_TYPE_UNIX:
return g_strdup_printf("%sunix:%s%s", prefix,
addr->u.q_unix.data->path,
addr->u.q_unix.path,
is_listen ? ",server" : "");
break;
case SOCKET_ADDRESS_LEGACY_KIND_FD:
return g_strdup_printf("%sfd:%s%s", prefix, addr->u.fd.data->str,
case SOCKET_ADDRESS_TYPE_FD:
return g_strdup_printf("%sfd:%s%s", prefix, addr->u.fd.str,
is_listen ? ",server" : "");
break;
case SOCKET_ADDRESS_LEGACY_KIND_VSOCK:
case SOCKET_ADDRESS_TYPE_VSOCK:
return g_strdup_printf("%svsock:%s:%s", prefix,
addr->u.vsock.data->cid,
addr->u.vsock.data->port);
addr->u.vsock.cid,
addr->u.vsock.port);
default:
abort();
}
Expand Down Expand Up @@ -648,7 +648,7 @@ static void tcp_chr_tls_init(Chardev *chr)
} else {
tioc = qio_channel_tls_new_client(
s->ioc, s->tls_creds,
s->addr->u.inet.data->host,
s->addr->u.inet.host,
&err);
}
if (tioc == NULL) {
Expand Down Expand Up @@ -796,7 +796,7 @@ static void char_socket_finalize(Object *obj)
g_source_remove(s->reconnect_timer);
s->reconnect_timer = 0;
}
qapi_free_SocketAddressLegacy(s->addr);
qapi_free_SocketAddress(s->addr);
if (s->listen_tag) {
g_source_remove(s->listen_tag);
s->listen_tag = 0;
Expand Down Expand Up @@ -859,14 +859,14 @@ static void qmp_chardev_open_socket(Chardev *chr,
{
SocketChardev *s = SOCKET_CHARDEV(chr);
ChardevSocket *sock = backend->u.socket.data;
SocketAddressLegacy *addr = sock->addr;
bool do_nodelay = sock->has_nodelay ? sock->nodelay : false;
bool is_listen = sock->has_server ? sock->server : true;
bool is_telnet = sock->has_telnet ? sock->telnet : false;
bool is_tn3270 = sock->has_tn3270 ? sock->tn3270 : false;
bool is_waitconnect = sock->has_wait ? sock->wait : false;
int64_t reconnect = sock->has_reconnect ? sock->reconnect : 0;
QIOChannelSocket *sioc = NULL;
SocketAddress *addr;

s->is_listen = is_listen;
s->is_telnet = is_telnet;
Expand Down Expand Up @@ -905,11 +905,11 @@ static void qmp_chardev_open_socket(Chardev *chr,
}
}

s->addr = QAPI_CLONE(SocketAddressLegacy, sock->addr);
s->addr = addr = socket_address_flatten(sock->addr);

qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_RECONNECTABLE);
/* TODO SOCKET_ADDRESS_FD where fd has AF_UNIX */
if (addr->type == SOCKET_ADDRESS_LEGACY_KIND_UNIX) {
if (addr->type == SOCKET_ADDRESS_TYPE_UNIX) {
qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_FD_PASS);
}

Expand Down Expand Up @@ -945,7 +945,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
goto error;
}

qapi_free_SocketAddressLegacy(s->addr);
qapi_free_SocketAddress(s->addr);
s->addr = socket_local_address(sioc->fd, errp);
update_disconnected_filename(s);

Expand Down Expand Up @@ -1051,7 +1051,7 @@ char_socket_get_addr(Object *obj, Visitor *v, const char *name,
{
SocketChardev *s = SOCKET_CHARDEV(obj);

visit_type_SocketAddressLegacy(v, name, &s->addr, errp);
visit_type_SocketAddress(v, name, &s->addr, errp);
}

static bool
Expand All @@ -1078,7 +1078,7 @@ static void char_socket_class_init(ObjectClass *oc, void *data)
cc->chr_add_watch = tcp_chr_add_watch;
cc->chr_update_read_handler = tcp_chr_update_read_handler;

object_class_property_add(oc, "addr", "SocketAddressLegacy",
object_class_property_add(oc, "addr", "SocketAddress",
char_socket_get_addr, NULL,
NULL, NULL, &error_abort);

Expand Down
10 changes: 7 additions & 3 deletions chardev/char-udp.c
Original file line number Diff line number Diff line change
Expand Up @@ -191,13 +191,17 @@ static void qmp_chardev_open_udp(Chardev *chr,
Error **errp)
{
ChardevUdp *udp = backend->u.udp.data;
SocketAddress *local_addr = socket_address_flatten(udp->local);
SocketAddress *remote_addr = socket_address_flatten(udp->remote);
QIOChannelSocket *sioc = qio_channel_socket_new();
char *name;
UdpChardev *s = UDP_CHARDEV(chr);
int ret;

if (qio_channel_socket_dgram_sync(sioc,
udp->local, udp->remote,
errp) < 0) {
ret = qio_channel_socket_dgram_sync(sioc, local_addr, remote_addr, errp);
qapi_free_SocketAddress(local_addr);
qapi_free_SocketAddress(remote_addr);
if (ret < 0) {
object_unref(OBJECT(sioc));
return;
}
Expand Down
7 changes: 4 additions & 3 deletions hmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "qapi-visit.h"
#include "qom/object_interfaces.h"
#include "ui/console.h"
#include "block/nbd.h"
#include "block/qapi.h"
#include "qemu-io.h"
#include "qemu/cutils.h"
Expand Down Expand Up @@ -2108,7 +2109,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
bool all = qdict_get_try_bool(qdict, "all", false);
Error *local_err = NULL;
BlockInfoList *block_list, *info;
SocketAddressLegacy *addr;
SocketAddress *addr;

if (writable && !all) {
error_setg(&local_err, "-w only valid together with -a");
Expand All @@ -2121,8 +2122,8 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
goto exit;
}

qmp_nbd_server_start(addr, false, NULL, &local_err);
qapi_free_SocketAddressLegacy(addr);
nbd_server_start(addr, NULL, &local_err);
qapi_free_SocketAddress(addr);
if (local_err != NULL) {
goto exit;
}
Expand Down
3 changes: 3 additions & 0 deletions include/block/nbd.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,4 +164,7 @@ void nbd_client_new(NBDExport *exp,
void nbd_client_get(NBDClient *client);
void nbd_client_put(NBDClient *client);

void nbd_server_start(SocketAddress *addr, const char *tls_creds,
Error **errp);

#endif
Loading

0 comments on commit bd269eb

Please sign in to comment.