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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 08, 2017 at 12:37:29PM +0200, Ján Tomko wrote:
> 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

Fair enough, in that case for patches 5-12

Reviewed-by: Pavel Hrdina <phrdina@xxxxxxxxxx>

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux