[libvirt] [PATCH] Expose SLIRP attributes

We allow users to use SLIRP stack. However, there are some knobs which are not exposed to users, such as host network address, DNS server, smb, and others. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.html.in | 7 +- docs/schemas/domaincommon.rng | 23 +++++- src/conf/domain_conf.c | 88 ++++++++++++++++++++++ src/conf/domain_conf.h | 6 ++ src/qemu/qemu_command.c | 19 +++++ .../qemuxml2argvdata/qemuxml2argv-net-user-ip.args | 7 ++ .../qemuxml2argvdata/qemuxml2argv-net-user-ip.xml | 33 ++++++++ tests/qemuxml2argvtest.c | 1 + 8 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7f90455..0a353ca 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3225,7 +3225,11 @@ starting from <code>10.0.2.15</code>. The default router will be <code>10.0.2.2</code> and the DNS server will be <code>10.0.2.3</code>. This networking is the only option for unprivileged users who need their - VMs to have outgoing access. + VMs to have outgoing access. <span class="since">Since 1.2.3</span> the + user network can have the <ip/> element to override the default + network of <code>10.0.2.0/24</code>. For example it can be set to + <code>192.168.2.0/24</code>. The whole element and its attributes are + optional. </p> <pre> @@ -3235,6 +3239,7 @@ ... <interface type='user'> <mac address="00:11:22:33:44:55"/> + <ip address="192.168.2.0" prefix="24" dns="192.168.2.3" dhcpstart="192.168.2.9"/> </interface> </devices> ...</pre> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index bcd8142..5745ce7 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2152,9 +2152,26 @@ </optional> <optional> <element name="ip"> - <attribute name="address"> - <ref name="ipv4Addr"/> - </attribute> + <optional> + <attribute name="address"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <optional> + <attribute name="prefix"> + <data type="integer"/> + </attribute> + </optional> + <optional> + <attribute name="dns"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <optional> + <attribute name="dhcpstart"> + <ref name="ipv4Addr"/> + </attribute> + </optional> <empty/> </element> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6fb216e..aec14ed 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1539,6 +1539,11 @@ void virDomainNetDefFree(virDomainNetDefPtr def) break; case VIR_DOMAIN_NET_TYPE_USER: + VIR_FREE(def->data.user.ipaddr); + VIR_FREE(def->data.user.dns); + VIR_FREE(def->data.user.dhcpstart); + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -6669,6 +6674,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, char *mode = NULL; char *linkstate = NULL; char *addrtype = NULL; + char *prefix = NULL; + char *dns = NULL; + char *dhcpstart = NULL; virNWFilterHashTablePtr filterparams = NULL; virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt->node; @@ -6750,6 +6758,13 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, def->type == VIR_DOMAIN_NET_TYPE_BRIDGE) && xmlStrEqual(cur->name, BAD_CAST "ip")) { address = virXMLPropString(cur, "address"); + } else if (!address && + def->type == VIR_DOMAIN_NET_TYPE_USER && + xmlStrEqual(cur->name, BAD_CAST "ip")) { + address = virXMLPropString(cur, "address"); + prefix = virXMLPropString(cur, "prefix"); + dns = virXMLPropString(cur, "dns"); + dhcpstart = virXMLPropString(cur, "dhcpstart"); } else if (!ifname && xmlStrEqual(cur->name, BAD_CAST "target")) { ifname = virXMLPropString(cur, "dev"); @@ -6988,6 +7003,59 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, break; case VIR_DOMAIN_NET_TYPE_USER: + if (prefix) { + if (!address) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("can't use prefix without an address")); + goto error; + } + + if (virStrToLong_i(prefix, NULL, 10, &def->data.user.prefix) < 0 || + def->data.user.prefix < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid prefix: '%s'"), prefix); + goto error; + } + } else { + def->data.user.prefix = -1; + } + + if (address) { + if (virSocketAddrParse(NULL, address, AF_UNSPEC) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid address: '%s'"), address); + goto error; + } + + def->data.user.ipaddr = address; + address = NULL; + } + + if (dns) { + if (virSocketAddrParse(NULL, dns, AF_UNSPEC) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid dns address: '%s'"), dns); + goto error; + } + + def->data.user.dns = dns; + dns = NULL; + } + + if (dhcpstart) { + if (virSocketAddrParse(NULL, dhcpstart, AF_UNSPEC) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid dhcpstart address: '%s'"), + dhcpstart); + goto error; + } + + def->data.user.dhcpstart = dhcpstart; + dhcpstart = NULL; + } + + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -7134,6 +7202,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(mode); VIR_FREE(linkstate); VIR_FREE(addrtype); + VIR_FREE(prefix); + VIR_FREE(dns); + VIR_FREE(dhcpstart); virNWFilterHashTableFree(filterparams); return def; @@ -15766,6 +15837,23 @@ virDomainNetDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_NET_TYPE_USER: + if (def->data.user.ipaddr || + def->data.user.dns || + def->data.user.dhcpstart) { + virBufferAddLit(buf, "<ip"); + + if (def->data.user.ipaddr) { + virBufferAsprintf(buf, " address='%s'", def->data.user.ipaddr); + if (def->data.user.prefix >= 0) + virBufferAsprintf(buf, " prefix='%d'", def->data.user.prefix); + } + + virBufferEscapeString(buf, " dns='%s'", def->data.user.dns); + virBufferEscapeString(buf, " dhcpstart='%s'", def->data.user.dhcpstart); + virBufferAddLit(buf, "/>\n"); + } + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f3f24c4..b5d52e9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1046,6 +1046,12 @@ struct _virDomainNetDef { struct { virDomainHostdevDef def; } hostdev; + struct { + char *ipaddr; + int prefix; + char *dns; + char *dhcpstart; + } user; } data; /* virtPortProfile is used by network/bridge/direct/hostdev */ virNetDevVPortProfilePtr virtPortProfile; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9a314bf..f17061e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5156,6 +5156,25 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, break; case VIR_DOMAIN_NET_TYPE_USER: + virBufferAddLit(&buf, "user"); + if (net->data.user.ipaddr) { + virBufferAsprintf(&buf, "%cnet=%s", type_sep, + net->data.user.ipaddr); + type_sep = ','; + if (net->data.user.prefix >= 0) + virBufferAsprintf(&buf, "/%d", net->data.user.prefix); + } + if (net->data.user.dns) { + virBufferAsprintf(&buf, "%cdns=%s", type_sep, net->data.user.dns); + type_sep = ','; + } + if (net->data.user.dhcpstart) { + virBufferAsprintf(&buf, "%cdhcpstart=%s", type_sep, + net->data.user.dhcpstart); + type_sep = ','; + } + break; + default: virBufferAddLit(&buf, "user"); break; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.args b/tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.args new file mode 100644 index 0000000..3e355f3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.args @@ -0,0 +1,7 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic \ +-monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 \ +-net nic,macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139 \ +-net user,net=192.168.2.0/24,dns=192.168.2.3,dhcpstart=192.168.2.9,vlan=0 \ +-serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.xml new file mode 100644 index 0000000..5c36c77 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <interface type='user'> + <ip address='192.168.2.0' prefix='24' dns='192.168.2.3' dhcpstart='192.168.2.9'/> + <mac address='00:11:22:33:44:55'/> + <model type='rtl8139'/> + </interface> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 56854dc..02ed4ba 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -923,6 +923,7 @@ mymain(void) DO_TEST("misc-no-reboot", NONE); DO_TEST("misc-uuid", QEMU_CAPS_NAME, QEMU_CAPS_UUID); DO_TEST("net-user", NONE); + DO_TEST("net-user-ip", NONE); DO_TEST("net-virtio", NONE); DO_TEST("net-virtio-device", QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_TX_ALG); -- 1.9.0

On 03/27/2014 11:17 AM, Michal Privoznik wrote:
We allow users to use SLIRP stack. However, there are some knobs which are not exposed to users, such as host network address, DNS server, smb, and others.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.html.in | 7 +- docs/schemas/domaincommon.rng | 23 +++++- src/conf/domain_conf.c | 88 ++++++++++++++++++++++ src/conf/domain_conf.h | 6 ++ src/qemu/qemu_command.c | 19 +++++ .../qemuxml2argvdata/qemuxml2argv-net-user-ip.args | 7 ++ .../qemuxml2argvdata/qemuxml2argv-net-user-ip.xml | 33 ++++++++ tests/qemuxml2argvtest.c | 1 + 8 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7f90455..0a353ca 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3225,7 +3225,11 @@ starting from <code>10.0.2.15</code>. The default router will be <code>10.0.2.2</code> and the DNS server will be <code>10.0.2.3</code>. This networking is the only option for unprivileged users who need their - VMs to have outgoing access. + VMs to have outgoing access. <span class="since">Since 1.2.3</span> the
We've already frozen for 1.2.3, and this feels like new features rather than bug fix. Is it better to wait until after the release?
+ user network can have the <ip/> element to override the default + network of <code>10.0.2.0/24</code>. For example it can be set to + <code>192.168.2.0/24</code>. The whole element and its attributes are
No mention of what those attributes are? Other than...
+ optional. </p>
<pre> @@ -3235,6 +3239,7 @@ ... <interface type='user'> <mac address="00:11:22:33:44:55"/> + <ip address="192.168.2.0" prefix="24" dns="192.168.2.3" dhcpstart="192.168.2.9"/>
...here in the example
+++ b/docs/schemas/domaincommon.rng @@ -2152,9 +2152,26 @@ </optional> <optional> <element name="ip">
Needs an <interleave> around the optional sub-elements. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Thu, Mar 27, 2014 at 11:55:45AM -0600, Eric Blake wrote:
On 03/27/2014 11:17 AM, Michal Privoznik wrote:
We allow users to use SLIRP stack. However, there are some knobs which are not exposed to users, such as host network address, DNS server, smb, and others.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.html.in | 7 +- docs/schemas/domaincommon.rng | 23 +++++- src/conf/domain_conf.c | 88 ++++++++++++++++++++++ src/conf/domain_conf.h | 6 ++ src/qemu/qemu_command.c | 19 +++++ .../qemuxml2argvdata/qemuxml2argv-net-user-ip.args | 7 ++ .../qemuxml2argvdata/qemuxml2argv-net-user-ip.xml | 33 ++++++++ tests/qemuxml2argvtest.c | 1 + 8 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7f90455..0a353ca 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3225,7 +3225,11 @@ starting from <code>10.0.2.15</code>. The default router will be <code>10.0.2.2</code> and the DNS server will be <code>10.0.2.3</code>. This networking is the only option for unprivileged users who need their - VMs to have outgoing access. + VMs to have outgoing access. <span class="since">Since 1.2.3</span> the
We've already frozen for 1.2.3, and this feels like new features rather than bug fix. Is it better to wait until after the release?
Given the ongoing comments, I think 1.2.4 is most suitable at this time. Regards, 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 Thu, Mar 27, 2014 at 06:17:46PM +0100, Michal Privoznik wrote:
We allow users to use SLIRP stack. However, there are some knobs which are not exposed to users, such as host network address, DNS server, smb, and others.
The XML looks good. I have posted a patch to libguestfs to consume this XML: https://www.redhat.com/archives/libguestfs/2014-March/thread.html#00254 I wasn't able to try it because of an unrelated segfault in libvirt.git at the moment, but will try it out later when that is fixed. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW

