[libvirt] [PATCH 1/1] conf: handle empty string in interface target name
Empty name is not allowed by schema but qemu is able to start with such a config (and I guess some other hypervisors too). As a result name will be generated by kernel and have form 'tap<N>'. At the same time if target element is ommited in config the name will be generated by libvirt and have form 'vnet<N>'. Let's have only the latter pattern for autogenerated names by treating empty name as ommited. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- There is a RFS for the issue - https://www.redhat.com/archives/libvir-list/2019-September/msg00645.html src/conf/domain_conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 76aaa63f57..7ff3972cbb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11592,6 +11592,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } else if (!ifname && virXMLNodeNameEqual(cur, "target")) { ifname = virXMLPropString(cur, "dev"); + if (ifname[0] == '\0') + VIR_FREE(ifname); managed_tap = virXMLPropString(cur, "managed"); } else if ((!ifname_guest || !ifname_guest_actual) && virXMLNodeNameEqual(cur, "guest")) { -- 2.23.0
On 9/23/19 8:55 AM, Nikolay Shirokovskiy wrote:
Empty name is not allowed by schema but qemu is able to start with such a config (and I guess some other hypervisors too). As a result name will be generated by kernel and have form 'tap<N>'. At the same time if target element is ommited in config the name will be generated by libvirt and have form 'vnet<N>'. Let's have only the latter pattern for autogenerated names by treating empty name as ommited.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
There is a RFS for the issue - https://www.redhat.com/archives/libvir-list/2019-September/msg00645.html
src/conf/domain_conf.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 76aaa63f57..7ff3972cbb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11592,6 +11592,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } else if (!ifname && virXMLNodeNameEqual(cur, "target")) { ifname = virXMLPropString(cur, "dev"); + if (ifname[0] == '\0') + VIR_FREE(ifname); managed_tap = virXMLPropString(cur, "managed"); } else if ((!ifname_guest || !ifname_guest_actual) && virXMLNodeNameEqual(cur, "guest")) {
On Mon, Sep 23, 2019 at 11:21:33 -0300, Daniel Henrique Barboza wrote:
On 9/23/19 8:55 AM, Nikolay Shirokovskiy wrote:
Empty name is not allowed by schema but qemu is able to start with such a config (and I guess some other hypervisors too). As a result name will be generated by kernel and have form 'tap<N>'. At the same time if target element is ommited in config the name will be generated by libvirt and have form 'vnet<N>'. Let's have only the latter pattern for autogenerated names by treating empty name as ommited.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Change like this usually requires a test XML addition so that we can validate and prevent regressions.
On 23.09.2019 17:44, Peter Krempa wrote:
On Mon, Sep 23, 2019 at 11:21:33 -0300, Daniel Henrique Barboza wrote:
On 9/23/19 8:55 AM, Nikolay Shirokovskiy wrote:
Empty name is not allowed by schema but qemu is able to start with such a config (and I guess some other hypervisors too). As a result name will be generated by kernel and have form 'tap<N>'. At the same time if target element is ommited in config the name will be generated by libvirt and have form 'vnet<N>'. Let's have only the latter pattern for autogenerated names by treating empty name as ommited.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Change like this usually requires a test XML addition so that we can validate and prevent regressions.
I would like to, but seems it requires mocking syscalls for dealing with taps/interfaces which is a good deal of work so I guess next time... Nikolay
On Wed, Sep 25, 2019 at 12:42:07 +0000, Nikolay Shirokovskiy wrote:
On 23.09.2019 17:44, Peter Krempa wrote:
On Mon, Sep 23, 2019 at 11:21:33 -0300, Daniel Henrique Barboza wrote:
On 9/23/19 8:55 AM, Nikolay Shirokovskiy wrote:
Empty name is not allowed by schema but qemu is able to start with such a config (and I guess some other hypervisors too). As a result name will be generated by kernel and have form 'tap<N>'. At the same time if target element is ommited in config the name will be generated by libvirt and have form 'vnet<N>'. Let's have only the latter pattern for autogenerated names by treating empty name as ommited.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Change like this usually requires a test XML addition so that we can validate and prevent regressions.
I would like to, but seems it requires mocking syscalls for dealing with taps/interfaces which is a good deal of work so I guess next time...
The XML2XML test which does not invoke any preparation code should be enough in this case. Also rather than mocking syscalls libvirt's prepare steps were split up so that tests can skip steps which would actually modify the host so possibly you'll just need to refactor the setup code to be executed in a different place.
On 25.09.2019 15:52, Peter Krempa wrote:
On Wed, Sep 25, 2019 at 12:42:07 +0000, Nikolay Shirokovskiy wrote:
On 23.09.2019 17:44, Peter Krempa wrote:
On Mon, Sep 23, 2019 at 11:21:33 -0300, Daniel Henrique Barboza wrote:
On 9/23/19 8:55 AM, Nikolay Shirokovskiy wrote:
Empty name is not allowed by schema but qemu is able to start with such a config (and I guess some other hypervisors too). As a result name will be generated by kernel and have form 'tap<N>'. At the same time if target element is ommited in config the name will be generated by libvirt and have form 'vnet<N>'. Let's have only the latter pattern for autogenerated names by treating empty name as ommited.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Change like this usually requires a test XML addition so that we can validate and prevent regressions.
I would like to, but seems it requires mocking syscalls for dealing with taps/interfaces which is a good deal of work so I guess next time...
The XML2XML test which does not invoke any preparation code should be enough in this case.
xml2xml can only check that dev="" will turn in missing <target> on parse/format round. But this looks like not enough as we could have dev="" turn in dev="tap0" as now which is scrapped out on format as autogenerated. Unfortunately dev name is autogenerated in code xml2xml does not touch. I hoped to use xml2argv test thus the above words on mocking but I realized this test is of no help too because host dev name is not present in command line the test checks. Nikolay
Also rather than mocking syscalls libvirt's prepare steps were split up so that tests can skip steps which would actually modify the host so possibly you'll just need to refactor the setup code to be executed in a different place.
participants (3)
-
Daniel Henrique Barboza -
Nikolay Shirokovskiy -
Peter Krempa