7:29 a.m.
Since Michal did not want to finis this I did. I also found a few bugs in
my previous patches which are addressed here.
Peter Krempa (6):
qemu: command: Rename and move qemuNetworkDriveGetPort
util: uri: Convert port number to unsigned integer
qemu: command: Remove condition to use default sheepdog port
util: storage: fill in default ports when parsing backing chain
conf: domain: Split up virDomainStorageHostParse and rename it
virStorageNetHostDef: Turn @port into integer
src/conf/domain_conf.c | 133 +++++++++++++++++++---------------
src/libvirt_private.syms | 1 +
src/libxl/libxl_conf.c | 2 +-
src/qemu/qemu_block.c | 7 +-
src/qemu/qemu_command.c | 30 ++------
src/qemu/qemu_parse_command.c | 13 ++--
src/storage/storage_backend_gluster.c | 17 +----
src/storage/storage_driver.c | 7 +-
src/util/virstoragefile.c | 68 +++++++----------
src/util/virstoragefile.h | 4 +-
src/util/virstring.c | 37 ++++++++++
src/util/virstring.h | 4 +
src/util/viruri.h | 2 +-
src/xenconfig/xen_xl.c | 2 +-
tests/virstoragetest.c | 14 ++--
15 files changed, 182 insertions(+), 159 deletions(-)
--
2.13.2
7:29 a.m.
New subject: [libvirt] [PATCH v2 1/6] qemu: command: Rename and move qemuNetworkDriveGetPort
Move it to virstring.c and improve it to parse and validate ports. New
name is virStringParsePort.
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_command.c | 18 +-----------------
src/util/virstring.c | 37 +++++++++++++++++++++++++++++++++++++
src/util/virstring.h | 4 ++++
4 files changed, 43 insertions(+), 17 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 187b12b32..f0c2c8446 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2656,6 +2656,7 @@ virStringListJoin;
virStringListLength;
virStringListRemove;
virStringMatch;
+virStringParsePort;
virStringReplace;
virStringSearch;
virStringSortCompare;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6ac26af3e..be23796df 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,7 +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)
+ if (virStringParsePort(src->hosts->port, &uri->port) < 0)
goto cleanup;
if (VIR_STRDUP(uri->scheme,
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 9b54bd6fb..fe61a3516 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -1344,3 +1344,40 @@ void virStringTrimOptionalNewline(char *str)
if (*tmp == '\n')
*tmp = '\0';
}
+
+
+/**
+ * virStringParsePort:
+ * @str: port number to parse
+ * @port: pointer to parse port into
+ *
+ * Parses a string representation of a network port and validates it. Returns
+ * 0 on success and -1 on error.
+ */
+int
+virStringParsePort(const char *str,
+ int *port)
+{
+ unsigned int p = 0;
+
+ *port = 0;
+
+ if (!str)
+ return 0;
+
+ if (virStrToLong_uip(str, NULL, 10, &p) < 0) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("failed to parse port number '%s'"), str);
+ return -1;
+ }
+
+ if (p > 65535) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("port '%s' out of range"), str);
+ return -1;
+ }
+
+ *port = p;
+
+ return 0;
+}
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 5eaaaea0a..e562bf514 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -296,4 +296,8 @@ char *virStringEncodeBase64(const uint8_t *buf, size_t buflen);
void virStringTrimOptionalNewline(char *str);
+int virStringParsePort(const char *str,
+ int *port)
+ ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+
#endif /* __VIR_STRING_H__ */
--
2.13.2
6:58 a.m.
New subject: [libvirt] [PATCH v2 1/6] qemu: command: Rename and move qemuNetworkDriveGetPort
[...]
Before it's too late...
+
+/**
+ * virStringParsePort:
+ * @str: port number to parse
+ * @port: pointer to parse port into
+ *
+ * Parses a string representation of a network port and validates it. Returns
+ * 0 on success and -1 on error.
+ */
+int
+virStringParsePort(const char *str,
+ int *port)
+{
+ unsigned int p = 0;
+
+ *port = 0;
+
+ if (!str)
+ return 0;
+
+ if (virStrToLong_uip(str, NULL, 10, &p) < 0) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("failed to parse port number '%s'"), str);
+ return -1;
+ }
+
+ if (p > 65535) {
Should this be UINT16_MAX ?
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("port '%s' out of range"), str);
+ return -1;
+ }
+
+ *port = p;
+
+ return 0;
+}
3:43 a.m.
New subject: [libvirt] [PATCH v2 1/6] qemu: command: Rename and move qemuNetworkDriveGetPort
On Fri, Jul 21, 2017 at 07:58:52 -0400, John Ferlan wrote:
[...]
Before it's too late...
> +
> +/**
> + * virStringParsePort:
> + * @str: port number to parse
> + * @port: pointer to parse port into
> + *
> + * Parses a string representation of a network port and validates it. Returns
> + * 0 on success and -1 on error.
> + */
> +int
> +virStringParsePort(const char *str,
> + int *port)
> +{
> + unsigned int p = 0;
> +
> + *port = 0;
> +
> + if (!str)
> + return 0;
> +
> + if (virStrToLong_uip(str, NULL, 10, &p) < 0) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("failed to parse port number '%s'"),
str);
> + return -1;
> + }
> +
> + if (p > 65535) {
Should this be UINT16_MAX ?
Yup. RFC 791 describes it as 16 bits.
7:29 a.m.
New subject: [libvirt] [PATCH v2 2/6] util: uri: Convert port number to unsigned integer
Negative ports don't make sense so use a unsigned integer.
---
src/util/virstring.c | 2 +-
src/util/virstring.h | 2 +-
src/util/viruri.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/util/virstring.c b/src/util/virstring.c
index fe61a3516..69e0fd173 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -1356,7 +1356,7 @@ void virStringTrimOptionalNewline(char *str)
*/
int
virStringParsePort(const char *str,
- int *port)
+ unsigned int *port)
{
unsigned int p = 0;
diff --git a/src/util/virstring.h b/src/util/virstring.h
index e562bf514..ff5f0148d 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -297,7 +297,7 @@ char *virStringEncodeBase64(const uint8_t *buf, size_t buflen);
void virStringTrimOptionalNewline(char *str);
int virStringParsePort(const char *str,
- int *port)
+ unsigned int *port)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
#endif /* __VIR_STRING_H__ */
diff --git a/src/util/viruri.h b/src/util/viruri.h
index 1e53abb0b..7850c38c2 100644
--- a/src/util/viruri.h
+++ b/src/util/viruri.h
@@ -42,7 +42,7 @@ struct _virURI {
char *scheme; /* the URI scheme */
char *server; /* the server part */
char *user; /* the user part */
- int port; /* the port number */
+ unsigned int port; /* the port number */
char *path; /* the path string */
char *query; /* the query string */
char *fragment; /* the fragment string */
--
2.13.2
7:30 a.m.
New subject: [libvirt] [PATCH v2 3/6] qemu: command: Remove condition to use default sheepdog port
Since we now set the default ports when parsing disks, it's not
necessary to have default port numbers encoded in the command line
generator.
---
src/qemu/qemu_command.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index be23796df..810578840 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -938,8 +938,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src,
goto cleanup;
} else if (src->nhosts == 1) {
if (virAsprintf(&ret, "sheepdog:%s:%s:%s",
- src->hosts->name,
- src->hosts->port ? src->hosts->port :
"7000",
+ src->hosts->name, src->hosts->port,
src->path) < 0)
goto cleanup;
} else {
--
2.13.2
7:30 a.m.
New subject: [libvirt] [PATCH v2 4/6] util: storage: fill in default ports when parsing backing chain
Similarly to when parsing XML we need to fill in default ports for the
backing chain. This was missed in commit 5bda835466a8050625dd8bb10566e
---
src/util/virstoragefile.c | 3 +++
tests/virstoragetest.c | 14 +++++++-------
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 2d0ff7812..bc1b616d4 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3340,6 +3340,9 @@ virStorageSourceNewFromBackingAbsolute(const char *path)
if (rc < 0)
goto error;
+
+ if (virStorageSourceNetworkAssignDefaultPorts(ret) < 0)
+ goto error;
}
return ret;
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 5352500b7..d83db78f5 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1349,7 +1349,7 @@ mymain(void)
TEST_BACKING_PARSE("://", NULL);
TEST_BACKING_PARSE("http://example.com/file";,
"<source protocol='http'
name='file'>\n"
- " <host name='example.com'/>\n"
+ " <host name='example.com'
port='80'/>\n"
"</source>\n");
TEST_BACKING_PARSE("rbd:testshare:id=asdf:mon_host=example.com",
"<source protocol='rbd'
name='testshare'>\n"
@@ -1385,14 +1385,14 @@ mymain(void)
TEST_BACKING_PARSE("json:{\"file.driver\":\"http\", "
"\"file.url\":\"http://example.com/file\"}",
"<source protocol='http'
name='file'>\n"
- " <host name='example.com'/>\n"
+ " <host name='example.com'
port='80'/>\n"
"</source>\n");
TEST_BACKING_PARSE("json:{\"file\":{
\"driver\":\"http\","
"\"url\":\"http://example.com/file\""
"}"
"}",
"<source protocol='http'
name='file'>\n"
- " <host name='example.com'/>\n"
+ " <host name='example.com'
port='80'/>\n"
"</source>\n");
TEST_BACKING_PARSE("json:{\"file.driver\":\"ftp\", "
"\"file.url\":\"http://example.com/file\"}",
@@ -1400,7 +1400,7 @@ mymain(void)
TEST_BACKING_PARSE("json:{\"file.driver\":\"gluster\",
"
"\"file.filename\":\"gluster://example.com/vol/file\"}",
"<source protocol='gluster'
name='vol/file'>\n"
- " <host name='example.com'/>\n"
+ " <host name='example.com'
port='24007'/>\n"
"</source>\n");
TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"gluster\","
"\"volume\":\"testvol\","
@@ -1421,7 +1421,7 @@ mymain(void)
"<source protocol='gluster'
name='testvol/img.qcow2'>\n"
" <host name='example.com'
port='1234'/>\n"
" <host transport='unix'
socket='/path/socket'/>\n"
- " <host name='example.com'/>\n"
+ " <host name='example.com'
port='24007'/>\n"
"</source>\n");
TEST_BACKING_PARSE("json:{\"file.driver\":\"gluster\","
"\"file.volume\":\"testvol\","
@@ -1441,7 +1441,7 @@ mymain(void)
"<source protocol='gluster'
name='testvol/img.qcow2'>\n"
" <host name='example.com'
port='1234'/>\n"
" <host transport='unix'
socket='/path/socket'/>\n"
- " <host name='example.com'/>\n"
+ " <host name='example.com'
port='24007'/>\n"
"</source>\n");
TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"nbd\","
"\"path\":\"/path/to/socket\""
@@ -1552,7 +1552,7 @@ mymain(void)
"}"
"}",
"<source protocol='iscsi'
name='iqn.2016-12.com.virttest:emulated-iscsi-noauth.target/0'>\n"
- " <host name='test.org'/>\n"
+ " <host name='test.org'
port='3260'/>\n"
"</source>\n");
TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"iscsi\","
"\"transport\":\"tcp\","
--
2.13.2
7:30 a.m.
New subject: [libvirt] [PATCH v2 5/6] conf: domain: Split up virDomainStorageHostParse and rename it
Split out parsing of one host into a separate function and add a new
function to loop through all the host XML nodes.
This change removes multiple levels of nesting due to the old XML
parsing approach used.
---
src/conf/domain_conf.c | 117 +++++++++++++++++++++++++++----------------------
1 file changed, 64 insertions(+), 53 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9320794de..a3fd7195a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6429,71 +6429,61 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node,
return ret;
}
+
static int
-virDomainStorageHostParse(xmlNodePtr node,
- virStorageNetHostDefPtr *hosts,
- size_t *nhosts)
+virDomainStorageNetworkParseHost(xmlNodePtr hostnode,
+ virStorageNetHostDefPtr *hosts,
+ size_t *nhosts)
{
int ret = -1;
- xmlNodePtr child;
char *transport = NULL;
virStorageNetHostDef host;
memset(&host, 0, sizeof(host));
+ host.transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
- child = node->children;
- while (child != NULL) {
- if (child->type == XML_ELEMENT_NODE &&
- xmlStrEqual(child->name, BAD_CAST "host")) {
-
- host.transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
-
- /* transport can be tcp (default), unix or rdma. */
- if ((transport = virXMLPropString(child, "transport"))) {
- host.transport = virStorageNetHostTransportTypeFromString(transport);
- if (host.transport < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown protocol transport type
'%s'"),
- transport);
- goto cleanup;
- }
- }
+ /* transport can be tcp (default), unix or rdma. */
+ if ((transport = virXMLPropString(hostnode, "transport"))) {
+ host.transport = virStorageNetHostTransportTypeFromString(transport);
+ if (host.transport < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown protocol transport type '%s'"),
+ transport);
+ goto cleanup;
+ }
+ }
- host.socket = virXMLPropString(child, "socket");
+ host.socket = virXMLPropString(hostnode, "socket");
- if (host.transport == VIR_STORAGE_NET_HOST_TRANS_UNIX &&
- host.socket == NULL) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing socket for unix transport"));
- goto cleanup;
- }
+ if (host.transport == VIR_STORAGE_NET_HOST_TRANS_UNIX &&
+ host.socket == NULL) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing socket for unix transport"));
+ goto cleanup;
+ }
- if (host.transport != VIR_STORAGE_NET_HOST_TRANS_UNIX &&
- host.socket != NULL) {
- virReportError(VIR_ERR_XML_ERROR,
- _("transport '%s' does not support "
- "socket attribute"),
- transport);
- goto cleanup;
- }
+ if (host.transport != VIR_STORAGE_NET_HOST_TRANS_UNIX &&
+ host.socket != NULL) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("transport '%s' does not support "
+ "socket attribute"),
+ transport);
+ goto cleanup;
+ }
- VIR_FREE(transport);
+ if (host.transport != VIR_STORAGE_NET_HOST_TRANS_UNIX) {
+ if (!(host.name = virXMLPropString(hostnode, "name"))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing name for host"));
+ goto cleanup;
+ }
- if (host.transport != VIR_STORAGE_NET_HOST_TRANS_UNIX) {
- if (!(host.name = virXMLPropString(child, "name"))) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing name for host"));
- goto cleanup;
- }
+ host.port = virXMLPropString(hostnode, "port");
+ }
- host.port = virXMLPropString(child, "port");
- }
+ if (VIR_APPEND_ELEMENT(*hosts, *nhosts, host) < 0)
+ goto cleanup;
- if (VIR_APPEND_ELEMENT(*hosts, *nhosts, host) < 0)
- goto cleanup;
- }
- child = child->next;
- }
ret = 0;
cleanup:
@@ -6502,6 +6492,27 @@ virDomainStorageHostParse(xmlNodePtr node,
return ret;
}
+
+static int
+virDomainStorageNetworkParseHosts(xmlNodePtr node,
+ virStorageNetHostDefPtr *hosts,
+ size_t *nhosts)
+{
+ xmlNodePtr child;
+
+ for (child = node->children; child; child = child->next) {
+ if (child->type == XML_ELEMENT_NODE &&
+ xmlStrEqual(child->name, BAD_CAST "host")) {
+
+ if (virDomainStorageNetworkParseHost(child, hosts, nhosts) < 0)
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
+
static int
virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode,
virDomainHostdevSubsysSCSIPtr scsisrc)
@@ -6609,8 +6620,8 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode,
goto cleanup;
}
- if (virDomainStorageHostParse(sourcenode, &iscsisrc->hosts,
- &iscsisrc->nhosts) < 0)
+ if (virDomainStorageNetworkParseHosts(sourcenode, &iscsisrc->hosts,
+ &iscsisrc->nhosts) < 0)
goto cleanup;
if (iscsisrc->nhosts < 1) {
@@ -7879,7 +7890,7 @@ virDomainDiskSourceParse(xmlNodePtr node,
/* config file currently only works with remote disks */
src->configFile = virXPathString("string(./config/@file)", ctxt);
- if (virDomainStorageHostParse(node, &src->hosts, &src->nhosts) <
0)
+ if (virDomainStorageNetworkParseHosts(node, &src->hosts,
&src->nhosts) < 0)
goto cleanup;
if (virStorageSourceNetworkAssignDefaultPorts(src) < 0)
--
2.13.2
7:30 a.m.
New subject: [libvirt] [PATCH v2 6/6] virStorageNetHostDef: Turn @port into integer
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(a)redhat.com>
---
src/conf/domain_conf.c | 20 +++++++---
src/libxl/libxl_conf.c | 2 +-
src/qemu/qemu_block.c | 7 +++-
src/qemu/qemu_command.c | 11 +++---
src/qemu/qemu_parse_command.c | 13 +++----
src/storage/storage_backend_gluster.c | 17 ++-------
src/storage/storage_driver.c | 7 +---
src/util/virstoragefile.c | 69 ++++++++++++++---------------------
src/util/virstoragefile.h | 4 +-
src/xenconfig/xen_xl.c | 2 +-
10 files changed, 68 insertions(+), 84 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a3fd7195a..7ba6804ac 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6437,6 +6437,7 @@ virDomainStorageNetworkParseHost(xmlNodePtr hostnode,
{
int ret = -1;
char *transport = NULL;
+ char *port = NULL;
virStorageNetHostDef host;
memset(&host, 0, sizeof(host));
@@ -6478,7 +6479,10 @@ virDomainStorageNetworkParseHost(xmlNodePtr hostnode,
goto cleanup;
}
- host.port = virXMLPropString(hostnode, "port");
+ if ((port = virXMLPropString(hostnode, "port"))) {
+ if (virStringParsePort(port, &host.port) < 0)
+ goto cleanup;
+ }
}
if (VIR_APPEND_ELEMENT(*hosts, *nhosts, host) < 0)
@@ -6489,6 +6493,7 @@ virDomainStorageNetworkParseHost(xmlNodePtr hostnode,
cleanup:
virStorageNetHostDefClear(&host);
VIR_FREE(transport);
+ VIR_FREE(port);
return ret;
}
@@ -7893,8 +7898,8 @@ virDomainDiskSourceParse(xmlNodePtr node,
if (virDomainStorageNetworkParseHosts(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)
@@ -14920,7 +14925,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;
@@ -21419,7 +21424,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='%u'",
src->hosts[n].port);
if (src->hosts[n].transport)
virBufferAsprintf(buf, " transport='%s'",
@@ -22266,7 +22273,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='%u'",
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..4416a09dd 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, "\\:%u",
src->hosts[i].port);
}
}
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 93124c5ba..22de70657 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -447,6 +447,7 @@ qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr
src,
virJSONValuePtr ret = NULL;
virStorageNetHostDefPtr host;
const char *transport;
+ char *port = NULL;
size_t i;
if (!(servers = virJSONValueNewArray()))
@@ -462,10 +463,13 @@
qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src,
else
transport = "inet";
+ if (virAsprintf(&port, "%u", host->port) < 0)
+ goto cleanup;
+
if (virJSONValueObjectCreate(&server,
"s:type", transport,
"s:host", host->name,
- "s:port", host->port,
+ "s:port", port,
NULL) < 0)
goto cleanup;
break;
@@ -497,6 +501,7 @@ qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr
src,
cleanup:
virJSONValueFree(servers);
virJSONValueFree(server);
+ VIR_FREE(port);
return ret;
}
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 810578840..065217013 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -809,8 +809,7 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
goto cleanup;
if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) {
- if (virStringParsePort(src->hosts->port, &uri->port) < 0)
- goto cleanup;
+ uri->port = src->hosts->port;
if (VIR_STRDUP(uri->scheme,
virStorageNetProtocolTypeToString(src->protocol)) < 0)
@@ -881,8 +880,8 @@ 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:%u",
+ src->hosts->name, src->hosts->port);
break;
case VIR_STORAGE_NET_HOST_TRANS_UNIX:
@@ -937,7 +936,7 @@ 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:%u:%s",
src->hosts->name, src->hosts->port,
src->path) < 0)
goto cleanup;
@@ -979,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, "\\:%u",
src->hosts[i].port);
}
}
diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index af9063c02..15ed5c9e6 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,7 +239,7 @@ qemuParseNBDString(virDomainDiskDefPtr disk)
if (src)
*src++ = '\0';
- if (VIR_STRDUP(h->port, port) < 0)
+ if (virStringParsePort(port, &h->port) < 0)
goto error;
}
@@ -730,7 +729,7 @@ 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 (virStringParsePort(port, &def->src->hosts->port)
< 0)
goto error;
def->src->hosts->transport =
VIR_STORAGE_NET_HOST_TRANS_TCP;
def->src->hosts->socket = NULL;
@@ -2005,7 +2004,7 @@ 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 (virStringParsePort(port,
&disk->src->hosts->port) < 0)
goto error;
if (VIR_STRDUP(disk->src->path, vdi) < 0)
goto error;
@@ -2541,12 +2540,12 @@ qemuParseCommandLine(virCapsPtr caps,
port = strchr(token, ':');
if (port) {
*port++ = '\0';
- if (VIR_STRDUP(port, port) < 0) {
+ if (virStringParsePort(port,
+
&first_rbd_disk->src->hosts[first_rbd_disk->src->nhosts].port) < 0) {
VIR_FREE(hosts);
goto error;
}
}
- 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..92038c131 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:%u/%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:%u/%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 bc1b616d4..0add4c146 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1689,7 +1689,6 @@ virStorageNetHostDefClear(virStorageNetHostDefPtr def)
return;
VIR_FREE(def->name);
- VIR_FREE(def->port);
VIR_FREE(def->socket);
}
@@ -1736,13 +1735,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 +2427,7 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
tmp[0] = '\0';
}
- if (uri->port > 0) {
- if (virAsprintf(&src->hosts->port, "%d", uri->port) <
0)
- goto cleanup;
- }
+ src->hosts->port = uri->port;
if (VIR_STRDUP(src->hosts->name, uri->server) < 0)
goto cleanup;
@@ -2470,7 +2464,7 @@ virStorageSourceRBDAddHost(virStorageSourcePtr src,
if (port) {
*port = '\0';
port += skip;
- if (VIR_STRDUP(src->hosts[src->nhosts - 1].port, port) < 0)
+ if (virStringParsePort(port, &src->hosts[src->nhosts - 1].port) <
0)
goto error;
}
@@ -2488,7 +2482,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;
}
@@ -2649,7 +2642,7 @@ virStorageSourceParseNBDColonString(const char *nbdstr,
goto cleanup;
}
- if (VIR_STRDUP(src->hosts->port, backing[2]) < 0)
+ if (virStringParsePort(backing[2], &src->hosts->port) < 0)
goto cleanup;
}
@@ -2825,7 +2818,7 @@
virStorageSourceParseBackingJSONInetSocketAddress(virStorageNetHostDefPtr host,
host->transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
if (VIR_STRDUP(host->name, hostname) < 0 ||
- VIR_STRDUP(host->port, port) < 0)
+ virStringParsePort(port, &host->port) < 0)
return -1;
return 0;
@@ -2985,12 +2978,9 @@ virStorageSourceParseBackingJSONiSCSI(virStorageSourcePtr src,
goto cleanup;
if ((port = strchr(src->hosts->name, ':'))) {
- if (VIR_STRDUP(src->hosts->port, port + 1) < 0)
+ if (virStringParsePort(port + 1, &src->hosts->port) < 0)
goto cleanup;
- if (strlen(src->hosts->port) == 0)
- VIR_FREE(src->hosts->port);
-
*port = '\0';
}
@@ -3050,7 +3040,7 @@ virStorageSourceParseBackingJSONNbd(virStorageSourcePtr src,
if (VIR_STRDUP(src->hosts[0].name, host) < 0)
return -1;
- if (VIR_STRDUP(src->hosts[0].port, port) < 0)
+ if (virStringParsePort(port, &src->hosts[0].port) < 0)
return -1;
}
}
@@ -3136,8 +3126,9 @@ 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)
+ virStringParsePort(port, &src->hosts[0].port) < 0)
return -1;
}
@@ -3341,8 +3332,7 @@ virStorageSourceNewFromBackingAbsolute(const char *path)
if (rc < 0)
goto error;
- if (virStorageSourceNetworkAssignDefaultPorts(ret) < 0)
- goto error;
+ virStorageSourceNetworkAssignDefaultPorts(ret);
}
return ret;
@@ -3964,66 +3954,61 @@ virStorageSourceFindByNodeName(virStorageSourcePtr top,
}
-static const char *
+static unsigned 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..d1e8ffe07 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;
+ unsigned 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..7d8995933 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, "\\\\:%u",
src->hosts[i].port);
}
}
--
2.13.2
5:02 a.m.
On Thu, Jul 20, 2017 at 02:29:57PM +0200, Peter Krempa wrote:
Since Michal did not want to finis this I did. I also found a few bugs
in
my previous patches which are addressed here.
Thank you for taking this up, even though you did not finish the word
'finish'.
Peter Krempa (6):
qemu: command: Rename and move qemuNetworkDriveGetPort
util: uri: Convert port number to unsigned integer
qemu: command: Remove condition to use default sheepdog port
util: storage: fill in default ports when parsing backing chain
conf: domain: Split up virDomainStorageHostParse and rename it
virStorageNetHostDef: Turn @port into integer
src/conf/domain_conf.c | 133 +++++++++++++++++++---------------
src/libvirt_private.syms | 1 +
src/libxl/libxl_conf.c | 2 +-
src/qemu/qemu_block.c | 7 +-
src/qemu/qemu_command.c | 30 ++------
src/qemu/qemu_parse_command.c | 13 ++--
src/storage/storage_backend_gluster.c | 17 +----
src/storage/storage_driver.c | 7 +-
src/util/virstoragefile.c | 68 +++++++----------
src/util/virstoragefile.h | 4 +-
src/util/virstring.c | 37 ++++++++++
src/util/virstring.h | 4 +
src/util/viruri.h | 2 +-
src/xenconfig/xen_xl.c | 2 +-
tests/virstoragetest.c | 14 ++--
15 files changed, 182 insertions(+), 159 deletions(-)
ACK series.
Jan
2678
days inactive
2682
days old
9 comments
3 participants
participants (3)
-
John Ferlan -
Ján Tomko -
Peter Krempa