On Thu, Mar 27, 2014 at 06:48:24PM +0000, Richard W.M. Jones wrote:
On Thu, Mar 27, 2014 at 06:17:46PM +0100, Michal Privoznik wrote:
We allow users to use SLIRP stack. However, there are some knobs which are not exposed to users, such as host network address, DNS server, smb, and others.
The XML looks good. I have posted a patch to libguestfs to consume this XML:
https://www.redhat.com/archives/libguestfs/2014-March/thread.html#00254
I wasn't able to try it because of an unrelated segfault in libvirt.git at the moment, but will try it out later when that is fixed.
I am now able to test this patch, and it works for me. Not ACKing because AIUI Laine wants you to make some further changes. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/

On 03/27/2014 07:17 PM, Michal Privoznik wrote:
We allow users to use SLIRP stack. However, there are some knobs which are not exposed to users, such as host network address, DNS server, smb, and others.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.html.in | 7 +- docs/schemas/domaincommon.rng | 23 +++++- src/conf/domain_conf.c | 88 ++++++++++++++++++++++ src/conf/domain_conf.h | 6 ++ src/qemu/qemu_command.c | 19 +++++ .../qemuxml2argvdata/qemuxml2argv-net-user-ip.args | 7 ++ .../qemuxml2argvdata/qemuxml2argv-net-user-ip.xml | 33 ++++++++ tests/qemuxml2argvtest.c | 1 + 8 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.xml
It is essential that this new <ip> element be rejected, preferably at parse time in the qemu-specific post-parse callback, for any interface type that doesn't support it. Since it is the most obvious way of specifying an IP address for a guest (and it is a way that has been requested in the past) we are otherwise certain to have a lot of support questions asking why the IP address setting isn't being used. Also, the attribute names seem confusing. The "address" attribute is the address of the *network*, not of the interface, and "dhcpstart" is the address of the interface. Even though qemu specifies the network address and interface address separately, the network address is really pointless, as it can/should be derived from the interface address. (I realize that "dhcpstart" isn't *exactly* the interface IP, since qemu allows for multiple IP leases to be acquired from it's internal dhcp server, but I think that usage is very rare, and unlikely to be possible for any other backend we might support). Beyond that, a question not with your patch, but with qemu's implemenation - does it always assume that the gateway address is $network.1 ? If that's the case, then I think definitely we should just have <ip address='x' prefix='y'/>. If there is support for specifying the gateway address, then it should be named "gateway", as is already done in the network xml for static routes.
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7f90455..0a353ca 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3225,7 +3225,11 @@ starting from <code>10.0.2.15</code>. The default router will be <code>10.0.2.2</code> and the DNS server will be <code>10.0.2.3</code>. This networking is the only option for unprivileged users who need their - VMs to have outgoing access. + VMs to have outgoing access. <span class="since">Since 1.2.3</span> the + user network can have the <ip/> element to override the default + network of <code>10.0.2.0/24</code>. For example it can be set to + <code>192.168.2.0/24</code>. The whole element and its attributes are + optional. </p>
<pre> @@ -3235,6 +3239,7 @@ ... <interface type='user'> <mac address="00:11:22:33:44:55"/> + <ip address="192.168.2.0" prefix="24" dns="192.168.2.3" dhcpstart="192.168.2.9"/> </interface> </devices> ...</pre> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index bcd8142..5745ce7 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2152,9 +2152,26 @@ </optional> <optional> <element name="ip"> - <attribute name="address"> - <ref name="ipv4Addr"/> - </attribute> + <optional> + <attribute name="address"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <optional> + <attribute name="prefix"> + <data type="integer"/> + </attribute> + </optional> + <optional> + <attribute name="dns"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <optional> + <attribute name="dhcpstart"> + <ref name="ipv4Addr"/> + </attribute> + </optional> <empty/> </element> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6fb216e..aec14ed 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1539,6 +1539,11 @@ void virDomainNetDefFree(virDomainNetDefPtr def) break;
case VIR_DOMAIN_NET_TYPE_USER: + VIR_FREE(def->data.user.ipaddr); + VIR_FREE(def->data.user.dns); + VIR_FREE(def->data.user.dhcpstart); + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -6669,6 +6674,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, char *mode = NULL; char *linkstate = NULL; char *addrtype = NULL; + char *prefix = NULL; + char *dns = NULL; + char *dhcpstart = NULL; virNWFilterHashTablePtr filterparams = NULL; virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt->node; @@ -6750,6 +6758,13 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, def->type == VIR_DOMAIN_NET_TYPE_BRIDGE) && xmlStrEqual(cur->name, BAD_CAST "ip")) { address = virXMLPropString(cur, "address"); + } else if (!address && + def->type == VIR_DOMAIN_NET_TYPE_USER && + xmlStrEqual(cur->name, BAD_CAST "ip")) { + address = virXMLPropString(cur, "address"); + prefix = virXMLPropString(cur, "prefix"); + dns = virXMLPropString(cur, "dns"); + dhcpstart = virXMLPropString(cur, "dhcpstart"); } else if (!ifname && xmlStrEqual(cur->name, BAD_CAST "target")) { ifname = virXMLPropString(cur, "dev"); @@ -6988,6 +7003,59 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, break;
case VIR_DOMAIN_NET_TYPE_USER: + if (prefix) { + if (!address) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("can't use prefix without an address")); + goto error; + } + + if (virStrToLong_i(prefix, NULL, 10, &def->data.user.prefix) < 0 || + def->data.user.prefix < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid prefix: '%s'"), prefix); + goto error; + } + } else { + def->data.user.prefix = -1; + } + + if (address) { + if (virSocketAddrParse(NULL, address, AF_UNSPEC) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid address: '%s'"), address); + goto error; + } + + def->data.user.ipaddr = address; + address = NULL; + } + + if (dns) { + if (virSocketAddrParse(NULL, dns, AF_UNSPEC) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid dns address: '%s'"), dns); + goto error; + } + + def->data.user.dns = dns; + dns = NULL; + } + + if (dhcpstart) { + if (virSocketAddrParse(NULL, dhcpstart, AF_UNSPEC) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid dhcpstart address: '%s'"), + dhcpstart); + goto error; + } + + def->data.user.dhcpstart = dhcpstart; + dhcpstart = NULL; + } + + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } @@ -7134,6 +7202,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(mode); VIR_FREE(linkstate); VIR_FREE(addrtype); + VIR_FREE(prefix); + VIR_FREE(dns); + VIR_FREE(dhcpstart); virNWFilterHashTableFree(filterparams);
return def; @@ -15766,6 +15837,23 @@ virDomainNetDefFormat(virBufferPtr buf, break;
case VIR_DOMAIN_NET_TYPE_USER: + if (def->data.user.ipaddr || + def->data.user.dns || + def->data.user.dhcpstart) { + virBufferAddLit(buf, "<ip"); + + if (def->data.user.ipaddr) { + virBufferAsprintf(buf, " address='%s'", def->data.user.ipaddr); + if (def->data.user.prefix >= 0) + virBufferAsprintf(buf, " prefix='%d'", def->data.user.prefix); + } + + virBufferEscapeString(buf, " dns='%s'", def->data.user.dns); + virBufferEscapeString(buf, " dhcpstart='%s'", def->data.user.dhcpstart); + virBufferAddLit(buf, "/>\n"); + } + break; + case VIR_DOMAIN_NET_TYPE_LAST: break; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f3f24c4..b5d52e9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1046,6 +1046,12 @@ struct _virDomainNetDef { struct { virDomainHostdevDef def; } hostdev; + struct { + char *ipaddr; + int prefix; + char *dns; + char *dhcpstart; + } user; } data; /* virtPortProfile is used by network/bridge/direct/hostdev */ virNetDevVPortProfilePtr virtPortProfile; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9a314bf..f17061e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5156,6 +5156,25 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, break;
case VIR_DOMAIN_NET_TYPE_USER: + virBufferAddLit(&buf, "user"); + if (net->data.user.ipaddr) { + virBufferAsprintf(&buf, "%cnet=%s", type_sep, + net->data.user.ipaddr); + type_sep = ','; + if (net->data.user.prefix >= 0) + virBufferAsprintf(&buf, "/%d", net->data.user.prefix); + } + if (net->data.user.dns) { + virBufferAsprintf(&buf, "%cdns=%s", type_sep, net->data.user.dns); + type_sep = ','; + } + if (net->data.user.dhcpstart) { + virBufferAsprintf(&buf, "%cdhcpstart=%s", type_sep, + net->data.user.dhcpstart); + type_sep = ','; + } + break; + default: virBufferAddLit(&buf, "user"); break; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.args b/tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.args new file mode 100644 index 0000000..3e355f3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.args @@ -0,0 +1,7 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic \ +-monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 \ +-net nic,macaddr=00:11:22:33:44:55,vlan=0,model=rtl8139 \ +-net user,net=192.168.2.0/24,dns=192.168.2.3,dhcpstart=192.168.2.9,vlan=0 \ +-serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.xml new file mode 100644 index 0000000..5c36c77 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <interface type='user'> + <ip address='192.168.2.0' prefix='24' dns='192.168.2.3' dhcpstart='192.168.2.9'/> + <mac address='00:11:22:33:44:55'/> + <model type='rtl8139'/> + </interface> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 56854dc..02ed4ba 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -923,6 +923,7 @@ mymain(void) DO_TEST("misc-no-reboot", NONE); DO_TEST("misc-uuid", QEMU_CAPS_NAME, QEMU_CAPS_UUID); DO_TEST("net-user", NONE); + DO_TEST("net-user-ip", NONE); DO_TEST("net-virtio", NONE); DO_TEST("net-virtio-device", QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_TX_ALG);

On Fri, Mar 28, 2014 at 10:33:39AM +0200, Laine Stump wrote:
Beyond that, a question not with your patch, but with qemu's implemenation - does it always assume that the gateway address is $network.1 ?
Actually network.2. The default addresses are: network: 10.0.2.0/24 (ie. mask 255.255.255.0) default gateway: 10.0.2.2 dns server: 10.0.2.3 dhcp start / normal guest address: 10.0.2.15 It _is_ possible to change the gateway address, by specifying the (confusingly named) 'host=' parameter. As you suggested I think this could be mapped to a gateway XML attribute, although libguestfs would not need to use it. Rich. ---------------------------------------------------------------------- Full parameters: -netdev user,id=id[,option][,option][,...] -net user[,option][,option][,...] Use the user mode network stack which requires no administrator privilege to run. Valid options are: vlan=n Connect user mode stack to VLAN n (n = 0 is the default). id=id name=name Assign symbolic name for use in monitor commands. net=addr[/mask] Set IP network address the guest will see. Optionally specify the netmask, either in the form a.b.c.d or as number of valid top-most bits. Default is 10.0.2.0/24. host=addr Specify the guest-visible address of the host. Default is the 2nd IP in the guest network, i.e. x.x.x.2. restrict=on|off If this option is enabled, the guest will be isolated, i.e. it will not be able to contact the host and no guest IP packets will be routed over the host to the outside. This option does not affect any explicitly set forwarding rules. hostname=name Specifies the client hostname reported by the built-in DHCP server. dhcpstart=addr Specify the first of the 16 IPs the built-in DHCP server can assign. Default is the 15th to 31st IP in the guest network, i.e. x.x.x.15 to x.x.x.31. dns=addr Specify the guest-visible address of the virtual nameserver. The address must be different from the host address. Default is the 3rd IP in the guest network, i.e. x.x.x.3. dnssearch=domain Provides an entry for the domain-search list sent by the built- in DHCP server. More than one domain suffix can be transmitted by specifying this option multiple times. If supported, this will cause the guest to automatically try to append the given domain suffix(es) in case a domain name can not be resolved. Example: qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...] tftp=dir When using the user mode network stack, activate a built-in TFTP server. The files in dir will be exposed as the root of a TFTP server. The TFTP client on the guest must be configured in binary mode (use the command "bin" of the Unix TFTP client). bootfile=file When using the user mode network stack, broadcast file as the BOOTP filename. In conjunction with tftp, this can be used to network boot a guest from a local directory. Example (using pxelinux): qemu-system-i386 -hda linux.img -boot n -net user,tftp=/path/to/tftp/files,bootfile=/pxelinux.0 smb=dir[,smbserver=addr] When using the user mode network stack, activate a built-in SMB server so that Windows OSes can access to the host files in dir transparently. The IP address of the SMB server can be set to addr. By default the 4th IP in the guest network is used, i.e. x.x.x.4. In the guest Windows OS, the line: 10.0.2.4 smbserver must be added in the file C:\WINDOWS\LMHOSTS (for windows 9x/Me) or C:\WINNT\SYSTEM32\DRIVERS\ETC\LMHOSTS (Windows NT/2000). Then dir can be accessed in \\smbserver\qemu. Note that a SAMBA server must be installed on the host OS. QEMU was tested successfully with smbd versions from Red Hat 9, Fedora Core 3 and OpenSUSE 11.x. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top

