[libvirt] [PATCH libvirt 1/2] domain: add <forward> element for user mode networking

Add element <forward> to add TCP or UDP port redirection from host to guest in user mode networking. --- docs/formatdomain.html.in | 21 ++++ docs/schemas/domaincommon.rng | 29 +++++ src/conf/domain_conf.c | 140 ++++++++++++++++++++++ src/conf/domain_conf.h | 23 ++++ tests/qemuargv2xmltest.c | 3 +- tests/qemuxml2argvdata/qemuxml2argv-net-user.xml | 2 + 6 files changed, 217 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d6e90f1..9c9bbf5 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2268,6 +2268,27 @@ VMs to have outgoing access. </p> + <p> + Port redirections from host to guest can be added by + providing <code>forward</code> elements that takes the + following attributes: + </p> + + <dl> + <dt><code>type</code></dt> + <dd>Either <code>tcp</code> (default) or <code>udp</code>.</dd> + + <dt><code>host_port</code></dt> + <dd>Host port to redirect.</dd> + + <dt><code>guest_port</code></dt> + <dd>Guest port to redirect to.</dd> + + <dt><code>host</code></dt> + <dd>IPv4 address to bound the redirection to a specific host + interface.</dd> + </dl> + <pre> ... <devices> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 34f63c3..740f5af 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1408,6 +1408,35 @@ <value>user</value> </attribute> <interleave> + <zeroOrMore> + <element name="forward"> + <attribute name="host_port"> + <ref name="PortNumber"/> + </attribute> + <attribute name="guest_port"> + <ref name="PortNumber"/> + </attribute> + <optional> + <attribute name="type"> + <choice> + <value>tcp</value> + <value>udp</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="host"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <optional> + <attribute name="guest"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <empty/> + </element> + </zeroOrMore> <ref name="interface-options"/> </interleave> </group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 51d6cb9..4b9b644 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -650,6 +650,10 @@ VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode, "static", "auto"); +VIR_ENUM_IMPL(virDomainNetForward, VIR_DOMAIN_NET_FORWARD_TYPE_LAST, + "tcp", + "udp") + #define virDomainReportError(code, ...) \ virReportErrorHelper(VIR_FROM_DOMAIN, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) @@ -1019,8 +1023,22 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def) VIR_FREE(def); } +void +virDomainNetForwardDefFree(virDomainNetForwardDefPtr def) +{ + if (!def) + return; + + VIR_FREE(def->host_addr); + VIR_FREE(def->guest_addr); + + VIR_FREE(def); +} + void virDomainNetDefFree(virDomainNetDefPtr def) { + int i; + if (!def) return; @@ -1066,6 +1084,11 @@ void virDomainNetDefFree(virDomainNetDefPtr def) break; case VIR_DOMAIN_NET_TYPE_USER: + for (i = 0; i < def->data.user.nforward; i++) + virDomainNetForwardDefFree(def->data.user.forwards[i]); + VIR_FREE(def->data.user.forwards); + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -4351,6 +4374,60 @@ error: return ret; } +static virDomainNetForwardDefPtr +virDomainNetForwardDefParseXML(const xmlNodePtr node) +{ + char *type = NULL; + char *host_port = NULL; + char *guest_port = NULL; + virDomainNetForwardDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + type = virXMLPropString(node, "type"); + if (type == NULL) { + def->type = VIR_DOMAIN_NET_FORWARD_TYPE_TCP; + } else if ((def->type = virDomainNetForwardTypeFromString(type)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown forward type '%s'"), type); + goto error; + } + + host_port = virXMLPropString(node, "host_port"); + if (!host_port || + virStrToLong_i(host_port, NULL, 10, &def->host_port) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <forward> 'host_port' attribute")); + goto error; + } + + guest_port = virXMLPropString(node, "guest_port"); + if (!guest_port || + virStrToLong_i(guest_port, NULL, 10, &def->guest_port) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <forward> 'guest_port' attribute")); + goto error; + } + + def->host_addr = virXMLPropString(node, "host"); + def->guest_addr = virXMLPropString(node, "guest"); + +cleanup: + VIR_FREE(type); + VIR_FREE(host_port); + VIR_FREE(guest_port); + + return def; + +error: + virDomainNetForwardDefFree(def); + def = NULL; + goto cleanup; +} + #define NET_MODEL_CHARS \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ091234567890_-" @@ -4394,6 +4471,8 @@ virDomainNetDefParseXML(virCapsPtr caps, virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt->node; int ret; + int nforwards; + xmlNodePtr *forwardNodes = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -4683,6 +4762,28 @@ virDomainNetDefParseXML(virCapsPtr caps, break; case VIR_DOMAIN_NET_TYPE_USER: + /* parse the <forward> elements */ + nforwards = virXPathNodeSet("./forward", ctxt, &forwardNodes); + if (nforwards < 0) + goto error; + + if (nforwards > 0) { + int i; + if (VIR_ALLOC_N(def->data.user.forwards, nforwards) < 0) { + virReportOOMError(); + goto error; + } + for (i = 0; i < nforwards; i++) { + virDomainNetForwardDefPtr fwd = + virDomainNetForwardDefParseXML(forwardNodes[i]); + if (fwd == NULL) + goto error; + def->data.user.forwards[def->data.user.nforward++] = fwd; + } + VIR_FREE(forwardNodes); + } + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -11413,11 +11514,42 @@ error: } static int +virDomainNetForwardDefFormat(virBufferPtr buf, + virDomainNetForwardDefPtr def) +{ + const char *type; + if (!def) + return 0; + + type = virDomainNetForwardTypeToString(def->type); + if (!type) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected net type %d"), def->type); + return -1; + } + virBufferAsprintf(buf, "<forward type='%s'", type); + + if (def->host_addr) + virBufferAsprintf(buf, " host='%s'", def->host_addr); + + virBufferAsprintf(buf, " host_port='%d'", def->host_port); + + if (def->guest_addr) + virBufferAsprintf(buf, " guest='%s'", def->guest_addr); + + virBufferAsprintf(buf, " guest_port='%d'", def->guest_port); + + virBufferAddLit(buf, "/>\n"); + return 0; +} + +static int virDomainNetDefFormat(virBufferPtr buf, virDomainNetDefPtr def, unsigned int flags) { const char *type = virDomainNetTypeToString(def->type); + int i; if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -11517,6 +11649,14 @@ virDomainNetDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_NET_TYPE_USER: + virBufferAdjustIndent(buf, 6); + for (i = 0; i < def->data.user.nforward; i++) { + if (virDomainNetForwardDefFormat(buf, def->data.user.forwards[i]) < 0) + return -1; + } + virBufferAdjustIndent(buf, -6); + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 70129fe..7fde05e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -768,6 +768,23 @@ struct _virDomainActualNetDef { virNetDevBandwidthPtr bandwidth; }; +enum virDomainNetForwardType { + VIR_DOMAIN_NET_FORWARD_TYPE_TCP, + VIR_DOMAIN_NET_FORWARD_TYPE_UDP, + + VIR_DOMAIN_NET_FORWARD_TYPE_LAST, +}; + +typedef struct _virDomainNetForwardDef virDomainNetForwardDef; +typedef virDomainNetForwardDef *virDomainNetForwardDefPtr; +struct _virDomainNetForwardDef { + int type; /* enum virDomainNetForwardType */ + char *host_addr; + int host_port; + char *guest_addr; + int guest_port; +}; + /* Stores the virtual network interface configuration */ struct _virDomainNetDef { enum virDomainNetType type; @@ -821,6 +838,10 @@ struct _virDomainNetDef { virDomainHostdevDef def; virNetDevVPortProfilePtr virtPortProfile; } hostdev; + struct { + int nforward; + virDomainNetForwardDefPtr *forwards; + } user; } data; struct { bool sndbuf_specified; @@ -1856,6 +1877,7 @@ int virDomainDiskFindControllerModel(virDomainDefPtr def, void virDomainControllerDefFree(virDomainControllerDefPtr def); void virDomainFSDefFree(virDomainFSDefPtr def); void virDomainActualNetDefFree(virDomainActualNetDefPtr def); +void virDomainNetForwardDefFree(virDomainNetForwardDefPtr def); void virDomainNetDefFree(virDomainNetDefPtr def); void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def); void virDomainChrDefFree(virDomainChrDefPtr def); @@ -2170,6 +2192,7 @@ VIR_ENUM_DECL(virDomainFSAccessMode) VIR_ENUM_DECL(virDomainFSWrpolicy) VIR_ENUM_DECL(virDomainNet) VIR_ENUM_DECL(virDomainNetBackend) +VIR_ENUM_DECL(virDomainNetForward) VIR_ENUM_DECL(virDomainNetVirtioTxMode) VIR_ENUM_DECL(virDomainNetInterfaceLinkState) VIR_ENUM_DECL(virDomainChrDevice) diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 439218e..cf2862b 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -207,7 +207,8 @@ mymain(void) DO_TEST("misc-acpi"); DO_TEST("misc-no-reboot"); DO_TEST("misc-uuid"); - DO_TEST("net-user"); + /* Fixed in following commit */ + /* DO_TEST("net-user"); */ DO_TEST("net-virtio"); DO_TEST("net-eth"); DO_TEST("net-eth-ifname"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml index 37e5edf..ecefdeb 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml @@ -23,6 +23,8 @@ <controller type='ide' index='0'/> <interface type='user'> <mac address='00:11:22:33:44:55'/> + <forward type='tcp' host_port='2222' guest_port='22'/> + <forward type='udp' host='127.0.0.1' host_port='2242' guest='10.0.2.15' guest_port='42'/> </interface> <memballoon model='virtio'/> </devices> -- 1.7.10.1

