Re: [libvirt] [PATCHv3 05/14] Add virtio-related options to interfaces

On Thu, Jun 08, 2017 at 12:30:15PM +0200, Pavel Hrdina wrote: >On Wed, Jun 07, 2017 at 09:24:55PM +0200, Ján Tomko wrote: >> <interface type='user'> >> <mac address='52:54:56:5a:5c:5e'/> >> <model type='virtio'/> >> <driver iommu='on' ats='on'/> >> </interface> >> >> https://bugzilla.redhat.com/show_bug.cgi?id=1283251 >> --- >> docs/formatdomain.html.in | 19 ++++ >> docs/schemas/domaincommon.rng | 12 ++ >> src/conf/domain_conf.c | 121 +++++++++++++++++++++ >> src/conf/domain_conf.h | 10 ++ >> .../qemuxml2argv-virtio-options.xml | 1 + >> 5 files changed, 163 insertions(+) >> >> @@ -19163,6 +19278,10 @@ virDomainNetDefCheckABIStability(virDomainNetDefPtr src, >> return false; >> } >> >> + if (src->virtio && dst->virtio && >> + !virDomainVirtioOptionsCheckABIStability(src->virtio, dst->virtio)) >> + return false; > >Shouldn't we also check !!src->virtion == !!dst->virtio? The parser >for virtio options always allocates @virtio so technically it shouldn't >happen but throughout the code we check whether @virtio is allocated. > virtio can be NULL in case the domain definition was not created by parsing XML. Without the non-NULL checks in the formatter, some of the tests were crashing. If someone somehow manages to bypass the parser for the QEMU driver, which is the only one where these options make any sense, I'm okay with not having its ABI stability checked, as long as we don't crash. Jan