On 03/28/2014 10:47 AM, Richard W.M. Jones wrote:
On Fri, Mar 28, 2014 at 10:33:39AM +0200, Laine Stump wrote:
Beyond that, a question not with your patch, but with qemu's implemenation - does it always assume that the gateway address is $network.1 ? Actually network.2. The default addresses are:
network: 10.0.2.0/24 (ie. mask 255.255.255.0) default gateway: 10.0.2.2 dns server: 10.0.2.3 dhcp start / normal guest address: 10.0.2.15
It _is_ possible to change the gateway address, by specifying the (confusingly named) 'host=' parameter. As you suggested I think this could be mapped to a gateway XML attribute, although libguestfs would not need to use it.
Ah, good. I didn't see that in the parameters that I found (why is my first reaction now to do a google search instead of looking at the man page? What is wrong with me?? :-P). In that case, I think that the XML should be <ip address='guest's IP address' prefix='prefix for network' gateway='host IP address according to guest' dns='DNS server IP given in DHCP response (and answering requests)'/> I think that * address should be mandatory * prefix should default to the natural prefix for that particular address (we have a function somewhere in the network conf or driver code that already does this) * gateway should default to address & ~(0xffffffff<<(32-prefix)) + 1 * dns should default to gateway (I know this doesn't match qemu's default, but we shouldn't be designing our XML based on qemu's default, nor should we be assuming that their default will remain unchanged in the future - we've been burned by that before. Rather, we should make the defaults as similar to what we do in other parts of libvirt as possible). As for the setting of qemu's options: * qemu's "network" option should be filled in with $(gateway & ~(0xffffffff<<(32-prefix)) / $prefix * qemu's "host" should be filled in with $gateway * qemu's "dns" should be filled in with $dns * qemu's "dhcpstart" should be filled in with $address (do we also want to add a "domain" attribute, which would default to empty, and be put into qemu's "dnssearch" option?) When I saw the list of options, I briefly considered that we should support restricted, but on reading the details it turns out that option *completely* shuts down network traffic, even to the host, and you need to add forwarding rules to individually enable what you want. Although that might be useful in some cases, it is very different from libvirt networks' "isolated" vs. forward modes, meaning I don't have a clear idea of a way to make it fit nicely with existing paradigms, so I won't pursue it for now (or later, unless someone specifically requests it :-)
Rich.
----------------------------------------------------------------------
Full parameters:
-netdev user,id=id[,option][,option][,...] -net user[,option][,option][,...] Use the user mode network stack which requires no administrator privilege to run. Valid options are:
vlan=n Connect user mode stack to VLAN n (n = 0 is the default).
id=id name=name Assign symbolic name for use in monitor commands.
net=addr[/mask] Set IP network address the guest will see. Optionally specify the netmask, either in the form a.b.c.d or as number of valid top-most bits. Default is 10.0.2.0/24.
host=addr Specify the guest-visible address of the host. Default is the 2nd IP in the guest network, i.e. x.x.x.2.
restrict=on|off If this option is enabled, the guest will be isolated, i.e. it will not be able to contact the host and no guest IP packets will be routed over the host to the outside. This option does not affect any explicitly set forwarding rules.
hostname=name Specifies the client hostname reported by the built-in DHCP server.
dhcpstart=addr Specify the first of the 16 IPs the built-in DHCP server can assign. Default is the 15th to 31st IP in the guest network, i.e. x.x.x.15 to x.x.x.31.
dns=addr Specify the guest-visible address of the virtual nameserver. The address must be different from the host address. Default is the 3rd IP in the guest network, i.e. x.x.x.3.
dnssearch=domain Provides an entry for the domain-search list sent by the built- in DHCP server. More than one domain suffix can be transmitted by specifying this option multiple times. If supported, this will cause the guest to automatically try to append the given domain suffix(es) in case a domain name can not be resolved.
Example:
qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...]
tftp=dir When using the user mode network stack, activate a built-in TFTP server. The files in dir will be exposed as the root of a TFTP server. The TFTP client on the guest must be configured in binary mode (use the command "bin" of the Unix TFTP client).
bootfile=file When using the user mode network stack, broadcast file as the BOOTP filename. In conjunction with tftp, this can be used to network boot a guest from a local directory.
Example (using pxelinux): qemu-system-i386 -hda linux.img -boot n -net user,tftp=/path/to/tftp/files,bootfile=/pxelinux.0
smb=dir[,smbserver=addr] When using the user mode network stack, activate a built-in SMB server so that Windows OSes can access to the host files in dir transparently. The IP address of the SMB server can be set to addr. By default the 4th IP in the guest network is used, i.e. x.x.x.4.
In the guest Windows OS, the line:
10.0.2.4 smbserver
must be added in the file C:\WINDOWS\LMHOSTS (for windows 9x/Me) or C:\WINNT\SYSTEM32\DRIVERS\ETC\LMHOSTS (Windows NT/2000).
Then dir can be accessed in \\smbserver\qemu.
Note that a SAMBA server must be installed on the host OS. QEMU was tested successfully with smbd versions from Red Hat 9, Fedora Core 3 and OpenSUSE 11.x.

On Fri, Mar 28, 2014 at 12:16:43PM +0200, Laine Stump wrote:
On 03/28/2014 10:47 AM, Richard W.M. Jones wrote:
On Fri, Mar 28, 2014 at 10:33:39AM +0200, Laine Stump wrote:
Beyond that, a question not with your patch, but with qemu's implemenation - does it always assume that the gateway address is $network.1 ? Actually network.2. The default addresses are:
network: 10.0.2.0/24 (ie. mask 255.255.255.0) default gateway: 10.0.2.2 dns server: 10.0.2.3 dhcp start / normal guest address: 10.0.2.15
It _is_ possible to change the gateway address, by specifying the (confusingly named) 'host=' parameter. As you suggested I think this could be mapped to a gateway XML attribute, although libguestfs would not need to use it.
Ah, good. I didn't see that in the parameters that I found (why is my first reaction now to do a google search instead of looking at the man page? What is wrong with me?? :-P).
In that case, I think that the XML should be
<ip address='guest's IP address' prefix='prefix for network' gateway='host IP address according to guest' dns='DNS server IP given in DHCP response (and answering requests)'/>
I think that
* address should be mandatory
I don't understand why this should be mandatory. Actually I don't think any of them should be mandatory (the same as qemu), but the only one which libguestfs would specify is the network. The guest can choose any IP address on the network >= .5 and things will work. The libguestfs appliance arbitrarily uses .10 since DHCP would be too slow.
* prefix should default to the natural prefix for that particular address (we have a function somewhere in the network conf or driver code that already does this)
Right, qemu does this if you don't specify it on the command line.
* gateway should default to
address & ~(0xffffffff<<(32-prefix)) + 1
qemu will choose .2 unless specified. [...] For libguestfs we only care about setting the network address + prefix, and to fix this bug I'm quite happy for the patch to only do that, and we can worry about the other things another time. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top

On 03/28/2014 01:03 PM, Richard W.M. Jones wrote:
On Fri, Mar 28, 2014 at 12:16:43PM +0200, Laine Stump wrote:
On Fri, Mar 28, 2014 at 10:33:39AM +0200, Laine Stump wrote:
Beyond that, a question not with your patch, but with qemu's implemenation - does it always assume that the gateway address is $network.1 ? Actually network.2. The default addresses are:
network: 10.0.2.0/24 (ie. mask 255.255.255.0) default gateway: 10.0.2.2 dns server: 10.0.2.3 dhcp start / normal guest address: 10.0.2.15
It _is_ possible to change the gateway address, by specifying the (confusingly named) 'host=' parameter. As you suggested I think this could be mapped to a gateway XML attribute, although libguestfs would not need to use it. Ah, good. I didn't see that in the parameters that I found (why is my first reaction now to do a google search instead of looking at the man
On 03/28/2014 10:47 AM, Richard W.M. Jones wrote: page? What is wrong with me?? :-P).
In that case, I think that the XML should be
<ip address='guest's IP address' prefix='prefix for network' gateway='host IP address according to guest' dns='DNS server IP given in DHCP response (and answering requests)'/>
I think that
* address should be mandatory I don't understand why this should be mandatory.
Well, if the <ip> element is present, there should be *something* in it, otherwise why bother? But I do see the point that someone might want to specify the network (or rather the gateway), but not care which IP on that network was used by the guest.
Actually I don't think any of them should be mandatory (the same as qemu), but the only one which libguestfs would specify is the network. The guest can choose any IP address on the network >= .5 and things will work. The libguestfs appliance arbitrarily uses .10 since DHCP would be too slow.
But if the guest is going to choose an IP address itself rather than getting one from DHCP, it should know that it's choosing an address on the correct network, and I don't like the idea of depending on qemu's current choice of default network/host IP/dns IP to remain constant. That's why I think that, even if the attributes aren't required (or present) in the XML, we should define defaults for anything that could affect the correctness of what *is* specified, and always fill them in on the qemu commandline.
* prefix should default to the natural prefix for that particular address (we have a function somewhere in the network conf or driver code that already does this) Right, qemu does this if you don't specify it on the command line.
* gateway should default to
address & ~(0xffffffff<<(32-prefix)) + 1 qemu will choose .2 unless specified.
Yes, currently that's what it will do, and it's not likely to change, but it might. And if we support this config for a different network implementation, their default probably *won't* be .2.
[...]
For libguestfs we only care about setting the network address + prefix, and to fix this bug I'm quite happy for the patch to only do that, and we can worry about the other things another time.
Sure, that's fine with me too, as long as we don't do it in a way that paints us into a corner such that we *can't* add the other things in a logical manner. (/me has gotten in the habit of considering patches as one would consider moves in a chess game, which sometimes has the unfortunate side effect of leading to gridlock)

