[libvirt PATCH 3/9] conf: put interface <backend> parsing/formatting separate functions
Laine Stump
laine at redhat.com
Mon Jan 9 07:04:19 UTC 2023
On 1/9/23 1:47 AM, Ján Tomko wrote:
> On a Sunday in 2023, Laine Stump wrote:
>> In preparation for adding more stuff to <backend>.
>>
>> Signed-off-by: Laine Stump <laine at redhat.com>
>> ---
>>
>> I wanted virDomainNetBackendParseXML to simply take a
>> virDomainNetBackend*, but there is a test case specifically checking
>> to be sure that backend/vhost isn't parsed if the interface isn't
>> virtio. Silently Ignoring+stripping this during parse is arguably the
>> wrong thing to do - either we should log an error on validation, or we
>> should just leave it in (it's only ever used if the interface is
>> virtio), but that's a problem for another day.
>>
>> (Opinions on the proper thing to do are welcome - since it's currently
>> always stripped out on parse, I *think* I could begin checking for it
>> during validation - there is no way that old code could leave the
>> backend/vhost for a non-virtio interface in any domain xml written to
>> disk.
>
> This seems like the right thing to do.
>
>> Alternately would could just allow it to be parsed and
>> faithfully format it even when the interface isn't virtio, and not log
>> any error.)
>>
>> src/conf/domain_conf.c | 57 +++++++++++++++++++++++++++++++-----------
>> 1 file changed, 42 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 30b0cef131..9502f2ebab 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -8970,6 +8970,26 @@ virDomainNetDefParseXMLDriver(virDomainNetDef
>> *def,
>> }
>>
>>
>> +static int
>> +virDomainNetBackendParseXML(xmlNodePtr node,
>> + virDomainNetDef *def)
>> +{
>> + g_autofree char *tap = virXMLPropString(node, "tap");
>> + g_autofree char *vhost = virXMLPropString(node, "vhost");
>> +
>> + if (tap)
>> + def->backend.tap = virFileSanitizePath(tap);
>> +
>> + if (vhost &&
>> + def->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
>> + virDomainNetIsVirtioModel(def)) {
>> + def->backend.vhost = virFileSanitizePath(vhost);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +
>> static int
>> virDomainNetDefParseXMLRequireSource(virDomainNetDef *def,
>> xmlNodePtr source_node)
>> @@ -9016,6 +9036,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
>> xmlNodePtr mac_node = NULL;
>> xmlNodePtr target_node = NULL;
>> xmlNodePtr coalesce_node = NULL;
>> + xmlNodePtr backend_node = NULL;
>> VIR_XPATH_NODE_AUTORESTORE(ctxt)
>> int rv;
>> g_autofree char *macaddr = NULL;
>
> src/conf/domain_conf.c:9220:22: error: unused variable 'tap'
> [-Werror,-Wunused-variable]
> g_autofree char *tap = NULL;
> ^
> 1 error generated.
Yeah, I just found that when I looked at the results of gitlab CI (it
isn't found by gcc in Fedora 37 :-/). I fixed that as well as
eliminating a leak in the new function virDomainNetPortForwardFree() (I
had forgotten to free the pf->range after freeing all the ranges that it
pointed to).
>
>> @@ -9319,9 +9340,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
>> (virNetDevVlanParse(vlan_node, ctxt, &def->vlan) < 0))
>> return NULL;
>>
>> - if ((tap = virXPathString("string(./backend/@tap)", ctxt)))
>> - def->backend.tap = virFileSanitizePath(tap);
>> -
>> if ((mac_node = virXPathNode("./mac", ctxt))) {
>> if ((macaddr = virXMLPropString(mac_node, "address"))) {
>> if (virMacAddrParse((const char *)macaddr, &def->mac) < 0) {
>
> With the unused variable removed:
>
> Reviewed-by: Ján Tomko <jtomko at redhat.com>
>
> Jano
More information about the libvir-list
mailing list