From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTP id 818815A0082 for ; Mon, 9 Jan 2023 07:47:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1673246840; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=i4YfA2YFnUtdy3IJqqB69wG6fdVAR9PzfBgZ+tFL+pc=; b=G7jejkW3ek5ACTMF9dIxaQcgBxD9fWChMV5oudkRokHx9+i1kDaXbd5peI8Ex+Uxa+easC lytnhNH/bem4aOTXEfgONehkgmtK/iYHfNpJmjeMJp5r1eZoe5+mC+KxBM9XBc19V5B+hn GW/4jZFVmLmWRTjiymlbOmEyS7+TrxI= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-659-ZEoJ9s1fPIGiIL1BmN2Xjw-1; Mon, 09 Jan 2023 01:47:18 -0500 X-MC-Unique: ZEoJ9s1fPIGiIL1BmN2Xjw-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 59A643C10EC0 for ; Mon, 9 Jan 2023 06:47:18 +0000 (UTC) Received: from fedora (ovpn-192-19.brq.redhat.com [10.40.192.19]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 313152166B26; Mon, 9 Jan 2023 06:47:16 +0000 (UTC) From: =?iso-8859-1?B?SuFu?= Tomko To: Laine Stump Subject: Re: [libvirt PATCH 3/9] conf: put interface parsing/formatting separate functions Message-ID: References: <20230109041112.368790-1-laine@redhat.com> <20230109041112.368790-4-laine@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="AZ5rxQPYGU5gDT93" Content-Disposition: inline In-Reply-To: <20230109041112.368790-4-laine@redhat.com> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 X-MailFrom: jtomko@redhat.com X-Mailman-Rule-Hits: nonmember-moderation X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation Message-ID-Hash: 4AVNFVIT3ML7NQ5GOQZRKDWX3LIJAMJN X-Message-ID-Hash: 4AVNFVIT3ML7NQ5GOQZRKDWX3LIJAMJN X-Mailman-Approved-At: Tue, 10 Jan 2023 16:08:41 +0100 CC: libvir-list@redhat.com, sbrivio@redhat.com, passt-dev@passt.top X-Mailman-Version: 3.3.3 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Date: Mon, 09 Jan 2023 06:47:21 X-Original-Date: Mon, 9 Jan 2023 07:47:38 +0100 --AZ5rxQPYGU5gDT93 Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On a Sunday in 2023, Laine Stump wrote: >In preparation for adding more stuff to . > >Signed-off-by: Laine Stump >--- > >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 =3D virXMLPropString(node, "tap"); >+ g_autofree char *vhost =3D virXMLPropString(node, "vhost"); >+ >+ if (tap) >+ def->backend.tap =3D virFileSanitizePath(tap); >+ >+ if (vhost && >+ def->type !=3D VIR_DOMAIN_NET_TYPE_HOSTDEV && >+ virDomainNetIsVirtioModel(def)) { >+ def->backend.vhost =3D virFileSanitizePath(vhost); >+ } >+ >+ return 0; >+} >+ >+ > static int > virDomainNetDefParseXMLRequireSource(virDomainNetDef *def, > xmlNodePtr source_node) >@@ -9016,6 +9036,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, > xmlNodePtr mac_node =3D NULL; > xmlNodePtr target_node =3D NULL; > xmlNodePtr coalesce_node =3D NULL; >+ xmlNodePtr backend_node =3D NULL; > VIR_XPATH_NODE_AUTORESTORE(ctxt) > int rv; > g_autofree char *macaddr =3D NULL; src/conf/domain_conf.c:9220:22: error: unused variable 'tap' [-Werror,-Wunu= sed-variable] g_autofree char *tap =3D NULL; ^ 1 error generated. >@@ -9319,9 +9340,6 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, > (virNetDevVlanParse(vlan_node, ctxt, &def->vlan) < 0)) > return NULL; > >- if ((tap =3D virXPathString("string(./backend/@tap)", ctxt))) >- def->backend.tap =3D virFileSanitizePath(tap); >- > if ((mac_node =3D virXPathNode("./mac", ctxt))) { > if ((macaddr =3D virXMLPropString(mac_node, "address"))) { > if (virMacAddrParse((const char *)macaddr, &def->mac) < 0) { With the unused variable removed: Reviewed-by: J=E1n Tomko Jano --AZ5rxQPYGU5gDT93 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQT+Rn5j0qdK2hQgnuAU0rOr/y4PvAUCY7u4hgAKCRAU0rOr/y4P vKAjAP9dtYjyzvLMp1tPrMXgeyVPmiFaVkUOg2R17TjWBVTZTAD/U1a/lRuowo/J jz701CN/Q4G5Z8OKMcKwXIziHP6F7gY= =BnXR -----END PGP SIGNATURE----- --AZ5rxQPYGU5gDT93--