On Fri, Mar 28, 2014 at 02:17:50PM +0200, Laine Stump wrote: > On 03/28/2014 01:03 PM, Richard W.M. Jones wrote: > > On Fri, Mar 28, 2014 at 12:16:43PM +0200, Laine Stump wrote: > >> On 03/28/2014 10:47 AM, Richard W.M. Jones wrote: > >>> On Fri, Mar 28, 2014 at 10:33:39AM +0200, Laine Stump wrote: > >> * address should be mandatory > > I don't understand why this should be mandatory. [...] > > Actually I don't > > think any of them should be mandatory (the same as qemu), but the only > > one which libguestfs would specify is the network. The guest can > > choose any IP address on the network >= .5 and things will work. The > > libguestfs appliance arbitrarily uses .10 since DHCP would be too > > slow. > > But if the guest is going to choose an IP address itself rather than > getting one from DHCP, it should know that it's choosing an address on > the correct network, and I don't like the idea of depending on qemu's > current choice of default network/host IP/dns IP to remain constant. That's fair enough, although it seems unlikely it would change especially as no one is really touching the SLIRP code beyond urgent security fixes. However I'm not clear if when you said "address should be mandatory" you meant the network address or the guest DHCP start address. It seemed to me you meant the DHCP start address, but now I think perhaps you meant the network address? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/

