Re: [libvirt] [PATCH v2 2/2] qemu: Implement usernet address

[Laine Stump]

On 09/13/2017 06:47 AM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1075520
>
> Apart from generic checks, we need to constrain netmask/prefix
> lenght a bit. Thing is, with current implementation QEMU needs to

s/lenght/length/

> be able to 'assign' some IP addresses to the virtual network. For
> instance, the default gateway is at x.x.x.2, dns is at x.x.x.3,
> the default DHCP range is x.x.x.15-x.x.x.30. Since we don't
> expose these settings yet, it's safer to require shorter prefix
> to have room for the defaults.
>
> Signed-off-by: Michal Privoznik 

ACK/Reviewed-by/etc.

> ---
>  src/qemu/qemu_command.c| 22 +++
>  src/qemu/qemu_domain.c | 44 
> ++
>  .../qemuxml2argv-net-user-addr.args| 26 +
>  tests/qemuxml2argvtest.c   |  1 +
>  4 files changed, 93 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-addr.args
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index d553df57f..d7f7fa9b1 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3805,6 +3805,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
>  virDomainNetType netType = virDomainNetGetActualType(net);
>  virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>  size_t i;
> +char *addr = NULL;
>  char *ret = NULL;
>  
>  if (net->script && netType != VIR_DOMAIN_NET_TYPE_ETHERNET) {
> @@ -3873,6 +3874,26 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
>  break;
>  
>  case VIR_DOMAIN_NET_TYPE_USER:
> +virBufferAsprintf(&buf, "user%c", type_sep);
> +for (i = 0; i < net->guestIP.nips; i++) {
> +const virNetDevIPAddr *ip = net->guestIP.ips[i];
> +const char *prefix = "";
> +
> +if (!(addr = virSocketAddrFormat(&ip->address)))
> +goto cleanup;
> +
> +if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET))
> +prefix = "net=";
> +if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6))
> +prefix = "ipv6-net=";
> +
> +virBufferAsprintf(&buf, "%s%s", prefix, addr);
> +if (ip->prefix)
> +virBufferAsprintf(&buf, "/%u", ip->prefix);
> +virBufferAddChar(&buf, ',');
> +}
> +break;
> +
>  case VIR_DOMAIN_NET_TYPE_INTERNAL:
>  virBufferAsprintf(&buf, "user%c", type_sep);
>  break;
> @@ -3928,6 +3949,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
>   cleanup:
>  virBufferFreeAndReset(&buf);
>  virObjectUnref(cfg);
> +VIR_FREE(addr);
>  return ret;
>  }
>  
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index bccd30e12..849173fd1 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3338,9 +3338,11 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef 
> *dev,
>  void *opaque ATTRIBUTE_UNUSED)
>  {
>  int ret = -1;
> +size_t i;
>  
>  if (dev->type == VIR_DOMAIN_DEVICE_NET) {
>  const virDomainNetDef *net = dev->data.net;
> +bool hasIPv4 = false, hasIPv6 = false;
>  
>  if (net->type == VIR_DOMAIN_NET_TYPE_USER) {
>  if (net->guestIP.nroutes) {
> @@ -3349,6 +3351,48 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef 
> *dev,
>   "guest-side IP route, not supported by 
> QEMU"));
>  goto cleanup;
>  }
> +
> +for (i = 0; i < net->guestIP.nips; i++) {
> +const virNetDevIPAddr *ip = net->guestIP.ips[i];
> +
> +if (VIR_SOCKET_ADDR_VALID(&net->guestIP.ips[i]->peer)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("Invalid attempt to set peer IP for 
> guest"));
> +goto cleanup;
> +}
> +
> +if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET)) {
> +if (hasIPv4) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("Only one IPv4 address per "
> + "interface is allowed"));
> +goto cleanup;
> +}
> +hasIPv4 = true;
> +
> +if (ip->prefix > 27) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> +   _("prefix too long"));
> +goto cleanup;
> +}
> +}
> +
> +if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6)) {
> +if (hasIPv6) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("Only one IPv6 address per "
> +

[libvirt] [PATCH v2 2/2] qemu: Implement usernet address

[Michal Privoznik]

https://bugzilla.redhat.com/show_bug.cgi?id=1075520

Apart from generic checks, we need to constrain netmask/prefix
lenght a bit. Thing is, with current implementation QEMU needs to
be able to 'assign' some IP addresses to the virtual network. For
instance, the default gateway is at x.x.x.2, dns is at x.x.x.3,
the default DHCP range is x.x.x.15-x.x.x.30. Since we don't
expose these settings yet, it's safer to require shorter prefix
to have room for the defaults.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_command.c| 22 +++
 src/qemu/qemu_domain.c | 44 ++
 .../qemuxml2argv-net-user-addr.args| 26 +
 tests/qemuxml2argvtest.c   |  1 +
 4 files changed, 93 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-addr.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d553df57f..d7f7fa9b1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3805,6 +3805,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
 virDomainNetType netType = virDomainNetGetActualType(net);
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 size_t i;
+char *addr = NULL;
 char *ret = NULL;
 
 if (net->script && netType != VIR_DOMAIN_NET_TYPE_ETHERNET) {
@@ -3873,6 +3874,26 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
 break;
 
 case VIR_DOMAIN_NET_TYPE_USER:
+virBufferAsprintf(&buf, "user%c", type_sep);
+for (i = 0; i < net->guestIP.nips; i++) {
+const virNetDevIPAddr *ip = net->guestIP.ips[i];
+const char *prefix = "";
+
+if (!(addr = virSocketAddrFormat(&ip->address)))
+goto cleanup;
+
+if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET))
+prefix = "net=";
+if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6))
+prefix = "ipv6-net=";
+
+virBufferAsprintf(&buf, "%s%s", prefix, addr);
+if (ip->prefix)
+virBufferAsprintf(&buf, "/%u", ip->prefix);
+virBufferAddChar(&buf, ',');
+}
+break;
+
 case VIR_DOMAIN_NET_TYPE_INTERNAL:
 virBufferAsprintf(&buf, "user%c", type_sep);
 break;
