[libvirt] [PATCH 0/2] Removing backend support when net interface is user/direct/hostdev.

The <backend> xml tag is not supported for some interface types and the virsh command 'attach-device' permits to add <backend> settings to all of them. These commits avoid <backend> for user, direct and hostdev interface types. Julio Faracco (2): tests: removing backend xml tag inside some test cases. conf: network user/direct/hostdev do not support backend tag. src/conf/domain_conf.c | 10 ++++++++++ tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml | 2 -- tests/qemuxml2argvdata/qemuxml2argv-tap-vhost.xml | 1 - tests/qemuxml2argvtest.c | 6 +++--- .../qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml | 2 -- tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost.xml | 1 - 6 files changed, 13 insertions(+), 9 deletions(-) -- 2.7.4

Some test cases have the backend tag inside wrong interfaces. The backend xml tag does not support <interface type='user|direct|hostdev'>. So this commit removes some tags inside those interfaces. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml | 2 -- tests/qemuxml2argvdata/qemuxml2argv-tap-vhost.xml | 1 - tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml | 2 -- tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost.xml | 1 - 4 files changed, 6 deletions(-) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml b/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml index a1532cb..355c1e4 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml @@ -26,13 +26,11 @@ <mac address='52:54:00:e5:48:58'/> <model type='e1000'/> <driver name='vhost' queues='5'/> - <backend tap='/dev/null' vhost='/dev/zero'/> </interface> <interface type='user'> <mac address='52:54:00:e5:48:59'/> <model type='virtio'/> <driver name='vhost' queues='1'/> - <backend tap='/dev/null' vhost='/dev/zero'/> </interface> <serial type='pty'> <target port='0'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost.xml b/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost.xml index c2303b7..aac99aa 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-tap-vhost.xml @@ -39,7 +39,6 @@ <mac address='52:54:00:e5:48:58'/> <model type='virtio'/> <driver name='vhost' queues='5'/> - <backend tap='/dev/null' vhost='/dev/zero'/> </interface> <serial type='pty'> <target port='0'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml index 3a95b60..b9d5003 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml @@ -29,14 +29,12 @@ <interface type='user'> <mac address='52:54:00:e5:48:58'/> <model type='e1000'/> - <backend tap='/dev/null'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <interface type='user'> <mac address='52:54:00:e5:48:59'/> <model type='virtio'/> <driver name='vhost'/> - <backend tap='/dev/null' vhost='/dev/zero'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </interface> <serial type='pty'> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost.xml index 759b844..8c27470 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost.xml @@ -43,7 +43,6 @@ <mac address='52:54:00:e5:48:58'/> <model type='virtio'/> <driver name='vhost' queues='5'/> - <backend tap='/dev/null' vhost='/dev/zero'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> <serial type='pty'> -- 2.7.4

On Mon, Oct 16, 2017 at 10:07:57PM -0200, Julio Faracco wrote:
Some test cases have the backend tag inside wrong interfaces. The backend xml tag does not support <interface type='user|direct|hostdev'>. So this commit removes some tags inside those interfaces.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- tests/qemuxml2argvdata/qemuxml2argv-tap-vhost-incorrect.xml | 2 -- tests/qemuxml2argvdata/qemuxml2argv-tap-vhost.xml | 1 - tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml | 2 -- tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost.xml | 1 - 4 files changed, 6 deletions(-)
The point of these tests is to have the behavior of the <backend> parsing recorded in the test suite. IIRC these were added before we mocked the tap device creation in qemuxml2argvtest and we (ab)used the 'user' interface type to test XML parsing without having any effect on the host system. The right thing to do here is to change the interface type, not to delete the backend tag. Jan

The tag backend is not supported for user/direct/hostdev network when you try to define them inside the domain XML. So, other ways to include devices cannot be permitted too. But the attach-device command is wrongly adding unsupported features. This commit fixes this bug. After the patch: virsh # attach-device rhel7.4 backend.xml error: Failed to attach device from backend.xml error: unsupported configuration: Custom tap device path is not supported for: user Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1480251 Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/conf/domain_conf.c | 10 ++++++++++ tests/qemuxml2argvtest.c | 6 +++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0f306b7..2842533 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10518,6 +10518,16 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } else if (virXMLNodeNameEqual(cur, "backend")) { char *tmp = NULL; + /* user/direct/hostdev do not support backend */ + if (def->type == VIR_DOMAIN_NET_TYPE_USER || + def->type == VIR_DOMAIN_NET_TYPE_DIRECT || + def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Custom tap device path is not supported for: %s"), + virDomainNetTypeToString(def->type)); + goto error; + } + if ((tmp = virXMLPropString(cur, "tap"))) def->backend.tap = virFileSanitizePath(tmp); VIR_FREE(tmp); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 01e7d6f..fad8aba 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1219,9 +1219,9 @@ mymain(void) QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_HOST_PCI_MULTIDOMAIN); DO_TEST_FAILURE("net-hostdev-vfio-multidomain", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DEVICE_VFIO_PCI); - DO_TEST_FAILURE("net-hostdev-fail", - QEMU_CAPS_NODEFCONFIG, - QEMU_CAPS_DEVICE_VFIO_PCI); + DO_TEST_PARSE_ERROR("net-hostdev-fail", + QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_DEVICE_VFIO_PCI); DO_TEST("serial-file-log", -- 2.7.4

On Mon, Oct 16, 2017 at 10:07:58PM -0200, Julio Faracco wrote:
The tag backend is not supported for user/direct/hostdev network when you try to define them inside the domain XML. So, other ways to include devices cannot be permitted too. But the attach-device command is wrongly adding unsupported features. This commit fixes this bug.
After the patch:
virsh # attach-device rhel7.4 backend.xml error: Failed to attach device from backend.xml error: unsupported configuration: Custom tap device path is not supported for: user
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1480251
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/conf/domain_conf.c | 10 ++++++++++ tests/qemuxml2argvtest.c | 6 +++--- 2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0f306b7..2842533 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10518,6 +10518,16 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } else if (virXMLNodeNameEqual(cur, "backend")) { char *tmp = NULL;
+ /* user/direct/hostdev do not support backend */ + if (def->type == VIR_DOMAIN_NET_TYPE_USER || + def->type == VIR_DOMAIN_NET_TYPE_DIRECT || + def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Custom tap device path is not supported for: %s"), + virDomainNetTypeToString(def->type)); + goto error; + } + if ((tmp = virXMLPropString(cur, "tap"))) def->backend.tap = virFileSanitizePath(tmp); VIR_FREE(tmp);
Tightening the XML parser means that domains with this incorrect configuration would disappear from libvirt on upgrade. Also, the same error message is currently in qemuBuildInterfaceCommandLine, which would be dead code after this patch. qemuDomainDeviceDefValidate is a better place to move the error to; it's called for hotplugged devices too and it will only be called when parsing newly-defined domains. Jan
participants (2)
-
Julio Faracco
-
Ján Tomko