On Fri, Mar 28, 2014 at 08:47:48AM +0000, Richard W.M. Jones wrote:
On Fri, Mar 28, 2014 at 10:33:39AM +0200, Laine Stump wrote:
Beyond that, a question not with your patch, but with qemu's implemenation - does it always assume that the gateway address is $network.1 ?
Actually network.2. The default addresses are:
network: 10.0.2.0/24 (ie. mask 255.255.255.0) default gateway: 10.0.2.2 dns server: 10.0.2.3 dhcp start / normal guest address: 10.0.2.15
It _is_ possible to change the gateway address, by specifying the (confusingly named) 'host=' parameter. As you suggested I think this could be mapped to a gateway XML attribute, although libguestfs would not need to use it.
Another couple of thoughts on this patch. (1) Qemu rejects impossible network configurations -- for example, if you specify a default gateway address which is outside the network address range. However it does so without giving any specific error messages, see: http://git.qemu.org/?p=qemu.git;a=blob;f=net/slirp.c;h=cce026bf12bbead8a2bc8... Is there a case for making libvirt do the same checks and give proper error messages (and/or should we fix qemu)? (2) IPv6 is impossible with qemu's SLIRP. SLIRP is ancient BSD code and was written long before IPv6 existed, and not updated since. It looks as if this patch would allow you to do specify IPv6 addresses, but it should probably reject them at the libvirt level. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW

On 03/28/2014 12:26 PM, Richard W.M. Jones wrote:
On Fri, Mar 28, 2014 at 08:47:48AM +0000, Richard W.M. Jones wrote:
On Fri, Mar 28, 2014 at 10:33:39AM +0200, Laine Stump wrote:
Beyond that, a question not with your patch, but with qemu's implemenation - does it always assume that the gateway address is $network.1 ? Actually network.2. The default addresses are:
network: 10.0.2.0/24 (ie. mask 255.255.255.0) default gateway: 10.0.2.2 dns server: 10.0.2.3 dhcp start / normal guest address: 10.0.2.15
It _is_ possible to change the gateway address, by specifying the (confusingly named) 'host=' parameter. As you suggested I think this could be mapped to a gateway XML attribute, although libguestfs would not need to use it. Another couple of thoughts on this patch.
(1) Qemu rejects impossible network configurations -- for example, if you specify a default gateway address which is outside the network address range. However it does so without giving any specific error messages, see:
http://git.qemu.org/?p=qemu.git;a=blob;f=net/slirp.c;h=cce026bf12bbead8a2bc8...
Is there a case for making libvirt do the same checks and give proper error messages (and/or should we fix qemu)?
Yes, I agree we should check for things like that. (We should also check for that in the <network> config, as someone once pointed out in a BZ; I just keep forgetting to do it)