Implement port forwarding for user mode networking with QEMU. --- src/libvirt_private.syms | 3 + src/qemu/qemu_command.c | 115 +++++++++++++++++++++ tests/qemuargv2xmltest.c | 3 +- tests/qemuxml2argvdata/qemuxml2argv-net-user.args | 5 +- 4 files changed, 122 insertions(+), 4 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index afb308d..333284a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -396,6 +396,9 @@ virDomainNetInsert; virDomainNetRemove; virDomainNetRemoveByMac; virDomainNetTypeToString; +virDomainNetForwardDefFree; +virDomainNetForwardTypeFromString; +virDomainNetForwardTypeToString; virDomainNostateReasonTypeFromString; virDomainNostateReasonTypeToString; virDomainNumatuneMemModeTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 720f5b4..82c041c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2837,6 +2837,22 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, virBufferAsprintf(&buf, ",sndbuf=%lu", net->tune.sndbuf); } + if (netType == VIR_DOMAIN_NET_TYPE_USER) { + int i; + for (i = 0; i < net->data.user.nforward; ++i) { + virDomainNetForwardDefPtr fwd = net->data.user.forwards[i]; + const char *type = virDomainNetForwardTypeToString(fwd->type); + if (!type) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Invalid forward type")); + return NULL; + } + virBufferAsprintf(&buf, ",hostfwd=%s:%s:%d-%s:%d", type, + fwd->host_addr ? fwd->host_addr : "", fwd->host_port, + fwd->guest_addr ? fwd->guest_addr : "", fwd->guest_port); + } + } + if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); @@ -6704,6 +6720,82 @@ qemuFindNICForVLAN(int nnics, } +static virDomainNetForwardDefPtr +qemuParseNetForward(char *val) +{ + char *type, *host, *guest, *port; + virDomainNetForwardDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + type = val; + host = strchr(type, ':'); + if (!host) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse hostfwd host '%s'"), val); + goto error; + } + *host = '\0'; + host++; + guest = strchr(host, '-'); + if (!guest) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse hostfwd guest '%s'"), val); + goto error; + } + *guest = '\0'; + guest++; + + if (STREQ(type, "")) { + def->type = VIR_DOMAIN_NET_FORWARD_TYPE_TCP; + } else if ((def->type = virDomainNetForwardTypeFromString(type)) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown forward type '%s'"), type); + goto error; + } + + port = strchr(host, ':'); + if (!port) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing host port")); + goto error; + } + *port = '\0'; + port++; + if (!STREQ(host, "")) + def->host_addr = strdup(host); + if (virStrToLong_i(port, NULL, 10, &def->host_port) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse host port")); + goto error; + } + + port = strchr(guest, ':'); + if (!port) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing guest port")); + goto error; + } + *port = '\0'; + port++; + if (!STREQ(guest, "")) + def->guest_addr = strdup(guest); + if (virStrToLong_i(port, NULL, 10, &def->guest_port) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse guest port")); + goto error; + } + + return def; + +error: + virDomainNetForwardDefFree(def); + return NULL; +} + /* * Tries to parse a QEMU -net backend argument. Gets given * a list of all known -net frontend arguments to try and @@ -6723,6 +6815,7 @@ qemuParseCommandLineNet(virCapsPtr caps, int wantvlan = 0; const char *tmp; int genmac = 1; + int nforward = 0; int i; tmp = strchr(val, ','); @@ -6769,9 +6862,31 @@ qemuParseCommandLineNet(virCapsPtr caps, STREQ(keywords[i], "ifname")) { def->ifname = values[i]; values[i] = NULL; + } else if (def->type == VIR_DOMAIN_NET_TYPE_USER && + STREQ(keywords[i], "hostfwd")) { + nforward++; } } + if (nforward > 0) { + if (VIR_ALLOC_N(def->data.user.forwards, nforward) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < nkeywords ; i++) { + virDomainNetForwardDefPtr fwd; + if (def->type != VIR_DOMAIN_NET_TYPE_USER || + !STREQ(keywords[i], "hostfwd")) + continue; + + if ((fwd = qemuParseNetForward(values[i])) == NULL) + goto cleanup; + + def->data.user.forwards[def->data.user.nforward] = fwd; + def->data.user.nforward++; + } + } /* Done parsing the nic backend. Now to try and find corresponding * frontend, based off vlan number. NB this assumes a 1-1 mapping diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index cf2862b..439218e 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -207,8 +207,7 @@ mymain(void) DO_TEST("misc-acpi"); DO_TEST("misc-no-reboot"); DO_TEST("misc-uuid"); - /* Fixed in following commit */ - /* DO_TEST("net-user"); */ + DO_TEST("net-user"); DO_TEST("net-virtio"); DO_TEST("net-eth"); DO_TEST("net-eth-ifname"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user.args b/tests/qemuxml2argvdata/qemuxml2argv-net-user.args index 093ff01..db31e95 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-user.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user.args @@ -1,5 +1,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=00:11:22:33:44:55,vlan=0 -net user,vlan=0 -serial none -parallel none \ --usb +macaddr=00:11:22:33:44:55,vlan=0 \ +-net user,vlan=0,hostfwd=tcp::2222-:22,hostfwd=udp:127.0.0.1:2242-10.0.2.15:42 \ +-serial none -parallel none -usb -- 1.7.10.1

On Thu, May 10, 2012 at 02:10:45AM +0200, Marc-André Lureau wrote:
Implement port forwarding for user mode networking with QEMU. --- src/libvirt_private.syms | 3 + src/qemu/qemu_command.c | 115 +++++++++++++++++++++ tests/qemuargv2xmltest.c | 3 +- tests/qemuxml2argvdata/qemuxml2argv-net-user.args | 5 +- 4 files changed, 122 insertions(+), 4 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index afb308d..333284a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -396,6 +396,9 @@ virDomainNetInsert; virDomainNetRemove; virDomainNetRemoveByMac; virDomainNetTypeToString; +virDomainNetForwardDefFree; +virDomainNetForwardTypeFromString; +virDomainNetForwardTypeToString; virDomainNostateReasonTypeFromString; virDomainNostateReasonTypeToString; virDomainNumatuneMemModeTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 720f5b4..82c041c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2837,6 +2837,22 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, virBufferAsprintf(&buf, ",sndbuf=%lu", net->tune.sndbuf); }
+ if (netType == VIR_DOMAIN_NET_TYPE_USER) { + int i; + for (i = 0; i < net->data.user.nforward; ++i) { + virDomainNetForwardDefPtr fwd = net->data.user.forwards[i]; + const char *type = virDomainNetForwardTypeToString(fwd->type); + if (!type) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Invalid forward type")); + return NULL; + } + virBufferAsprintf(&buf, ",hostfwd=%s:%s:%d-%s:%d", type, + fwd->host_addr ? fwd->host_addr : "", fwd->host_port, + fwd->guest_addr ? fwd->guest_addr : "", fwd->guest_port); + } + } + if (virBufferError(&buf)) { virBufferFreeAndReset(&buf); virReportOOMError(); @@ -6704,6 +6720,82 @@ qemuFindNICForVLAN(int nnics, }
+static virDomainNetForwardDefPtr +qemuParseNetForward(char *val) +{ + char *type, *host, *guest, *port; + virDomainNetForwardDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + type = val; + host = strchr(type, ':'); + if (!host) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse hostfwd host '%s'"), val); + goto error; + } + *host = '\0'; + host++; + guest = strchr(host, '-'); + if (!guest) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse hostfwd guest '%s'"), val); + goto error; + } + *guest = '\0'; + guest++; + + if (STREQ(type, "")) { + def->type = VIR_DOMAIN_NET_FORWARD_TYPE_TCP; + } else if ((def->type = virDomainNetForwardTypeFromString(type)) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown forward type '%s'"), type); + goto error; + } + + port = strchr(host, ':'); + if (!port) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing host port")); + goto error; + } + *port = '\0'; + port++; + if (!STREQ(host, "")) + def->host_addr = strdup(host); + if (virStrToLong_i(port, NULL, 10, &def->host_port) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse host port")); + goto error; + } + + port = strchr(guest, ':'); + if (!port) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing guest port")); + goto error; + } + *port = '\0'; + port++; + if (!STREQ(guest, "")) + def->guest_addr = strdup(guest); + if (virStrToLong_i(port, NULL, 10, &def->guest_port) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse guest port")); + goto error; + } + + return def; + +error: + virDomainNetForwardDefFree(def); + return NULL; +} + /* * Tries to parse a QEMU -net backend argument. Gets given * a list of all known -net frontend arguments to try and @@ -6723,6 +6815,7 @@ qemuParseCommandLineNet(virCapsPtr caps, int wantvlan = 0; const char *tmp; int genmac = 1; + int nforward = 0; int i;
tmp = strchr(val, ','); @@ -6769,9 +6862,31 @@ qemuParseCommandLineNet(virCapsPtr caps, STREQ(keywords[i], "ifname")) { def->ifname = values[i]; values[i] = NULL; + } else if (def->type == VIR_DOMAIN_NET_TYPE_USER && + STREQ(keywords[i], "hostfwd")) { + nforward++; } }
+ if (nforward > 0) { + if (VIR_ALLOC_N(def->data.user.forwards, nforward) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < nkeywords ; i++) { + virDomainNetForwardDefPtr fwd; + if (def->type != VIR_DOMAIN_NET_TYPE_USER || + !STREQ(keywords[i], "hostfwd")) + continue; + + if ((fwd = qemuParseNetForward(values[i])) == NULL) + goto cleanup; + + def->data.user.forwards[def->data.user.nforward] = fwd; + def->data.user.nforward++; + } + }
/* Done parsing the nic backend. Now to try and find corresponding * frontend, based off vlan number. NB this assumes a 1-1 mapping diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index cf2862b..439218e 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -207,8 +207,7 @@ mymain(void) DO_TEST("misc-acpi"); DO_TEST("misc-no-reboot"); DO_TEST("misc-uuid"); - /* Fixed in following commit */ - /* DO_TEST("net-user"); */ + DO_TEST("net-user"); DO_TEST("net-virtio"); DO_TEST("net-eth"); DO_TEST("net-eth-ifname"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user.args b/tests/qemuxml2argvdata/qemuxml2argv-net-user.args index 093ff01..db31e95 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-user.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user.args @@ -1,5 +1,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=00:11:22:33:44:55,vlan=0 -net user,vlan=0 -serial none -parallel none \ --usb +macaddr=00:11:22:33:44:55,vlan=0 \ +-net user,vlan=0,hostfwd=tcp::2222-:22,hostfwd=udp:127.0.0.1:2242-10.0.2.15:42 \ +-serial none -parallel none -usb -- 1.7.10.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Reviewed-By: Hu Tao <hutao@cn.fujitsu.com> -- Thanks, Hu Tao

On Thu, May 10, 2012 at 02:10:44AM +0200, Marc-André Lureau wrote:
Add element <forward> to add TCP or UDP port redirection from host to guest in user mode networking. --- docs/formatdomain.html.in | 21 ++++ docs/schemas/domaincommon.rng | 29 +++++ src/conf/domain_conf.c | 140 ++++++++++++++++++++++ src/conf/domain_conf.h | 23 ++++ tests/qemuargv2xmltest.c | 3 +- tests/qemuxml2argvdata/qemuxml2argv-net-user.xml | 2 + 6 files changed, 217 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d6e90f1..9c9bbf5 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2268,6 +2268,27 @@ VMs to have outgoing access. </p>
+ <p> + Port redirections from host to guest can be added by + providing <code>forward</code> elements that takes the + following attributes: + </p> + + <dl> + <dt><code>type</code></dt> + <dd>Either <code>tcp</code> (default) or <code>udp</code>.</dd> + + <dt><code>host_port</code></dt> + <dd>Host port to redirect.</dd> + + <dt><code>guest_port</code></dt> + <dd>Guest port to redirect to.</dd> + + <dt><code>host</code></dt> + <dd>IPv4 address to bound the redirection to a specific host + interface.</dd> + </dl> + <pre> ... <devices> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 34f63c3..740f5af 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1408,6 +1408,35 @@ <value>user</value> </attribute> <interleave> + <zeroOrMore> + <element name="forward"> + <attribute name="host_port"> + <ref name="PortNumber"/> + </attribute> + <attribute name="guest_port"> + <ref name="PortNumber"/> + </attribute> + <optional> + <attribute name="type"> + <choice> + <value>tcp</value> + <value>udp</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="host"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <optional> + <attribute name="guest"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <empty/> + </element> + </zeroOrMore> <ref name="interface-options"/> </interleave> </group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 51d6cb9..4b9b644 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -650,6 +650,10 @@ VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode, "static", "auto");
+VIR_ENUM_IMPL(virDomainNetForward, VIR_DOMAIN_NET_FORWARD_TYPE_LAST, + "tcp", + "udp") + #define virDomainReportError(code, ...) \ virReportErrorHelper(VIR_FROM_DOMAIN, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) @@ -1019,8 +1023,22 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def) VIR_FREE(def); }
+void +virDomainNetForwardDefFree(virDomainNetForwardDefPtr def) +{ + if (!def) + return; + + VIR_FREE(def->host_addr); + VIR_FREE(def->guest_addr); + + VIR_FREE(def); +} + void virDomainNetDefFree(virDomainNetDefPtr def) { + int i; + if (!def) return;
@@ -1066,6 +1084,11 @@ void virDomainNetDefFree(virDomainNetDefPtr def) break;
case VIR_DOMAIN_NET_TYPE_USER: + for (i = 0; i < def->data.user.nforward; i++) + virDomainNetForwardDefFree(def->data.user.forwards[i]); + VIR_FREE(def->data.user.forwards); + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -4351,6 +4374,60 @@ error: return ret; }
+static virDomainNetForwardDefPtr +virDomainNetForwardDefParseXML(const xmlNodePtr node) +{ + char *type = NULL; + char *host_port = NULL; + char *guest_port = NULL; + virDomainNetForwardDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + type = virXMLPropString(node, "type"); + if (type == NULL) { + def->type = VIR_DOMAIN_NET_FORWARD_TYPE_TCP; + } else if ((def->type = virDomainNetForwardTypeFromString(type)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown forward type '%s'"), type); + goto error; + } + + host_port = virXMLPropString(node, "host_port"); + if (!host_port || + virStrToLong_i(host_port, NULL, 10, &def->host_port) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <forward> 'host_port' attribute")); + goto error; + } + + guest_port = virXMLPropString(node, "guest_port"); + if (!guest_port || + virStrToLong_i(guest_port, NULL, 10, &def->guest_port) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <forward> 'guest_port' attribute")); + goto error; + } + + def->host_addr = virXMLPropString(node, "host"); + def->guest_addr = virXMLPropString(node, "guest"); + +cleanup: + VIR_FREE(type); + VIR_FREE(host_port); + VIR_FREE(guest_port); + + return def; + +error: + virDomainNetForwardDefFree(def); + def = NULL; + goto cleanup; +} + #define NET_MODEL_CHARS \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ091234567890_-"
@@ -4394,6 +4471,8 @@ virDomainNetDefParseXML(virCapsPtr caps, virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt->node; int ret; + int nforwards; + xmlNodePtr *forwardNodes = NULL;
if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -4683,6 +4762,28 @@ virDomainNetDefParseXML(virCapsPtr caps, break;
case VIR_DOMAIN_NET_TYPE_USER: + /* parse the <forward> elements */ + nforwards = virXPathNodeSet("./forward", ctxt, &forwardNodes); + if (nforwards < 0) + goto error; + + if (nforwards > 0) { + int i; + if (VIR_ALLOC_N(def->data.user.forwards, nforwards) < 0) { + virReportOOMError(); + goto error; + } + for (i = 0; i < nforwards; i++) { + virDomainNetForwardDefPtr fwd = + virDomainNetForwardDefParseXML(forwardNodes[i]); + if (fwd == NULL) + goto error; + def->data.user.forwards[def->data.user.nforward++] = fwd; + } + VIR_FREE(forwardNodes); + } + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -11413,11 +11514,42 @@ error: }
static int +virDomainNetForwardDefFormat(virBufferPtr buf, + virDomainNetForwardDefPtr def) +{ + const char *type; + if (!def) + return 0; + + type = virDomainNetForwardTypeToString(def->type); + if (!type) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected net type %d"), def->type); + return -1; + } + virBufferAsprintf(buf, "<forward type='%s'", type); + + if (def->host_addr) + virBufferAsprintf(buf, " host='%s'", def->host_addr); + + virBufferAsprintf(buf, " host_port='%d'", def->host_port); + + if (def->guest_addr) + virBufferAsprintf(buf, " guest='%s'", def->guest_addr); + + virBufferAsprintf(buf, " guest_port='%d'", def->guest_port); + + virBufferAddLit(buf, "/>\n"); + return 0; +} + +static int virDomainNetDefFormat(virBufferPtr buf, virDomainNetDefPtr def, unsigned int flags) { const char *type = virDomainNetTypeToString(def->type); + int i;
if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -11517,6 +11649,14 @@ virDomainNetDefFormat(virBufferPtr buf, break;
case VIR_DOMAIN_NET_TYPE_USER: + virBufferAdjustIndent(buf, 6); + for (i = 0; i < def->data.user.nforward; i++) { + if (virDomainNetForwardDefFormat(buf, def->data.user.forwards[i]) < 0) + return -1; + } + virBufferAdjustIndent(buf, -6); + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 70129fe..7fde05e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -768,6 +768,23 @@ struct _virDomainActualNetDef { virNetDevBandwidthPtr bandwidth; };
+enum virDomainNetForwardType { + VIR_DOMAIN_NET_FORWARD_TYPE_TCP, + VIR_DOMAIN_NET_FORWARD_TYPE_UDP, + + VIR_DOMAIN_NET_FORWARD_TYPE_LAST, +}; + +typedef struct _virDomainNetForwardDef virDomainNetForwardDef; +typedef virDomainNetForwardDef *virDomainNetForwardDefPtr; +struct _virDomainNetForwardDef { + int type; /* enum virDomainNetForwardType */ + char *host_addr; + int host_port; + char *guest_addr; + int guest_port; +}; + /* Stores the virtual network interface configuration */ struct _virDomainNetDef { enum virDomainNetType type; @@ -821,6 +838,10 @@ struct _virDomainNetDef { virDomainHostdevDef def; virNetDevVPortProfilePtr virtPortProfile; } hostdev; + struct { + int nforward; + virDomainNetForwardDefPtr *forwards; + } user; } data; struct { bool sndbuf_specified; @@ -1856,6 +1877,7 @@ int virDomainDiskFindControllerModel(virDomainDefPtr def, void virDomainControllerDefFree(virDomainControllerDefPtr def); void virDomainFSDefFree(virDomainFSDefPtr def); void virDomainActualNetDefFree(virDomainActualNetDefPtr def); +void virDomainNetForwardDefFree(virDomainNetForwardDefPtr def); void virDomainNetDefFree(virDomainNetDefPtr def); void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def); void virDomainChrDefFree(virDomainChrDefPtr def); @@ -2170,6 +2192,7 @@ VIR_ENUM_DECL(virDomainFSAccessMode) VIR_ENUM_DECL(virDomainFSWrpolicy) VIR_ENUM_DECL(virDomainNet) VIR_ENUM_DECL(virDomainNetBackend) +VIR_ENUM_DECL(virDomainNetForward) VIR_ENUM_DECL(virDomainNetVirtioTxMode) VIR_ENUM_DECL(virDomainNetInterfaceLinkState) VIR_ENUM_DECL(virDomainChrDevice) diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 439218e..cf2862b 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -207,7 +207,8 @@ mymain(void) DO_TEST("misc-acpi"); DO_TEST("misc-no-reboot"); DO_TEST("misc-uuid"); - DO_TEST("net-user"); + /* Fixed in following commit */ + /* DO_TEST("net-user"); */ DO_TEST("net-virtio"); DO_TEST("net-eth"); DO_TEST("net-eth-ifname"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml index 37e5edf..ecefdeb 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml @@ -23,6 +23,8 @@ <controller type='ide' index='0'/> <interface type='user'> <mac address='00:11:22:33:44:55'/> + <forward type='tcp' host_port='2222' guest_port='22'/> + <forward type='udp' host='127.0.0.1' host_port='2242' guest='10.0.2.15' guest_port='42'/> </interface> <memballoon model='virtio'/> </devices> -- 1.7.10.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Reviewed-By: Hu Tao <hutao@cn.fujitsu.com> -- Thanks, Hu Tao

On Thu, May 10, 2012 at 02:10:44AM +0200, Marc-André Lureau wrote:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 70129fe..7fde05e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -768,6 +768,23 @@ struct _virDomainActualNetDef { virNetDevBandwidthPtr bandwidth; };
+enum virDomainNetForwardType { + VIR_DOMAIN_NET_FORWARD_TYPE_TCP, + VIR_DOMAIN_NET_FORWARD_TYPE_UDP, + + VIR_DOMAIN_NET_FORWARD_TYPE_LAST, +}; + +typedef struct _virDomainNetForwardDef virDomainNetForwardDef; +typedef virDomainNetForwardDef *virDomainNetForwardDefPtr; +struct _virDomainNetForwardDef { + int type; /* enum virDomainNetForwardType */
Lets call this 'protocol' since it matches the terminology used by ip[6]tables
+ char *host_addr; + int host_port; + char *guest_addr; + int guest_port; +};
Our current best practice is to store all IP addresses using virSocketAddr rather than char *. This ensures we perform syntactic validation at parse time. It also means when generating the rules we can easily distinguish between ipv4 and ipv6 addresses. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Hi Ah, I just realized that there is 'guestfwd' support already, so we may want to use the same syntax <channel type='tcp'> <source mode='bind' host='127.0.0.1' service='2222'/> <target type='hostfwd' address='10.0.2.15' port='22'/> </channel> It seems a bit contrieved to me, it is harder to interpret. 'host' address and guest 'address' are no longer optional. There is no way to specify the protocol. However, I imagined we may want to move the <forward/> element I added to the devices top level, since it is not bound to an interface device in fact. Please comment, I need some guidance. -- Marc-André Lureau

On Mon, May 14, 2012 at 01:29:11PM +0200, Marc-André Lureau wrote:
Hi
Ah, I just realized that there is 'guestfwd' support already, so we may want to use the same syntax
<channel type='tcp'> <source mode='bind' host='127.0.0.1' service='2222'/> <target type='hostfwd' address='10.0.2.15' port='22'/> </channel>
It seems a bit contrieved to me, it is harder to interpret. 'host' address and guest 'address' are no longer optional. There is no way to specify the protocol.
This device was a bit of a nasty hack that we probably shouldn't have done.
However, I imagined we may want to move the <forward/> element I added to the devices top level, since it is not bound to an interface device in fact.
I like your use of <forward> inside the <interface> element since that makes it applicable to more than just the type=user networking. eg the libvirt type=default NAT networking could also use this syntax Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 05/09/2012 08:10 PM, Marc-André Lureau wrote:
Add element <forward> to add TCP or UDP port redirection from host to guest in user mode networking. --- docs/formatdomain.html.in | 21 ++++ docs/schemas/domaincommon.rng | 29 +++++ src/conf/domain_conf.c | 140 ++++++++++++++++++++++ src/conf/domain_conf.h | 23 ++++ tests/qemuargv2xmltest.c | 3 +- tests/qemuxml2argvdata/qemuxml2argv-net-user.xml | 2 + 6 files changed, 217 insertions(+), 1 deletion(-)
This is actually a very common request for users of libvirt's default network (which connects the guest to a host bridge via a tap interface, and does NAT via iptables rules), so I think it's very important that the same functionality be achievable using the same XML with that networking setup. I had been thinking that port forwarding should be added to nwfilter rules, since they deal with iptables rules, and it's also iptables rules that will be used to add port forwarding. But your use case (user mode networking) points out that it really should be separated from nwfilter. Of course, in the case of libvirt virtual networks, we will need to add both a DNAT rule and a FORWARD rule (which could already be added by nwfilter). Another thought I had was that it might be nice to use similar attributes to nwfilter rules as much as possible, but nwfilter doesn't have any concept of host vs. guest, so it's a bit limited. However, here are some suggested names, which are at least closer: Current Suggested similar nwfilter ------- --------- ---------------- type protocol protocol (I see Dan already suggested this one) host_port hostport dstportstart guest_port guestport dstportstart host hostipaddr dstipaddr guest guestipaddr dstipaddr Looking ahead to adding support for this with other network types - does it sound like a reasonable idea to add/remove the rules for it in networkAllocateActualDevice() and networkReleaseActualDevice()?
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d6e90f1..9c9bbf5 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2268,6 +2268,27 @@ VMs to have outgoing access. </p>
+ <p> + Port redirections from host to guest can be added by + providing <code>forward</code> elements that takes the + following attributes: + </p> + + <dl> + <dt><code>type</code></dt> + <dd>Either <code>tcp</code> (default) or <code>udp</code>.</dd> + + <dt><code>host_port</code></dt> + <dd>Host port to redirect.</dd> + + <dt><code>guest_port</code></dt> + <dd>Guest port to redirect to.</dd> + + <dt><code>host</code></dt> + <dd>IPv4 address to bound the redirection to a specific host
Would there ever be a case where 1) a domain name was used in one or the other place, or 2) a proxy was used to redirect from IPv6 to IPv6, or translate between the two? (I suppose the RNG could always be expanded without change to the attribute name, if this was ever needed).
+ interface.</dd> + </dl> + <pre> ... <devices> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 34f63c3..740f5af 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1408,6 +1408,35 @@ <value>user</value> </attribute> <interleave> + <zeroOrMore> + <element name="forward"> + <attribute name="host_port"> + <ref name="PortNumber"/> + </attribute> + <attribute name="guest_port"> + <ref name="PortNumber"/> + </attribute> + <optional> + <attribute name="type"> + <choice> + <value>tcp</value> + <value>udp</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="host"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <optional> + <attribute name="guest"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <empty/> + </element> + </zeroOrMore>
I think this should be in the common part of <interface> rather than user-specific. This isn't something inherently specific to type='user' (as a matter of fact, I would like to expand it fairly soon after you're done to also work for type='network'), and restricting it as you're doing here will just need to be undone later (including moving things around in the virDomainNetDef struct, etc). I think instead the RNG, parser, and formatter should allow it for all types of interface, and the hypervisor driver should log a VIR_ERR_CONFIG_UNSUPPORTED error and fail to attach if <forward> is given for an interface type that doesn't support it.
<ref name="interface-options"/> </interleave> </group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 51d6cb9..4b9b644 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -650,6 +650,10 @@ VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode, "static", "auto");
+VIR_ENUM_IMPL(virDomainNetForward, VIR_DOMAIN_NET_FORWARD_TYPE_LAST, + "tcp", + "udp") + #define virDomainReportError(code, ...) \ virReportErrorHelper(VIR_FROM_DOMAIN, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) @@ -1019,8 +1023,22 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def) VIR_FREE(def); }
+void +virDomainNetForwardDefFree(virDomainNetForwardDefPtr def) +{ + if (!def) + return; + + VIR_FREE(def->host_addr); + VIR_FREE(def->guest_addr); + + VIR_FREE(def); +} + void virDomainNetDefFree(virDomainNetDefPtr def) { + int i; + if (!def) return;
@@ -1066,6 +1084,11 @@ void virDomainNetDefFree(virDomainNetDefPtr def) break;
case VIR_DOMAIN_NET_TYPE_USER: + for (i = 0; i < def->data.user.nforward; i++) + virDomainNetForwardDefFree(def->data.user.forwards[i]); + VIR_FREE(def->data.user.forwards); + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -4351,6 +4374,60 @@ error: return ret; }
+static virDomainNetForwardDefPtr +virDomainNetForwardDefParseXML(const xmlNodePtr node) +{ + char *type = NULL; + char *host_port = NULL; + char *guest_port = NULL; + virDomainNetForwardDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + type = virXMLPropString(node, "type"); + if (type == NULL) { + def->type = VIR_DOMAIN_NET_FORWARD_TYPE_TCP; + } else if ((def->type = virDomainNetForwardTypeFromString(type)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown forward type '%s'"), type); + goto error;
If you take this goto, you'll call virDomainNetForwardDefFree on an uninitialized def. You should initialize it to NULL.
+ } + + host_port = virXMLPropString(node, "host_port"); + if (!host_port || + virStrToLong_i(host_port, NULL, 10, &def->host_port) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <forward> 'host_port' attribute")); + goto error; + } + + guest_port = virXMLPropString(node, "guest_port"); + if (!guest_port || + virStrToLong_i(guest_port, NULL, 10, &def->guest_port) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <forward> 'guest_port' attribute")); + goto error; + } + + def->host_addr = virXMLPropString(node, "host"); + def->guest_addr = virXMLPropString(node, "guest"); + +cleanup: + VIR_FREE(type); + VIR_FREE(host_port); + VIR_FREE(guest_port); + + return def; + +error: + virDomainNetForwardDefFree(def); + def = NULL; + goto cleanup; +} + #define NET_MODEL_CHARS \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ091234567890_-"
@@ -4394,6 +4471,8 @@ virDomainNetDefParseXML(virCapsPtr caps, virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt->node; int ret; + int nforwards; + xmlNodePtr *forwardNodes = NULL;
if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -4683,6 +4762,28 @@ virDomainNetDefParseXML(virCapsPtr caps, break;
case VIR_DOMAIN_NET_TYPE_USER: + /* parse the <forward> elements */ + nforwards = virXPathNodeSet("./forward", ctxt, &forwardNodes); + if (nforwards < 0) + goto error;
Since you haven't modified the code down at error:, any error after this point will leak forwardNodes - you need to call VIR_FREE(forwardNodes) at error:
+ + if (nforwards > 0) { + int i; + if (VIR_ALLOC_N(def->data.user.forwards, nforwards) < 0) { + virReportOOMError(); + goto error; + } + for (i = 0; i < nforwards; i++) { + virDomainNetForwardDefPtr fwd = + virDomainNetForwardDefParseXML(forwardNodes[i]); + if (fwd == NULL) + goto error; + def->data.user.forwards[def->data.user.nforward++] = fwd; + } + VIR_FREE(forwardNodes); + } + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -11413,11 +11514,42 @@ error: }
static int +virDomainNetForwardDefFormat(virBufferPtr buf, + virDomainNetForwardDefPtr def) +{ + const char *type; + if (!def) + return 0; + + type = virDomainNetForwardTypeToString(def->type); + if (!type) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected net type %d"), def->type); + return -1; + } + virBufferAsprintf(buf, "<forward type='%s'", type); + + if (def->host_addr) + virBufferAsprintf(buf, " host='%s'", def->host_addr); + + virBufferAsprintf(buf, " host_port='%d'", def->host_port); + + if (def->guest_addr) + virBufferAsprintf(buf, " guest='%s'", def->guest_addr); + + virBufferAsprintf(buf, " guest_port='%d'", def->guest_port); + + virBufferAddLit(buf, "/>\n"); + return 0; +} + +static int virDomainNetDefFormat(virBufferPtr buf, virDomainNetDefPtr def, unsigned int flags) { const char *type = virDomainNetTypeToString(def->type); + int i;
if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -11517,6 +11649,14 @@ virDomainNetDefFormat(virBufferPtr buf, break;
case VIR_DOMAIN_NET_TYPE_USER: + virBufferAdjustIndent(buf, 6); + for (i = 0; i < def->data.user.nforward; i++) { + if (virDomainNetForwardDefFormat(buf, def->data.user.forwards[i]) < 0) + return -1; + } + virBufferAdjustIndent(buf, -6); + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 70129fe..7fde05e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -768,6 +768,23 @@ struct _virDomainActualNetDef { virNetDevBandwidthPtr bandwidth; };
+enum virDomainNetForwardType { + VIR_DOMAIN_NET_FORWARD_TYPE_TCP, + VIR_DOMAIN_NET_FORWARD_TYPE_UDP, + + VIR_DOMAIN_NET_FORWARD_TYPE_LAST, +}; + +typedef struct _virDomainNetForwardDef virDomainNetForwardDef; +typedef virDomainNetForwardDef *virDomainNetForwardDefPtr; +struct _virDomainNetForwardDef { + int type; /* enum virDomainNetForwardType */ + char *host_addr; + int host_port; + char *guest_addr; + int guest_port; +}; + /* Stores the virtual network interface configuration */ struct _virDomainNetDef { enum virDomainNetType type; @@ -821,6 +838,10 @@ struct _virDomainNetDef { virDomainHostdevDef def; virNetDevVPortProfilePtr virtPortProfile; } hostdev; + struct { + int nforward; + virDomainNetForwardDefPtr *forwards; + } user;
} data; struct { bool sndbuf_specified; @@ -1856,6 +1877,7 @@ int virDomainDiskFindControllerModel(virDomainDefPtr def, void virDomainControllerDefFree(virDomainControllerDefPtr def); void virDomainFSDefFree(virDomainFSDefPtr def); void virDomainActualNetDefFree(virDomainActualNetDefPtr def); +void virDomainNetForwardDefFree(virDomainNetForwardDefPtr def); void virDomainNetDefFree(virDomainNetDefPtr def); void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def); void virDomainChrDefFree(virDomainChrDefPtr def); @@ -2170,6 +2192,7 @@ VIR_ENUM_DECL(virDomainFSAccessMode) VIR_ENUM_DECL(virDomainFSWrpolicy) VIR_ENUM_DECL(virDomainNet) VIR_ENUM_DECL(virDomainNetBackend) +VIR_ENUM_DECL(virDomainNetForward) VIR_ENUM_DECL(virDomainNetVirtioTxMode) VIR_ENUM_DECL(virDomainNetInterfaceLinkState) VIR_ENUM_DECL(virDomainChrDevice) diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 439218e..cf2862b 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -207,7 +207,8 @@ mymain(void) DO_TEST("misc-acpi"); DO_TEST("misc-no-reboot"); DO_TEST("misc-uuid"); - DO_TEST("net-user"); + /* Fixed in following commit */ + /* DO_TEST("net-user"); */
It would be much nicer to make the test work rather than disable it (even for one commit), unless it's really hairy to fix it.
DO_TEST("net-virtio"); DO_TEST("net-eth"); DO_TEST("net-eth-ifname"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml index 37e5edf..ecefdeb 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml @@ -23,6 +23,8 @@ <controller type='ide' index='0'/> <interface type='user'> <mac address='00:11:22:33:44:55'/> + <forward type='tcp' host_port='2222' guest_port='22'/> + <forward type='udp' host='127.0.0.1' host_port='2242' guest='10.0.2.15' guest_port='42'/>
Ah, I see. You're adding new XML, so you want it represented in the xml2xml tests, but you haven't added the backend, so it will fail the xml2argv test? But since unrecognized XML is just ignored, leaving the argv file unmodified should do it for you, right?
</interface> <memballoon model='virtio'/> </devices>

Hi Some of your reviews are already adressed in the new patch: http://www.redhat.com/archives/libvir-list/2012-May/msg00834.html On Tue, May 15, 2012 at 10:58 PM, Laine Stump <laine@laine.org> wrote:
Current Suggested similar nwfilter ------- --------- ---------------- type protocol protocol (I see Dan already suggested this one) host_port hostport dstportstart guest_port guestport dstportstart host hostipaddr dstipaddr guest guestipaddr dstipaddr
I already renamed type to protocol. I don't mind renaming the rest of the names for the one you suggested.
I think this should be in the common part of <interface> rather than user-specific. This isn't something inherently specific to type='user' (as a matter of fact, I would like to expand it fairly soon after you're done to also work for type='network'), and restricting it as you're doing here will just need to be undone later (including moving things around in the virDomainNetDef struct, etc). I think instead the RNG, parser, and formatter should allow it for all types of interface, and the hypervisor driver should log a VIR_ERR_CONFIG_UNSUPPORTED error and fail to attach if <forward> is given for an interface type that doesn't support it.
As you say, It can be un-restricted once it is implemented for other types.
<ref name="interface-options"/> </interleave> </group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 51d6cb9..4b9b644 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -650,6 +650,10 @@ VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode, "static", "auto");
+VIR_ENUM_IMPL(virDomainNetForward, VIR_DOMAIN_NET_FORWARD_TYPE_LAST, + "tcp", + "udp") + #define virDomainReportError(code, ...) \ virReportErrorHelper(VIR_FROM_DOMAIN, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) @@ -1019,8 +1023,22 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def) VIR_FREE(def); }
+void +virDomainNetForwardDefFree(virDomainNetForwardDefPtr def) +{ + if (!def) + return; + + VIR_FREE(def->host_addr); + VIR_FREE(def->guest_addr); + + VIR_FREE(def); +} + void virDomainNetDefFree(virDomainNetDefPtr def) { + int i; + if (!def) return;
@@ -1066,6 +1084,11 @@ void virDomainNetDefFree(virDomainNetDefPtr def) break;
case VIR_DOMAIN_NET_TYPE_USER: + for (i = 0; i < def->data.user.nforward; i++) + virDomainNetForwardDefFree(def->data.user.forwards[i]); + VIR_FREE(def->data.user.forwards); + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -4351,6 +4374,60 @@ error: return ret; }
+static virDomainNetForwardDefPtr +virDomainNetForwardDefParseXML(const xmlNodePtr node) +{ + char *type = NULL; + char *host_port = NULL; + char *guest_port = NULL; + virDomainNetForwardDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + type = virXMLPropString(node, "type"); + if (type == NULL) { + def->type = VIR_DOMAIN_NET_FORWARD_TYPE_TCP; + } else if ((def->type = virDomainNetForwardTypeFromString(type)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown forward type '%s'"), type); + goto error;
If you take this goto, you'll call virDomainNetForwardDefFree on an uninitialized def. You should initialize it to NULL.
I am not sure I follow you. Calling virDomainNetForwardDefFree() after VIR_ALLOC(def) should be okay, no?
+ } + + host_port = virXMLPropString(node, "host_port"); + if (!host_port || + virStrToLong_i(host_port, NULL, 10, &def->host_port) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <forward> 'host_port' attribute")); + goto error; + } + + guest_port = virXMLPropString(node, "guest_port"); + if (!guest_port || + virStrToLong_i(guest_port, NULL, 10, &def->guest_port) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <forward> 'guest_port' attribute")); + goto error; + } + + def->host_addr = virXMLPropString(node, "host"); + def->guest_addr = virXMLPropString(node, "guest"); + +cleanup: + VIR_FREE(type); + VIR_FREE(host_port); + VIR_FREE(guest_port); + + return def; + +error: + virDomainNetForwardDefFree(def); + def = NULL; + goto cleanup; +} + #define NET_MODEL_CHARS \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ091234567890_-"
@@ -4394,6 +4471,8 @@ virDomainNetDefParseXML(virCapsPtr caps, virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt->node; int ret; + int nforwards; + xmlNodePtr *forwardNodes = NULL;
if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -4683,6 +4762,28 @@ virDomainNetDefParseXML(virCapsPtr caps, break;
case VIR_DOMAIN_NET_TYPE_USER: + /* parse the <forward> elements */ + nforwards = virXPathNodeSet("./forward", ctxt, &forwardNodes); + if (nforwards < 0) + goto error;
Since you haven't modified the code down at error:, any error after this point will leak forwardNodes - you need to call VIR_FREE(forwardNodes) at error:
Correct, this needs fixing.
It would be much nicer to make the test work rather than disable it (even for one commit), unless it's really hairy to fix it.
+ <forward type='tcp' host_port='2222' guest_port='22'/> + <forward type='udp' host='127.0.0.1' host_port='2242' guest='10.0.2.15' guest_port='42'/>
Ah, I see. You're adding new XML, so you want it represented in the xml2xml tests, but you haven't added the backend, so it will fail the xml2argv test? But since unrecognized XML is just ignored, leaving the argv file unmodified should do it for you, right?
It will fail because the resulting XML isn't the same, iirc. -- Marc-André Lureau

On 05/15/2012 05:47 PM, Marc-André Lureau wrote:
Hi
Some of your reviews are already adressed in the new patch: http://www.redhat.com/archives/libvir-list/2012-May/msg00834.html
On Tue, May 15, 2012 at 10:58 PM, Laine Stump <laine@laine.org> wrote:
I think this should be in the common part of <interface> rather than user-specific. This isn't something inherently specific to type='user' (as a matter of fact, I would like to expand it fairly soon after you're done to also work for type='network'), and restricting it as you're doing here will just need to be undone later (including moving things around in the virDomainNetDef struct, etc). I think instead the RNG, parser, and formatter should allow it for all types of interface, and the hypervisor driver should log a VIR_ERR_CONFIG_UNSUPPORTED error and fail to attach if <forward> is given for an interface type that doesn't support it. As you say, It can be un-restricted once it is implemented for other types.
Yes, but let's avoid the code churn of needing to move it around in the struct and changing the parser/formatter - it's better for them to have it in the main structure from the beginning, and just restricted in the drivers where it is used (or not). That way, as the hypervisor drivers add support, only the driver code itself will need to be changed (and only for that particular driver).
<ref name="interface-options"/> </interleave> </group> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 51d6cb9..4b9b644 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -650,6 +650,10 @@ VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode, "static", "auto");
+VIR_ENUM_IMPL(virDomainNetForward, VIR_DOMAIN_NET_FORWARD_TYPE_LAST, + "tcp", + "udp") + #define virDomainReportError(code, ...) \ virReportErrorHelper(VIR_FROM_DOMAIN, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) @@ -1019,8 +1023,22 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def) VIR_FREE(def); }
+void +virDomainNetForwardDefFree(virDomainNetForwardDefPtr def) +{ + if (!def) + return; + + VIR_FREE(def->host_addr); + VIR_FREE(def->guest_addr); + + VIR_FREE(def); +} + void virDomainNetDefFree(virDomainNetDefPtr def) { + int i; + if (!def) return;
@@ -1066,6 +1084,11 @@ void virDomainNetDefFree(virDomainNetDefPtr def) break;
case VIR_DOMAIN_NET_TYPE_USER: + for (i = 0; i < def->data.user.nforward; i++) + virDomainNetForwardDefFree(def->data.user.forwards[i]); + VIR_FREE(def->data.user.forwards); + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -4351,6 +4374,60 @@ error: return ret; }
+static virDomainNetForwardDefPtr +virDomainNetForwardDefParseXML(const xmlNodePtr node) +{ + char *type = NULL; + char *host_port = NULL; + char *guest_port = NULL; + virDomainNetForwardDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + type = virXMLPropString(node, "type"); + if (type == NULL) { + def->type = VIR_DOMAIN_NET_FORWARD_TYPE_TCP; + } else if ((def->type = virDomainNetForwardTypeFromString(type)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown forward type '%s'"), type); + goto error;
If you take this goto, you'll call virDomainNetForwardDefFree on an uninitialized def. You should initialize it to NULL.
I am not sure I follow you. Calling virDomainNetForwardDefFree() after VIR_ALLOC(def) should be okay, no?
You're correct. I guess I was moving over the code too quickly.
It would be much nicer to make the test work rather than disable it (even for one commit), unless it's really hairy to fix it.
+ <forward type='tcp' host_port='2222' guest_port='22'/> + <forward type='udp' host='127.0.0.1' host_port='2242' guest='10.0.2.15' guest_port='42'/> Ah, I see. You're adding new XML, so you want it represented in the xml2xml tests, but you haven't added the backend, so it will fail the xml2argv test? But since unrecognized XML is just ignored, leaving the argv file unmodified should do it for you, right? It will fail because the resulting XML isn't the same, iirc.
(now that I've taken the time to look *carefully*). Okay, I see now that the problematic test is argv2xml, which of course won't generate the expected XML, and the test framework isn't setup to allow for a different xmlout (as is the case for xml2xml), so temporarily disabling the test really is the most sane thing to do.

On Wed, May 16, 2012 at 5:51 PM, Laine Stump <laine@laine.org> wrote:
Yes, but let's avoid the code churn of needing to move it around in the struct and changing the parser/formatter - it's better for them to have it in the main structure from the beginning, and just restricted in the drivers where it is used (or not). That way, as the hypervisor drivers add support, only the driver code itself will need to be changed (and only for that particular driver).
ok, I'll be updating the patch. -- Marc-André Lureau

Add element <forward> to add TCP or UDP port redirection from host to guest in user mode networking. --- docs/formatdomain.html.in | 25 ++++ docs/schemas/domaincommon.rng | 29 ++++ src/conf/domain_conf.c | 165 ++++++++++++++++++++++ src/conf/domain_conf.h | 23 +++ tests/qemuargv2xmltest.c | 3 +- tests/qemuxml2argvdata/qemuxml2argv-net-user.xml | 2 + 6 files changed, 246 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3875167..255e91b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2268,6 +2268,31 @@ VMs to have outgoing access. </p> + <p> + <span class="since">Since 0.9.13</span>, port redirections from + host to guest can be added by providing <code>forward</code> + elements that takes the following attributes: + </p> + + <dl> + <dt><code>protocol</code></dt> + <dd>Either <code>tcp</code> (default) or <code>udp</code>.</dd> + + <dt><code>hostport</code></dt> + <dd>Host port to redirect.</dd> + + <dt><code>guestport</code></dt> + <dd>Guest port to redirect to.</dd> + + <dt><code>hostipaddr</code></dt> + <dd>IPv4 address to bound the redirection to a specific host + interface.</dd> + + <dt><code>guestipaddr</code></dt> + <dd>IPv4 address to bound the redirection to a specific guest + interface.</dd> + </dl> + <pre> ... <devices> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 34f63c3..b9943dc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1529,6 +1529,35 @@ </interleave> </group> </choice> + <zeroOrMore> + <element name="forward"> + <attribute name="hostport"> + <ref name="PortNumber"/> + </attribute> + <attribute name="guestport"> + <ref name="PortNumber"/> + </attribute> + <optional> + <attribute name="protocol"> + <choice> + <value>tcp</value> + <value>udp</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="hostaddr"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <optional> + <attribute name="guestaddr"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <empty/> + </element> + </zeroOrMore> </element> </define> <!-- diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 78755cf..219c10a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -650,6 +650,11 @@ VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode, "static", "auto"); + +VIR_ENUM_IMPL(virDomainNetForwardProtocol, VIR_DOMAIN_NET_FORWARD_PROTOCOL_LAST, + "tcp", + "udp") + #define virDomainReportError(code, ...) \ virReportErrorHelper(VIR_FROM_DOMAIN, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) @@ -1019,8 +1024,19 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def) VIR_FREE(def); } +void +virDomainNetForwardDefFree(virDomainNetForwardDefPtr def) +{ + if (!def) + return; + + VIR_FREE(def); +} + void virDomainNetDefFree(virDomainNetDefPtr def) { + int i; + if (!def) return; @@ -1080,6 +1096,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def) virNetDevBandwidthFree(def->bandwidth); + for (i = 0; i < def->nforward; i++) + virDomainNetForwardDefFree(def->forwards[i]); + VIR_FREE(def->forwards); + VIR_FREE(def); } @@ -4380,6 +4400,81 @@ error: return ret; } +static virDomainNetForwardDefPtr +virDomainNetForwardDefParseXML(const xmlNodePtr node) +{ + char *protocol = NULL; + char *hostaddr = NULL; + char *guestaddr = NULL; + char *hostport = NULL; + char *guestport = NULL; + virDomainNetForwardDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + protocol = virXMLPropString(node, "protocol"); + if (protocol == NULL) { + def->protocol = VIR_DOMAIN_NET_FORWARD_PROTOCOL_TCP; + } else if ((def->protocol = virDomainNetForwardProtocolTypeFromString(protocol)) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("unknown forward protocol '%s'"), protocol); + goto error; + } + + hostport = virXMLPropString(node, "hostport"); + if (!hostport || + virStrToLong_i(hostport, NULL, 10, &def->hostport) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("Cannot parse <forward> 'hostport' attribute")); + goto error; + } + + guestport = virXMLPropString(node, "guestport"); + if (!guestport || + virStrToLong_i(guestport, NULL, 10, &def->guestport) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("Cannot parse <forward> 'guestport' attribute")); + goto error; + } + + hostaddr = virXMLPropString(node, "hostaddr"); + if (hostaddr) { + if (virSocketAddrParse(&def->hostaddr, hostaddr, AF_INET) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("Bad host address '%s' for redirection"), hostaddr); + goto error; + } + def->has_hostaddr = true; + } + + guestaddr = virXMLPropString(node, "guestaddr"); + if (guestaddr) { + if (virSocketAddrParse(&def->guestaddr, guestaddr, AF_INET) < 0) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("Bad guest address '%s' for redirection"), guestaddr); + goto error; + } + def->has_guestaddr = true; + } + +cleanup: + VIR_FREE(protocol); + VIR_FREE(hostaddr); + VIR_FREE(guestaddr); + VIR_FREE(hostport); + VIR_FREE(guestport); + + return def; + +error: + virDomainNetForwardDefFree(def); + def = NULL; + goto cleanup; +} + #define NET_MODEL_CHARS \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ091234567890_-" @@ -4423,6 +4518,8 @@ virDomainNetDefParseXML(virCapsPtr caps, virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt->node; int ret; + int nforwards; + xmlNodePtr *forwardNodes = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -4822,8 +4919,30 @@ virDomainNetDefParseXML(virCapsPtr caps, goto error; } + /* parse the <forward> elements */ + nforwards = virXPathNodeSet("./forward", ctxt, &forwardNodes); + if (nforwards < 0) + goto error; + + if (nforwards > 0) { + int i; + if (VIR_ALLOC_N(def->forwards, nforwards) < 0) { + virReportOOMError(); + goto error; + } + for (i = 0; i < nforwards; i++) { + virDomainNetForwardDefPtr fwd = + virDomainNetForwardDefParseXML(forwardNodes[i]); + if (fwd == NULL) + goto error; + def->forwards[def->nforward++] = fwd; + } + VIR_FREE(forwardNodes); + } + cleanup: ctxt->node = oldnode; + VIR_FREE(forwardNodes); VIR_FREE(macaddr); VIR_FREE(network); VIR_FREE(portgroup); @@ -11446,11 +11565,50 @@ error: } static int +virDomainNetForwardDefFormat(virBufferPtr buf, + virDomainNetForwardDefPtr def) +{ + const char *protocol; + char *ip; + + if (!def) + return 0; + + protocol = virDomainNetForwardProtocolTypeToString(def->protocol); + if (!protocol) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected net type %d"), def->protocol); + return -1; + } + virBufferAsprintf(buf, "<forward protocol='%s'", protocol); + + if (def->has_hostaddr) { + ip = virSocketAddrFormat(&def->hostaddr); + virBufferAsprintf(buf, " hostaddr='%s'", ip); + VIR_FREE(ip); + } + + virBufferAsprintf(buf, " hostport='%d'", def->hostport); + + if (def->has_guestaddr) { + ip = virSocketAddrFormat(&def->guestaddr); + virBufferAsprintf(buf, " guestaddr='%s'", ip); + VIR_FREE(ip); + } + + virBufferAsprintf(buf, " guestport='%d'", def->guestport); + + virBufferAddLit(buf, "/>\n"); + return 0; +} + +static int virDomainNetDefFormat(virBufferPtr buf, virDomainNetDefPtr def, unsigned int flags) { const char *type = virDomainNetTypeToString(def->type); + int i; if (!type) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -11617,6 +11775,13 @@ virDomainNetDefFormat(virBufferPtr buf, | VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM) < 0) return -1; + virBufferAdjustIndent(buf, 6); + for (i = 0; i < def->nforward; i++) { + if (virDomainNetForwardDefFormat(buf, def->forwards[i]) < 0) + return -1; + } + virBufferAdjustIndent(buf, -6); + virBufferAddLit(buf, " </interface>\n"); return 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4c56902..f40b51a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -772,6 +772,25 @@ struct _virDomainActualNetDef { virNetDevBandwidthPtr bandwidth; }; +enum virDomainNetForwardProtocol { + VIR_DOMAIN_NET_FORWARD_PROTOCOL_TCP, + VIR_DOMAIN_NET_FORWARD_PROTOCOL_UDP, + + VIR_DOMAIN_NET_FORWARD_PROTOCOL_LAST, +}; + +typedef struct _virDomainNetForwardDef virDomainNetForwardDef; +typedef virDomainNetForwardDef *virDomainNetForwardDefPtr; +struct _virDomainNetForwardDef { + int protocol; /* enum virDomainNetForwardProtocol */ + bool has_hostaddr; + virSocketAddr hostaddr; + int hostport; + bool has_guestaddr; + virSocketAddr guestaddr; + int guestport; +}; + /* Stores the virtual network interface configuration */ struct _virDomainNetDef { enum virDomainNetType type; @@ -837,6 +856,8 @@ struct _virDomainNetDef { virNWFilterHashTablePtr filterparams; virNetDevBandwidthPtr bandwidth; int linkstate; + int nforward; + virDomainNetForwardDefPtr *forwards; }; /* Used for prefix of ifname of any network name generated dynamically @@ -1860,6 +1881,7 @@ int virDomainDiskFindControllerModel(virDomainDefPtr def, void virDomainControllerDefFree(virDomainControllerDefPtr def); void virDomainFSDefFree(virDomainFSDefPtr def); void virDomainActualNetDefFree(virDomainActualNetDefPtr def); +void virDomainNetForwardDefFree(virDomainNetForwardDefPtr def); void virDomainNetDefFree(virDomainNetDefPtr def); void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def); void virDomainChrDefFree(virDomainChrDefPtr def); @@ -2174,6 +2196,7 @@ VIR_ENUM_DECL(virDomainFSAccessMode) VIR_ENUM_DECL(virDomainFSWrpolicy) VIR_ENUM_DECL(virDomainNet) VIR_ENUM_DECL(virDomainNetBackend) +VIR_ENUM_DECL(virDomainNetForwardProtocol) VIR_ENUM_DECL(virDomainNetVirtioTxMode) VIR_ENUM_DECL(virDomainNetInterfaceLinkState) VIR_ENUM_DECL(virDomainChrDevice) diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 439218e..cf2862b 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -207,7 +207,8 @@ mymain(void) DO_TEST("misc-acpi"); DO_TEST("misc-no-reboot"); DO_TEST("misc-uuid"); - DO_TEST("net-user"); + /* Fixed in following commit */ + /* DO_TEST("net-user"); */ DO_TEST("net-virtio"); DO_TEST("net-eth"); DO_TEST("net-eth-ifname"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml index 37e5edf..c018d34 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml @@ -23,6 +23,8 @@ <controller type='ide' index='0'/> <interface type='user'> <mac address='00:11:22:33:44:55'/> + <forward protocol='tcp' hostport='2222' guestport='22'/> + <forward protocol='udp' hostaddr='127.0.0.1' hostport='2242' guestaddr='10.0.2.15' guestport='42'/> </interface> <memballoon model='virtio'/> </devices> -- 1.7.10.1

Implement port forwarding for user mode networking with QEMU. --- src/libvirt_private.syms | 3 + src/qemu/qemu_command.c | 154 ++++++++++++++++++++- tests/qemuargv2xmltest.c | 3 +- tests/qemuxml2argvdata/qemuxml2argv-net-user.args | 5 +- 4 files changed, 158 insertions(+), 7 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7d09f33..d497bce 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -396,6 +396,9 @@ virDomainNetInsert; virDomainNetRemove; virDomainNetRemoveByMac; virDomainNetTypeToString; +virDomainNetForwardDefFree; +virDomainNetForwardProtocolTypeFromString; +virDomainNetForwardProtocolTypeToString; virDomainNostateReasonTypeFromString; virDomainNostateReasonTypeToString; virDomainNumatuneMemModeTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9f99dce..abd6f6a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2844,7 +2844,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("scripts are not supported on interfaces of type %s"), virDomainNetTypeToString(netType)); - return NULL; + goto error; } switch (netType) { @@ -2917,13 +2917,50 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, virBufferAsprintf(&buf, ",sndbuf=%lu", net->tune.sndbuf); } + if (net->nforward > 0) { + int i; + + if (netType != VIR_DOMAIN_NET_TYPE_USER) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("forward is not supported on interfaces of type %s"), + virDomainNetTypeToString(netType)); + goto error; + } + + for (i = 0; i < net->nforward; ++i) { + char *hostaddr = NULL; + char *guestaddr = NULL; + virDomainNetForwardDefPtr fwd = net->forwards[i]; + const char *protocol = virDomainNetForwardProtocolTypeToString(fwd->protocol); + if (!protocol) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Invalid forward protocol")); + goto error; + } + + if (fwd->has_hostaddr) + hostaddr = virSocketAddrFormat(&fwd->hostaddr); + if (fwd->has_guestaddr) + guestaddr = virSocketAddrFormat(&fwd->guestaddr); + + virBufferAsprintf(&buf, ",hostfwd=%s:%s:%d-%s:%d", protocol, + hostaddr ? hostaddr : "", fwd->hostport, + guestaddr ? guestaddr : "", fwd->guestport); + + VIR_FREE(hostaddr); + VIR_FREE(guestaddr); + } + } + if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); virReportOOMError(); - return NULL; } return virBufferContentAndReset(&buf); + +error: + virBufferFreeAndReset(&buf); + return NULL; } @@ -6785,6 +6822,94 @@ qemuFindNICForVLAN(int nnics, } +static virDomainNetForwardDefPtr +qemuParseNetForward(char *val) +{ + char *protocol, *host, *guest, *port; + virDomainNetForwardDefPtr def; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + return NULL; + } + + protocol = val; + host = strchr(protocol, ':'); + if (!host) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse hostfwd host '%s'"), val); + goto error; + } + *host = '\0'; + host++; + guest = strchr(host, '-'); + if (!guest) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse hostfwd guest '%s'"), val); + goto error; + } + *guest = '\0'; + guest++; + + if (STREQ(protocol, "")) { + def->protocol = VIR_DOMAIN_NET_FORWARD_PROTOCOL_TCP; + } else if ((def->protocol = virDomainNetForwardProtocolTypeFromString(protocol)) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown forward protocol '%s'"), protocol); + goto error; + } + + port = strchr(host, ':'); + if (!port) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing host port")); + goto error; + } + *port = '\0'; + port++; + if (!STREQ(host, "")) { + if (virSocketAddrParse(&def->hostaddr, host, AF_INET) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Bad host address '%s' for redirection"), host); + goto error; + } + def->has_hostaddr = true; + } + if (virStrToLong_i(port, NULL, 10, &def->hostport) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse host port")); + goto error; + } + + port = strchr(guest, ':'); + if (!port) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing guest port")); + goto error; + } + *port = '\0'; + port++; + if (!STREQ(guest, "")) { + if (virSocketAddrParse(&def->guestaddr, guest, AF_INET) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Bad guest address '%s' for redirection"), guest); + goto error; + } + def->has_guestaddr = true; + } + if (virStrToLong_i(port, NULL, 10, &def->guestport) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse guest port")); + goto error; + } + + return def; + +error: + virDomainNetForwardDefFree(def); + return NULL; +} + /* * Tries to parse a QEMU -net backend argument. Gets given * a list of all known -net frontend arguments to try and @@ -6804,6 +6929,7 @@ qemuParseCommandLineNet(virCapsPtr caps, int wantvlan = 0; const char *tmp; int genmac = 1; + int nforward = 0; int i; tmp = strchr(val, ','); @@ -6850,9 +6976,31 @@ qemuParseCommandLineNet(virCapsPtr caps, STREQ(keywords[i], "ifname")) { def->ifname = values[i]; values[i] = NULL; + } else if (def->type == VIR_DOMAIN_NET_TYPE_USER && + STREQ(keywords[i], "hostfwd")) { + nforward++; } } + if (nforward > 0) { + if (VIR_ALLOC_N(def->forwards, nforward) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (i = 0 ; i < nkeywords ; i++) { + virDomainNetForwardDefPtr fwd; + if (def->type != VIR_DOMAIN_NET_TYPE_USER || + !STREQ(keywords[i], "hostfwd")) + continue; + + if ((fwd = qemuParseNetForward(values[i])) == NULL) + goto cleanup; + + def->forwards[def->nforward] = fwd; + def->nforward++; + } + } /* Done parsing the nic backend. Now to try and find corresponding * frontend, based off vlan number. NB this assumes a 1-1 mapping diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index cf2862b..439218e 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -207,8 +207,7 @@ mymain(void) DO_TEST("misc-acpi"); DO_TEST("misc-no-reboot"); DO_TEST("misc-uuid"); - /* Fixed in following commit */ - /* DO_TEST("net-user"); */ + DO_TEST("net-user"); DO_TEST("net-virtio"); DO_TEST("net-eth"); DO_TEST("net-eth-ifname"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user.args b/tests/qemuxml2argvdata/qemuxml2argv-net-user.args index 093ff01..db31e95 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-net-user.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user.args @@ -1,5 +1,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net nic,\ -macaddr=00:11:22:33:44:55,vlan=0 -net user,vlan=0 -serial none -parallel none \ --usb +macaddr=00:11:22:33:44:55,vlan=0 \ +-net user,vlan=0,hostfwd=tcp::2222-:22,hostfwd=udp:127.0.0.1:2242-10.0.2.15:42 \ +-serial none -parallel none -usb -- 1.7.10.1

On Wed, May 16, 2012 at 6:34 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); virReportOOMError(); - return NULL;
forgot a "goto error;" here, added. -- Marc-André Lureau
participants (4)
-
Daniel P. Berrange
-
Hu Tao
-
Laine Stump
-
Marc-André Lureau