src/conf/domain_conf.c | 25 ++++++-- src/libxl/libxl_conf.c | 2 +- src/qemu/qemu_block.c | 2 +- src/qemu/qemu_command.c | 28 ++------- src/qemu/qemu_parse_command.c | 33 +++++++--- src/storage/storage_backend_gluster.c | 17 ++--- src/storage/storage_driver.c | 7 +-- src/util/virstoragefile.c | 113 +++++++++++++++++++++------------- src/util/virstoragefile.h | 4 +- src/xenconfig/xen_xl.c | 2 +- 10 files changed, 130 insertions(+), 103 deletions(-)
Currently, @port is type of string. Well, that's overkill and
waste of memory. Port is always an integer. Use it as such.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/conf/domain_conf.c | 25 ++++++--
src/libxl/libxl_conf.c | 2 +-
src/qemu/qemu_block.c | 2 +-
src/qemu/qemu_command.c | 28 ++-------
src/qemu/qemu_parse_command.c | 33 +++++++---
src/storage/storage_backend_gluster.c | 17 ++---
src/storage/storage_driver.c | 7 +--
src/util/virstoragefile.c | 113 +++++++++++++++++++++-------------
src/util/virstoragefile.h | 4 +-
src/xenconfig/xen_xl.c | 2 +-
10 files changed, 130 insertions(+), 103 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9320794de..e8fdaa36f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6437,6 +6437,7 @@ virDomainStorageHostParse(xmlNodePtr node,
int ret = -1;
xmlNodePtr child;
char *transport = NULL;
+ char *port = NULL;
virStorageNetHostDef host;
memset(&host, 0, sizeof(host));
@@ -6486,7 +6487,15 @@ virDomainStorageHostParse(xmlNodePtr node,
goto cleanup;
}
- host.port = virXMLPropString(child, "port");
+ port = virXMLPropString(child, "port");
+ if (port &&
+ (virStrToLong_i(port, NULL, 10, &host.port) < 0 ||
+ host.port < 0)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("failed to parse port number '%s'"),
+ port);
+ goto cleanup;
+ }
}
if (VIR_APPEND_ELEMENT(*hosts, *nhosts, host) < 0)
@@ -6499,6 +6508,7 @@ virDomainStorageHostParse(xmlNodePtr node,
cleanup:
virStorageNetHostDefClear(&host);
VIR_FREE(transport);
+ VIR_FREE(port);
return ret;
}
@@ -7882,8 +7892,8 @@ virDomainDiskSourceParse(xmlNodePtr node,
if (virDomainStorageHostParse(node, &src->hosts, &src->nhosts) < 0)
goto cleanup;
- if (virStorageSourceNetworkAssignDefaultPorts(src) < 0)
- goto cleanup;
+ virStorageSourceNetworkAssignDefaultPorts(src);
+
break;
case VIR_STORAGE_TYPE_VOLUME:
if (virDomainDiskSourcePoolDefParse(node, &src->srcpool) < 0)
@@ -14909,7 +14919,7 @@ virDomainHostdevMatchSubsysSCSIiSCSI(virDomainHostdevDefPtr first,
&second->source.subsys.u.scsi.u.iscsi;
if (STREQ(first_iscsisrc->hosts[0].name, second_iscsisrc->hosts[0].name) &&
- STREQ(first_iscsisrc->hosts[0].port, second_iscsisrc->hosts[0].port) &&
+ first_iscsisrc->hosts[0].port == second_iscsisrc->hosts[0].port &&
STREQ(first_iscsisrc->path, second_iscsisrc->path))
return 1;
return 0;
@@ -21408,7 +21418,9 @@ virDomainDiskSourceFormatNetwork(virBufferPtr buf,
for (n = 0; n < src->nhosts; n++) {
virBufferAddLit(buf, "<host");
virBufferEscapeString(buf, " name='%s'", src->hosts[n].name);
- virBufferEscapeString(buf, " port='%s'", src->hosts[n].port);
+
+ if (src->hosts[n].port)
+ virBufferAsprintf(buf, " port='%d'", src->hosts[n].port);
if (src->hosts[n].transport)
virBufferAsprintf(buf, " transport='%s'",
@@ -22255,7 +22267,8 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
virBufferAddLit(buf, "<host");
virBufferEscapeString(buf, " name='%s'", iscsisrc->hosts[0].name);
- virBufferEscapeString(buf, " port='%s'", iscsisrc->hosts[0].port);
+ if (iscsisrc->hosts[0].port)
+ virBufferAsprintf(buf, " port='%d'", iscsisrc->hosts[0].port);
virBufferAddLit(buf, "/>\n");
} else {
virBufferAsprintf(buf, "<adapter name='%s'/>\n",
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index a85bc71d2..1b20fb906 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -706,7 +706,7 @@ libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src,
virBufferAsprintf(&buf, "%s", src->hosts[i].name);
if (src->hosts[i].port)
- virBufferAsprintf(&buf, "\\:%s", src->hosts[i].port);
+ virBufferAsprintf(&buf, "\\:%d", src->hosts[i].port);
}
}
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 93124c5ba..3d64528a8 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -465,7 +465,7 @@ qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src,
if (virJSONValueObjectCreate(&server,
"s:type", transport,
"s:host", host->name,
- "s:port", host->port,
+ "i:port", host->port,
NULL) < 0)
goto cleanup;
break;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6ac26af3e..b4a99c651 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -491,22 +491,6 @@ qemuSafeSerialParamValue(const char *value)
}
-static int
-qemuNetworkDriveGetPort(const char *port)
-{
- int ret = 0;
-
- if (virStrToLong_i(port, NULL, 10, &ret) < 0 || ret < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to parse port number '%s'"),
- port);
- return -1;
- }
-
- return ret;
-}
-
-
/**
* qemuBuildSecretInfoProps:
* @secinfo: pointer to the secret info object
@@ -825,8 +809,7 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
goto cleanup;
if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) {
- if ((uri->port = qemuNetworkDriveGetPort(src->hosts->port)) < 0)
- goto cleanup;
+ uri->port = src->hosts->port;
if (VIR_STRDUP(uri->scheme,
virStorageNetProtocolTypeToString(src->protocol)) < 0)
@@ -897,8 +880,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
switch (src->hosts->transport) {
case VIR_STORAGE_NET_HOST_TRANS_TCP:
- virBufferStrcat(&buf, src->hosts->name, ":",
- src->hosts->port, NULL);
+ virBufferAsprintf(&buf, "%s:%d", src->hosts->name, src->hosts->port);
break;
case VIR_STORAGE_NET_HOST_TRANS_UNIX:
@@ -953,9 +935,9 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
if (virAsprintf(&ret, "sheepdog:%s", src->path) < 0)
goto cleanup;
} else if (src->nhosts == 1) {
- if (virAsprintf(&ret, "sheepdog:%s:%s:%s",
+ if (virAsprintf(&ret, "sheepdog:%s:%d:%s",
src->hosts->name,
- src->hosts->port ? src->hosts->port : "7000",
+ src->hosts->port ? src->hosts->port : 7000,
src->path) < 0)
goto cleanup;
} else {
@@ -996,7 +978,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
virBufferAsprintf(&buf, "%s", src->hosts[i].name);
if (src->hosts[i].port)
- virBufferAsprintf(&buf, "\\:%s", src->hosts[i].port);
+ virBufferAsprintf(&buf, "\\:%d", src->hosts[i].port);
}
}
diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index af9063c02..45d9c77d8 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -92,8 +92,7 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri,
if (VIR_STRDUP(def->src->hosts->name, uri->server) < 0)
goto error;
- if (virAsprintf(&def->src->hosts->port, "%d", uri->port) < 0)
- goto error;
+ def->src->hosts->port = uri->port;
} else {
def->src->hosts->name = NULL;
def->src->hosts->port = 0;
@@ -240,8 +239,13 @@ qemuParseNBDString(virDomainDiskDefPtr disk)
if (src)
*src++ = '\0';
- if (VIR_STRDUP(h->port, port) < 0)
+ if (virStrToLong_i(port, NULL, 10, &h->port) < 0 ||
+ h->port < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to parse port number '%s'"),
+ port);
goto error;
+ }
}
if (src && STRPREFIX(src, "exportname=")) {
@@ -730,8 +734,13 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
goto error;
def->src->nhosts = 1;
def->src->hosts->name = def->src->path;
- if (VIR_STRDUP(def->src->hosts->port, port) < 0)
+ if (virStrToLong_i(port, NULL, 10, &def->src->hosts->port) < 0 ||
+ def->src->hosts->port < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to parse port number '%s'"),
+ port);
goto error;
+ }
def->src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
def->src->hosts->socket = NULL;
if (VIR_STRDUP(def->src->path, vdi) < 0)
@@ -2005,8 +2014,13 @@ qemuParseCommandLine(virCapsPtr caps,
goto error;
disk->src->nhosts = 1;
disk->src->hosts->name = disk->src->path;
- if (VIR_STRDUP(disk->src->hosts->port, port) < 0)
+ if (virStrToLong_i(port, NULL, 10, &disk->src->hosts->port) < 0 ||
+ disk->src->hosts->port < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to parse port number '%s'"),
+ port);
goto error;
+ }
if (VIR_STRDUP(disk->src->path, vdi) < 0)
goto error;
}
@@ -2540,13 +2554,18 @@ qemuParseCommandLine(virCapsPtr caps,
}
port = strchr(token, ':');
if (port) {
+ int port_i;
*port++ = '\0';
- if (VIR_STRDUP(port, port) < 0) {
+ if (virStrToLong_i(port, NULL, 10, &port_i) < 0 ||
+ port_i < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to parse port number '%s'"),
+ port);
VIR_FREE(hosts);
goto error;
}
+ first_rbd_disk->src->hosts[first_rbd_disk->src->nhosts].port = port_i;
}
- first_rbd_disk->src->hosts[first_rbd_disk->src->nhosts].port = port;
if (VIR_STRDUP(first_rbd_disk->src->hosts[first_rbd_disk->src->nhosts].name,
token) < 0) {
VIR_FREE(hosts);
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
index add8d34bf..cc042982e 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -575,9 +575,8 @@ virStorageFileBackendGlusterDeinit(virStorageSourcePtr src)
{
virStorageFileBackendGlusterPrivPtr priv = src->drv->priv;
- VIR_DEBUG("deinitializing gluster storage file %p (gluster://%s:%s/%s%s)",
- src, src->hosts->name, src->hosts->port ? src->hosts->port : "0",
- src->volume, src->path);
+ VIR_DEBUG("deinitializing gluster storage file %p (gluster://%s:%d/%s%s)",
+ src, src->hosts->name, src->hosts->port, src->volume, src->path);
if (priv->vol)
glfs_fini(priv->vol);
@@ -599,15 +598,7 @@ virStorageFileBackendGlusterInitServer(virStorageFileBackendGlusterPrivPtr priv,
case VIR_STORAGE_NET_HOST_TRANS_RDMA:
case VIR_STORAGE_NET_HOST_TRANS_TCP:
hoststr = host->name;
-
- if (host->port &&
- virStrToLong_i(host->port, NULL, 10, &port) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to parse port number '%s'"),
- host->port);
- return -1;
- }
-
+ port = host->port;
break;
case VIR_STORAGE_NET_HOST_TRANS_UNIX:
@@ -828,7 +819,7 @@ virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src)
priv)))
return NULL;
- ignore_value(virAsprintf(&priv->canonpath, "gluster://%s:%s/%s/%s",
+ ignore_value(virAsprintf(&priv->canonpath, "gluster://%s:%d/%s/%s",
src->hosts->name,
src->hosts->port,
src->volume,
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 18d630319..d3e81794d 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2781,11 +2781,8 @@ virStorageAddISCSIPoolSourceHost(virDomainDiskDefPtr def,
if (VIR_STRDUP(def->src->hosts[0].name, pooldef->source.hosts[0].name) < 0)
goto cleanup;
- if (virAsprintf(&def->src->hosts[0].port, "%d",
- pooldef->source.hosts[0].port ?
- pooldef->source.hosts[0].port :
- 3260) < 0)
- goto cleanup;
+ def->src->hosts[0].port = pooldef->source.hosts[0].port ?
+ pooldef->source.hosts[0].port : 3260;
/* iscsi volume has name like "unit:0:0:1" */
if (!(tokens = virStringSplit(def->src->srcpool->volume, ":", 0)))
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 2d0ff7812..bea4a42ad 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1688,8 +1688,8 @@ virStorageNetHostDefClear(virStorageNetHostDefPtr def)
if (!def)
return;
+ def->port = 0;
VIR_FREE(def->name);
- VIR_FREE(def->port);
VIR_FREE(def->socket);
}
@@ -1736,13 +1736,11 @@ virStorageNetHostDefCopy(size_t nhosts,
virStorageNetHostDefPtr dst = &ret[i];
dst->transport = src->transport;
+ dst->port = src->port;
if (VIR_STRDUP(dst->name, src->name) < 0)
goto error;
- if (VIR_STRDUP(dst->port, src->port) < 0)
- goto error;
-
if (VIR_STRDUP(dst->socket, src->socket) < 0)
goto error;
}
@@ -2430,10 +2428,8 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
tmp[0] = '\0';
}
- if (uri->port > 0) {
- if (virAsprintf(&src->hosts->port, "%d", uri->port) < 0)
- goto cleanup;
- }
+ if (uri->port > 0)
+ src->hosts->port = uri->port;
if (VIR_STRDUP(src->hosts->name, uri->server) < 0)
goto cleanup;
@@ -2470,8 +2466,14 @@ virStorageSourceRBDAddHost(virStorageSourcePtr src,
if (port) {
*port = '\0';
port += skip;
- if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, port) < 0)
+ if (virStrToLong_i(port, NULL, 10,
+ &src->hosts[src->nhosts - 1].port) < 0 ||
+ src->hosts[src->nhosts - 1].port < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to parse port number '%s'"),
+ port);
goto error;
+ }
}
parts = virStringSplit(hostport, "\\:", 0);
@@ -2488,7 +2490,6 @@ virStorageSourceRBDAddHost(virStorageSourcePtr src,
return 0;
error:
- VIR_FREE(src->hosts[src->nhosts-1].port);
VIR_FREE(src->hosts[src->nhosts-1].name);
return -1;
}
@@ -2638,7 +2639,7 @@ virStorageSourceParseNBDColonString(const char *nbdstr,
if (VIR_STRDUP(src->hosts->socket, backing[2]) < 0)
goto cleanup;
- } else {
+ } else {
if (VIR_STRDUP(src->hosts->name, backing[1]) < 0)
goto cleanup;
@@ -2649,8 +2650,13 @@ virStorageSourceParseNBDColonString(const char *nbdstr,
goto cleanup;
}
- if (VIR_STRDUP(src->hosts->port, backing[2]) < 0)
+ if (virStrToLong_i(backing[2], NULL, 10, &src->hosts->port) < 0 ||
+ src->hosts->port < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to parse port number '%s'"),
+ backing[2]);
goto cleanup;
+ }
}
if (backing[3] && STRPREFIX(backing[3], "exportname=")) {
@@ -2824,8 +2830,16 @@ virStorageSourceParseBackingJSONInetSocketAddress(virStorageNetHostDefPtr host,
host->transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
- if (VIR_STRDUP(host->name, hostname) < 0 ||
- VIR_STRDUP(host->port, port) < 0)
+ if (port &&
+ (virStrToLong_i(port, NULL, 10, &host->port) < 0 ||
+ host->port < 0)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to parse port number '%s'"),
+ port);
+ return -1;
+ }
+
+ if (VIR_STRDUP(host->name, hostname) < 0)
return -1;
return 0;
@@ -2985,11 +2999,12 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src,
goto cleanup;
if ((port = strchr(src->hosts->name, ':'))) {
- if (VIR_STRDUP(src->hosts->port, port + 1) < 0)
- goto cleanup;
-
- if (strlen(src->hosts->port) == 0)
- VIR_FREE(src->hosts->port);
+ if (virStrToLong_i(port + 1, NULL, 10, &src->hosts->port) < 0 ||
+ src->hosts->port < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to parse port number '%s'"),
+ port + 1);
+ }
*port = '\0';
}
@@ -3050,8 +3065,14 @@ virStorageSourceParseBackingJSONNbd(virStorageSourcePtr src,
if (VIR_STRDUP(src->hosts[0].name, host) < 0)
return -1;
- if (VIR_STRDUP(src->hosts[0].port, port) < 0)
+ if (port &&
+ (virStrToLong_i(port, NULL, 10, &src->hosts[0].port) < 0 ||
+ src->hosts[0].port < 0)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to parse port number '%s'"),
+ port);
return -1;
+ }
}
}
@@ -3136,8 +3157,17 @@ virStorageSourceParseBackingJSONSSH(virStorageSourcePtr src,
return -1;
} else {
src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
- if (VIR_STRDUP(src->hosts[0].name, host) < 0 ||
- VIR_STRDUP(src->hosts[0].port, port) < 0)
+
+ if (port &&
+ (virStrToLong_i(port, NULL, 10, &src->hosts[0].port) < 0 ||
+ src->hosts[0].port < 0)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to parse port number '%s'"),
+ port);
+ return -1;
+ }
+
+ if (VIR_STRDUP(src->hosts[0].name, host) < 0)
return -1;
}
@@ -3961,66 +3991,61 @@ virStorageSourceFindByNodeName(virStorageSourcePtr top,
}
-static const char *
+static int
virStorageSourceNetworkDefaultPort(virStorageNetProtocol protocol)
{
switch (protocol) {
case VIR_STORAGE_NET_PROTOCOL_HTTP:
- return "80";
+ return 80;
case VIR_STORAGE_NET_PROTOCOL_HTTPS:
- return "443";
+ return 443;
case VIR_STORAGE_NET_PROTOCOL_FTP:
- return "21";
+ return 21;
case VIR_STORAGE_NET_PROTOCOL_FTPS:
- return "990";
+ return 990;
case VIR_STORAGE_NET_PROTOCOL_TFTP:
- return "69";
+ return 69;
case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
- return "7000";
+ return 7000;
case VIR_STORAGE_NET_PROTOCOL_NBD:
- return "10809";
+ return 10809;
case VIR_STORAGE_NET_PROTOCOL_SSH:
- return "22";
+ return 22;
case VIR_STORAGE_NET_PROTOCOL_ISCSI:
- return "3260";
+ return 3260;
case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
- return "24007";
+ return 24007;
case VIR_STORAGE_NET_PROTOCOL_RBD:
/* we don't provide a default for RBD */
- return NULL;
+ return 0;
case VIR_STORAGE_NET_PROTOCOL_LAST:
case VIR_STORAGE_NET_PROTOCOL_NONE:
- return NULL;
+ return 0;
}
- return NULL;
+ return 0;
}
-int
+void
virStorageSourceNetworkAssignDefaultPorts(virStorageSourcePtr src)
{
size_t i;
for (i = 0; i < src->nhosts; i++) {
if (src->hosts[i].transport == VIR_STORAGE_NET_HOST_TRANS_TCP &&
- src->hosts[i].port == NULL) {
- if (VIR_STRDUP(src->hosts[i].port,
- virStorageSourceNetworkDefaultPort(src->protocol)) < 0)
- return -1;
- }
+ src->hosts[i].port == 0)
+ src->hosts[i].port = virStorageSourceNetworkDefaultPort(src->protocol);
}
-
- return 0;
}
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 98992e04a..934504806 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -155,7 +155,7 @@ typedef struct _virStorageNetHostDef virStorageNetHostDef;
typedef virStorageNetHostDef *virStorageNetHostDefPtr;
struct _virStorageNetHostDef {
char *name;
- char *port;
+ int port;
int transport; /* virStorageNetHostTransport */
char *socket; /* path to unix socket */
};
@@ -406,7 +406,7 @@ virStorageSourceFindByNodeName(virStorageSourcePtr top,
unsigned int *index)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-int
+void
virStorageSourceNetworkAssignDefaultPorts(virStorageSourcePtr src)
ATTRIBUTE_NONNULL(1);
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index cac440cd4..e21da5410 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -1057,7 +1057,7 @@ xenFormatXLDiskSrcNet(virStorageSourcePtr src)
virBufferAsprintf(&buf, "%s", src->hosts[i].name);
if (src->hosts[i].port)
- virBufferAsprintf(&buf, "\\\\:%s", src->hosts[i].port);
+ virBufferAsprintf(&buf, "\\\\:%d", src->hosts[i].port);
}
}
--
2.13.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Jul 19, 2017 at 17:26:27 +0200, Michal Privoznik wrote:
> Currently, @port is type of string. Well, that's overkill and
> waste of memory. Port is always an integer. Use it as such.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> src/conf/domain_conf.c | 25 ++++++--
> src/libxl/libxl_conf.c | 2 +-
> src/qemu/qemu_block.c | 2 +-
> src/qemu/qemu_command.c | 28 ++-------
> src/qemu/qemu_parse_command.c | 33 +++++++---
> src/storage/storage_backend_gluster.c | 17 ++---
> src/storage/storage_driver.c | 7 +--
> src/util/virstoragefile.c | 113 +++++++++++++++++++++-------------
> src/util/virstoragefile.h | 4 +-
> src/xenconfig/xen_xl.c | 2 +-
> 10 files changed, 130 insertions(+), 103 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9320794de..e8fdaa36f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
[...]
> @@ -6486,7 +6487,15 @@ virDomainStorageHostParse(xmlNodePtr node,
> goto cleanup;
> }
>
> - host.port = virXMLPropString(child, "port");
> + port = virXMLPropString(child, "port");
> + if (port &&
> + (virStrToLong_i(port, NULL, 10, &host.port) < 0 ||
> + host.port < 0)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("failed to parse port number '%s'"),
> + port);
> + goto cleanup;
> + }
> }
'port' is leaked when multiple <host> element are passed.
>
> if (VIR_APPEND_ELEMENT(*hosts, *nhosts, host) < 0)
> @@ -14909,7 +14919,7 @@ virDomainHostdevMatchSubsysSCSIiSCSI(virDomainHostdevDefPtr first,
> &second->source.subsys.u.scsi.u.iscsi;
>
> if (STREQ(first_iscsisrc->hosts[0].name, second_iscsisrc->hosts[0].name) &&
> - STREQ(first_iscsisrc->hosts[0].port, second_iscsisrc->hosts[0].port) &&
> + first_iscsisrc->hosts[0].port == second_iscsisrc->hosts[0].port &&
> STREQ(first_iscsisrc->path, second_iscsisrc->path))
> return 1;
> return 0;
> @@ -22255,7 +22267,8 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
> if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
> virBufferAddLit(buf, "<host");
> virBufferEscapeString(buf, " name='%s'", iscsisrc->hosts[0].name);
> - virBufferEscapeString(buf, " port='%s'", iscsisrc->hosts[0].port);
> + if (iscsisrc->hosts[0].port)
> + virBufferAsprintf(buf, " port='%d'", iscsisrc->hosts[0].port);
> virBufferAddLit(buf, "/>\n");
> } else {
> virBufferAsprintf(buf, "<adapter name='%s'/>\n",
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index a85bc71d2..1b20fb906 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -706,7 +706,7 @@ libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src,
> virBufferAsprintf(&buf, "%s", src->hosts[i].name);
>
> if (src->hosts[i].port)
> - virBufferAsprintf(&buf, "\\:%s", src->hosts[i].port);
> + virBufferAsprintf(&buf, "\\:%d", src->hosts[i].port);
> }
> }
>
> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index 93124c5ba..3d64528a8 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -465,7 +465,7 @@ qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src,
> if (virJSONValueObjectCreate(&server,
> "s:type", transport,
> "s:host", host->name,
> - "s:port", host->port,
> + "i:port", host->port,
> NULL) < 0)
> goto cleanup;
> break;
> @@ -897,8 +880,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
>
> switch (src->hosts->transport) {
> case VIR_STORAGE_NET_HOST_TRANS_TCP:
> - virBufferStrcat(&buf, src->hosts->name, ":",
> - src->hosts->port, NULL);
> + virBufferAsprintf(&buf, "%s:%d", src->hosts->name, src->hosts->port);
Line too long
> @@ -953,9 +935,9 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
> if (virAsprintf(&ret, "sheepdog:%s", src->path) < 0)
> goto cleanup;
> } else if (src->nhosts == 1) {
> - if (virAsprintf(&ret, "sheepdog:%s:%s:%s",
> + if (virAsprintf(&ret, "sheepdog:%s:%d:%s",
> src->hosts->name,
> - src->hosts->port ? src->hosts->port : "7000",
> + src->hosts->port ? src->hosts->port : 7000,
Hmm, this was missed in my cleanup patch.
> src->path) < 0)
> goto cleanup;
> } else {
[...]
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index 2d0ff7812..bea4a42ad 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -1688,8 +1688,8 @@ virStorageNetHostDefClear(virStorageNetHostDefPtr def)
> if (!def)
> return;
>
> + def->port = 0;
'transport' is not reset here, so port doesn't need to be either.
> VIR_FREE(def->name);
> - VIR_FREE(def->port);
> VIR_FREE(def->socket);
> @@ -2430,10 +2428,8 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
> tmp[0] = '\0';
> }
>
> - if (uri->port > 0) {
> - if (virAsprintf(&src->hosts->port, "%d", uri->port) < 0)
> - goto cleanup;
> - }
> + if (uri->port > 0)
> + src->hosts->port = uri->port;
The condition is redundant.
>
> if (VIR_STRDUP(src->hosts->name, uri->server) < 0)
> goto cleanup;
[...]
> @@ -2638,7 +2639,7 @@ virStorageSourceParseNBDColonString(const char *nbdstr,
> if (VIR_STRDUP(src->hosts->socket, backing[2]) < 0)
> goto cleanup;
>
> - } else {
> + } else {
Surious change.
> if (VIR_STRDUP(src->hosts->name, backing[1]) < 0)
> goto cleanup;
>
[...]
> @@ -2985,11 +2999,12 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src,
> goto cleanup;
>
> if ((port = strchr(src->hosts->name, ':'))) {
> - if (VIR_STRDUP(src->hosts->port, port + 1) < 0)
> - goto cleanup;
> -
> - if (strlen(src->hosts->port) == 0)
> - VIR_FREE(src->hosts->port);
> + if (virStrToLong_i(port + 1, NULL, 10, &src->hosts->port) < 0 ||
This will report the error if the port is not specified after the colon,
whereas previously it would not.
> + src->hosts->port < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("failed to parse port number '%s'"),
> + port + 1);
> + }
>
> *port = '\0';
> }
[...]
> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> index 98992e04a..934504806 100644
> --- a/src/util/virstoragefile.h
> +++ b/src/util/virstoragefile.h
> @@ -155,7 +155,7 @@ typedef struct _virStorageNetHostDef virStorageNetHostDef;
> typedef virStorageNetHostDef *virStorageNetHostDefPtr;
> struct _virStorageNetHostDef {
> char *name;
> - char *port;
> + int port;
If you want to be precise ... have you ever seen negative ports?
> int transport; /* virStorageNetHostTransport */
> char *socket; /* path to unix socket */
> };
This will require a lot of fixing since you blindly copied the check
that also checks that the port is not less than 0.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Jul 19, 2017 at 17:58:15 +0200, Peter Krempa wrote:
> On Wed, Jul 19, 2017 at 17:26:27 +0200, Michal Privoznik wrote:
> > Currently, @port is type of string. Well, that's overkill and
> > waste of memory. Port is always an integer. Use it as such.
> >
> > Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> > ---
> > src/conf/domain_conf.c | 25 ++++++--
> > src/libxl/libxl_conf.c | 2 +-
> > src/qemu/qemu_block.c | 2 +-
> > src/qemu/qemu_command.c | 28 ++-------
> > src/qemu/qemu_parse_command.c | 33 +++++++---
> > src/storage/storage_backend_gluster.c | 17 ++---
> > src/storage/storage_driver.c | 7 +--
> > src/util/virstoragefile.c | 113 +++++++++++++++++++++-------------
> > src/util/virstoragefile.h | 4 +-
> > src/xenconfig/xen_xl.c | 2 +-
> > 10 files changed, 130 insertions(+), 103 deletions(-)
[...]
> > diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> > index 98992e04a..934504806 100644
> > --- a/src/util/virstoragefile.h
> > +++ b/src/util/virstoragefile.h
> > @@ -155,7 +155,7 @@ typedef struct _virStorageNetHostDef virStorageNetHostDef;
> > typedef virStorageNetHostDef *virStorageNetHostDefPtr;
> > struct _virStorageNetHostDef {
> > char *name;
> > - char *port;
> > + int port;
>
> If you want to be precise ... have you ever seen negative ports?
>
> > int transport; /* virStorageNetHostTransport */
> > char *socket; /* path to unix socket */
> > };
>
> This will require a lot of fixing since you blindly copied the check
> that also checks that the port is not less than 0.
I'll send two patches that I have on a branch that convert 'port' in
virURI to unsigned, which will possibly avoid you from typecasting.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07/19/2017 05:58 PM, Peter Krempa wrote:
> On Wed, Jul 19, 2017 at 17:26:27 +0200, Michal Privoznik wrote:
>> Currently, @port is type of string. Well, that's overkill and
>> waste of memory. Port is always an integer. Use it as such.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>> src/conf/domain_conf.c | 25 ++++++--
>> src/libxl/libxl_conf.c | 2 +-
>> src/qemu/qemu_block.c | 2 +-
>> src/qemu/qemu_command.c | 28 ++-------
>> src/qemu/qemu_parse_command.c | 33 +++++++---
>> src/storage/storage_backend_gluster.c | 17 ++---
>> src/storage/storage_driver.c | 7 +--
>> src/util/virstoragefile.c | 113 +++++++++++++++++++++-------------
>> src/util/virstoragefile.h | 4 +-
>> src/xenconfig/xen_xl.c | 2 +-
>> 10 files changed, 130 insertions(+), 103 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 9320794de..e8fdaa36f 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>
> [...]
>
>> @@ -6486,7 +6487,15 @@ virDomainStorageHostParse(xmlNodePtr node,
>> goto cleanup;
>> }
>>
>> - host.port = virXMLPropString(child, "port");
>> + port = virXMLPropString(child, "port");
>> + if (port &&
>> + (virStrToLong_i(port, NULL, 10, &host.port) < 0 ||
>> + host.port < 0)) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("failed to parse port number '%s'"),
>> + port);
>> + goto cleanup;
>> + }
>> }
>
> 'port' is leaked when multiple <host> element are passed.
>
>>
>> if (VIR_APPEND_ELEMENT(*hosts, *nhosts, host) < 0)
>
>
>
>> @@ -14909,7 +14919,7 @@ virDomainHostdevMatchSubsysSCSIiSCSI(virDomainHostdevDefPtr first,
>> &second->source.subsys.u.scsi.u.iscsi;
>>
>> if (STREQ(first_iscsisrc->hosts[0].name, second_iscsisrc->hosts[0].name) &&
>> - STREQ(first_iscsisrc->hosts[0].port, second_iscsisrc->hosts[0].port) &&
>> + first_iscsisrc->hosts[0].port == second_iscsisrc->hosts[0].port &&
>> STREQ(first_iscsisrc->path, second_iscsisrc->path))
>> return 1;
>> return 0;
>
>
>> @@ -22255,7 +22267,8 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
>> if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
>> virBufferAddLit(buf, "<host");
>> virBufferEscapeString(buf, " name='%s'", iscsisrc->hosts[0].name);
>> - virBufferEscapeString(buf, " port='%s'", iscsisrc->hosts[0].port);
>> + if (iscsisrc->hosts[0].port)
>> + virBufferAsprintf(buf, " port='%d'", iscsisrc->hosts[0].port);
>> virBufferAddLit(buf, "/>\n");
>> } else {
>> virBufferAsprintf(buf, "<adapter name='%s'/>\n",
>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>> index a85bc71d2..1b20fb906 100644
>> --- a/src/libxl/libxl_conf.c
>> +++ b/src/libxl/libxl_conf.c
>> @@ -706,7 +706,7 @@ libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src,
>> virBufferAsprintf(&buf, "%s", src->hosts[i].name);
>>
>> if (src->hosts[i].port)
>> - virBufferAsprintf(&buf, "\\:%s", src->hosts[i].port);
>> + virBufferAsprintf(&buf, "\\:%d", src->hosts[i].port);
>> }
>> }
>>
>> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
>> index 93124c5ba..3d64528a8 100644
>> --- a/src/qemu/qemu_block.c
>> +++ b/src/qemu/qemu_block.c
>> @@ -465,7 +465,7 @@ qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src,
>> if (virJSONValueObjectCreate(&server,
>> "s:type", transport,
>> "s:host", host->name,
>> - "s:port", host->port,
>> + "i:port", host->port,
>> NULL) < 0)
>> goto cleanup;
>> break;
>
>
>
>> @@ -897,8 +880,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
>>
>> switch (src->hosts->transport) {
>> case VIR_STORAGE_NET_HOST_TRANS_TCP:
>> - virBufferStrcat(&buf, src->hosts->name, ":",
>> - src->hosts->port, NULL);
>> + virBufferAsprintf(&buf, "%s:%d", src->hosts->name, src->hosts->port);
>
> Line too long
>
>> @@ -953,9 +935,9 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
>> if (virAsprintf(&ret, "sheepdog:%s", src->path) < 0)
>> goto cleanup;
>> } else if (src->nhosts == 1) {
>> - if (virAsprintf(&ret, "sheepdog:%s:%s:%s",
>> + if (virAsprintf(&ret, "sheepdog:%s:%d:%s",
>> src->hosts->name,
>> - src->hosts->port ? src->hosts->port : "7000",
>> + src->hosts->port ? src->hosts->port : 7000,
>
> Hmm, this was missed in my cleanup patch.
>
>> src->path) < 0)
>> goto cleanup;
>> } else {
>
> [...]
>
>> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
>> index 2d0ff7812..bea4a42ad 100644
>> --- a/src/util/virstoragefile.c
>> +++ b/src/util/virstoragefile.c
>> @@ -1688,8 +1688,8 @@ virStorageNetHostDefClear(virStorageNetHostDefPtr def)
>> if (!def)
>> return;
>>
>> + def->port = 0;
>
> 'transport' is not reset here, so port doesn't need to be either.
>
>> VIR_FREE(def->name);
>> - VIR_FREE(def->port);
>> VIR_FREE(def->socket);
>
>
>
>> @@ -2430,10 +2428,8 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
>> tmp[0] = '\0';
>> }
>>
>> - if (uri->port > 0) {
>> - if (virAsprintf(&src->hosts->port, "%d", uri->port) < 0)
>> - goto cleanup;
>> - }
>> + if (uri->port > 0)
>> + src->hosts->port = uri->port;
>
> The condition is redundant.
>
>>
>> if (VIR_STRDUP(src->hosts->name, uri->server) < 0)
>> goto cleanup;
>
> [...]
>
>> @@ -2638,7 +2639,7 @@ virStorageSourceParseNBDColonString(const char *nbdstr,
>> if (VIR_STRDUP(src->hosts->socket, backing[2]) < 0)
>> goto cleanup;
>>
>> - } else {
>> + } else {
>
> Surious change.
>
>> if (VIR_STRDUP(src->hosts->name, backing[1]) < 0)
>> goto cleanup;
>>
>
> [...]
>
>> @@ -2985,11 +2999,12 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src,
>> goto cleanup;
>>
>> if ((port = strchr(src->hosts->name, ':'))) {
>> - if (VIR_STRDUP(src->hosts->port, port + 1) < 0)
>> - goto cleanup;
>> -
>> - if (strlen(src->hosts->port) == 0)
>> - VIR_FREE(src->hosts->port);
>> + if (virStrToLong_i(port + 1, NULL, 10, &src->hosts->port) < 0 ||
>
> This will report the error if the port is not specified after the colon,
> whereas previously it would not.
>
>> + src->hosts->port < 0) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("failed to parse port number '%s'"),
>> + port + 1);
>> + }
>>
>> *port = '\0';
>> }
>
>
> [...]
>
>> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
>> index 98992e04a..934504806 100644
>> --- a/src/util/virstoragefile.h
>> +++ b/src/util/virstoragefile.h
>> @@ -155,7 +155,7 @@ typedef struct _virStorageNetHostDef virStorageNetHostDef;
>> typedef virStorageNetHostDef *virStorageNetHostDefPtr;
>> struct _virStorageNetHostDef {
>> char *name;
>> - char *port;
>> + int port;
>
> If you want to be precise ... have you ever seen negative ports?
>
>> int transport; /* virStorageNetHostTransport */
>> char *socket; /* path to unix socket */
>> };
>
> This will require a lot of fixing since you blindly copied the check
> that also checks that the port is not less than 0.
>
Okay, I don't care that much to post v2. If somebody wants to take it
over from here, be my guest.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.