Re: [PATCH v2] qemu: Assign device address before qemuDomainSupportsNicdev

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

 



> $SUBJ:
>
> s/address before qemuDomainSupportsNicdev/addresses earlier in
> qemuDomainAttachNetDevice/
>
>
> On 12/17/18 6:30 AM, Wang Yechao wrote:
> > qemuDomainSupportsNicdev will check the device address type on
> > aarch64. If it is invoked before device address assigned, hotadd
> > vhostuser interface with no address specified will get error.
> > Let qemuDomainEnsurePCIAddress run before qemuDomainSupportsNicdev.
> >
>
> I would change this to:
>
> If code in the @actualType switch needs to have/know which PCI Address
> is being used, then we must assign it earlier.  In particular a
> vhost-user device needs to call qemuDomainSupportsNicdev which requires
> an address to be defined.
>
>
> > Signed-off-by: Wang Yechao <wang yechao255 zte com cn>
> > ---
> > v1 patch:
> > https://www.redhat.com/archives/libvir-list/2018-December/msg00435.html
> >
> > Changes in v2:
> >  - do not modify the address type, let qemuDomainEnsurePCIAddress do it.
> > ---
> >  src/qemu/qemu_hotplug.c | 38 +++++++++++++++++++-------------------
> >  1 file changed, 19 insertions(+), 19 deletions(-)
> >
>
> One concern I have is when moving code is checking anything that gets
> called in between the moved code for "anomalies".  The only one that the
> when @actualType is VIR_DOMAIN_NET_TYPE_HOSTDEV, then
> qemuDomainAttachHostDevice is called.  Still AttachHostDevice will
> eventually check the address anyway, so I believe it's fine.
>
> As long as you're OK with my commit message adjustments and no one jumps
> in with something else...
>
> Reviewed-by: John Ferlan <jferlan redhat com>
>
> John

I am OK with your commit message adjustments, it's more clearer and better.

---
Best wishes
Wang Yechao
--
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