[libvirt] [PATCH] virStorageNetHostDef: Turn @port into integer
Peter Krempa
pkrempa at redhat.com
Wed Jul 19 15:58:15 UTC 2017
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 at 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170719/2b4ec7a6/attachment-0001.sig>
More information about the libvir-list
mailing list