@@ -3928,6 +3949,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
  cleanup:
 virBufferFreeAndReset(&buf);
 virObjectUnref(cfg);
+VIR_FREE(addr);
 return ret;
 }
 
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bccd30e12..849173fd1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3338,9 +3338,11 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef 
*dev,
 void *opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
+size_t i;
 
 if (dev->type == VIR_DOMAIN_DEVICE_NET) {
 const virDomainNetDef *net = dev->data.net;
+bool hasIPv4 = false, hasIPv6 = false;
 
 if (net->type == VIR_DOMAIN_NET_TYPE_USER) {
 if (net->guestIP.nroutes) {
@@ -3349,6 +3351,48 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef 
*dev,
  "guest-side IP route, not supported by 
QEMU"));
 goto cleanup;
 }
+
+for (i = 0; i < net->guestIP.nips; i++) {
+const virNetDevIPAddr *ip = net->guestIP.ips[i];
+
+if (VIR_SOCKET_ADDR_VALID(&net->guestIP.ips[i]->peer)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Invalid attempt to set peer IP for 
guest"));
+goto cleanup;
+}
+
+if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET)) {
+if (hasIPv4) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Only one IPv4 address per "
+ "interface is allowed"));
+goto cleanup;
+}
+hasIPv4 = true;
+
+if (ip->prefix > 27) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("prefix too long"));
+goto cleanup;
+}
+}
+
+if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6)) {
+if (hasIPv6) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Only one IPv6 address per "
+ "interface is allowed"));
+goto cleanup;
+}
+hasIPv6 = true;
+
+if (ip->prefix > 120) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("prefix too long"));