On 28.03.2014 09:33, Laine Stump wrote:
On 03/27/2014 07:17 PM, Michal Privoznik wrote:
We allow users to use SLIRP stack. However, there are some knobs which are not exposed to users, such as host network address, DNS server, smb, and others.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.html.in | 7 +- docs/schemas/domaincommon.rng | 23 +++++- src/conf/domain_conf.c | 88 ++++++++++++++++++++++ src/conf/domain_conf.h | 6 ++ src/qemu/qemu_command.c | 19 +++++ .../qemuxml2argvdata/qemuxml2argv-net-user-ip.args | 7 ++ .../qemuxml2argvdata/qemuxml2argv-net-user-ip.xml | 33 ++++++++ tests/qemuxml2argvtest.c | 1 + 8 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.xml
It is essential that this new <ip> element be rejected, preferably at parse time in the qemu-specific post-parse callback, for any interface type that doesn't support it. Since it is the most obvious way of specifying an IP address for a guest (and it is a way that has been requested in the past) we are otherwise certain to have a lot of support questions asking why the IP address setting isn't being used.
Well, I think rejecting it goes against current policy 'silently ignore elements unknown to the libvirt'. But I can live with that here.
Also, the attribute names seem confusing. The "address" attribute is the address of the *network*, not of the interface, and "dhcpstart" is the address of the interface. Even though qemu specifies the network address and interface address separately, the network address is really pointless, as it can/should be derived from the interface address.
It is, so what XML schema do you propose? I'm not pleased with <ip/> either. But it was the best I could come up with so far. Maybe we are aiming for more structured XML here: <interface type='user'> <network address='192.168.2.0' prefix='24'/> <dns address='92.168.2.3'/> <dhcp start='192.168.2.9'/> <mac address='00:11:22:33:44:55'/> <model type='rtl8139'/> </interface> Or even more neting: <interface type='user'> <network> <ip address='192.168.2.0' prefix='24'/> <dns address='92.168.2.3'/> <dhcp start='192.168.2.9'/> </network> <mac address='00:11:22:33:44:55'/> <model type='rtl8139'/> </interface>
(I realize that "dhcpstart" isn't *exactly* the interface IP, since qemu allows for multiple IP leases to be acquired from it's internal dhcp server, but I think that usage is very rare, and unlikely to be possible for any other backend we might support).
Beyond that, a question not with your patch, but with qemu's implemenation - does it always assume that the gateway address is $network.1 ? If that's the case, then I think definitely we should just have <ip address='x' prefix='y'/>. If there is support for specifying the gateway address, then it should be named "gateway", as is already done in the network xml for static routes.
That's good question, but my qemu code base reading skills are just too weak to answer it. Michal

On 03/28/2014 12:34 PM, Michal Privoznik wrote:
On 28.03.2014 09:33, Laine Stump wrote:
On 03/27/2014 07:17 PM, Michal Privoznik wrote:
We allow users to use SLIRP stack. However, there are some knobs which are not exposed to users, such as host network address, DNS server, smb, and others.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.html.in | 7 +- docs/schemas/domaincommon.rng | 23 +++++- src/conf/domain_conf.c | 88 ++++++++++++++++++++++ src/conf/domain_conf.h | 6 ++ src/qemu/qemu_command.c | 19 +++++ .../qemuxml2argvdata/qemuxml2argv-net-user-ip.args | 7 ++ .../qemuxml2argvdata/qemuxml2argv-net-user-ip.xml | 33 ++++++++ tests/qemuxml2argvtest.c | 1 + 8 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-user-ip.xml
It is essential that this new <ip> element be rejected, preferably at parse time in the qemu-specific post-parse callback, for any interface type that doesn't support it. Since it is the most obvious way of specifying an IP address for a guest (and it is a way that has been requested in the past) we are otherwise certain to have a lot of support questions asking why the IP address setting isn't being used.
Well, I think rejecting it goes against current policy 'silently ignore elements unknown to the libvirt'. But I can live with that here.
It's no longer unknown :-)
Also, the attribute names seem confusing. The "address" attribute is the address of the *network*, not of the interface, and "dhcpstart" is the address of the interface. Even though qemu specifies the network address and interface address separately, the network address is really pointless, as it can/should be derived from the interface address.
It is, so what XML schema do you propose? I'm not pleased with <ip/> either. But it was the best I could come up with so far. Maybe we are aiming for more structured XML here:
<interface type='user'> <network address='192.168.2.0' prefix='24'/> <dns address='92.168.2.3'/> <dhcp start='192.168.2.9'/> <mac address='00:11:22:33:44:55'/> <model type='rtl8139'/> </interface>
Or even more neting:
<interface type='user'> <network> <ip address='192.168.2.0' prefix='24'/> <dns address='92.168.2.3'/> <dhcp start='192.168.2.9'/> </network>
Interesting idea - a mini <network> inside the <interface>. If we were going to go this far, it should be in order to replicate the schema of <network> as much as possible though. Otherwise it could lead to confusion. For that reason, I think just a single <ip> element is fine, I just think the attribute names should follow function rather than following qemu (see my response to Rich). This will make it more likely that we can add support for <ip> on some other network type in the future. (For example, I could see us possibly adding the capability to use the <ip> inside an interface to automatically populate the static IP list of a libvirt network's dnsmasq instance before the domain is started, then removing it once the domain is stopped - it would really only be interested in the guest IP address, but would fail if any other info was present and didn't match the network's config).
participants (5)
-
Daniel P. Berrange
-
Eric Blake
-
Laine Stump
-
Michal Privoznik
-
Richard W.M. Jones