[libvirt] [PATCH V3 2/2] libxl: support assignment from a pool of SRIOV VFs

[Chun Yan Liu]

>>> On 3/11/2016 at 05:56 AM, in message <56E1ED8F.4070807 at suse.com>, Jim Fehlig
<jfehlig at suse.com> wrote: 
> Chunyan Liu wrote: 
> > Add codes to support creating domain with network defition of assigning 
> > SRIOV VF from a pool. And fix hot plug and unplug SRIOV VF under this 
> > kind of network defition. 
> >  
> > Signed-off-by: Chunyan Liu <cyliu at suse.com> 
> > --- 
> > Changes: 
> >   * move a common change in libxl_conf.c into separate patch 
> >  
> >  src/libxl/libxl_domain.c | 46  
> ++++++++++++++++++++++++++++++++++++++++++++++ 
> >  src/libxl/libxl_driver.c | 12 ++++++++++++ 
> >  tests/Makefile.am        |  3 +++ 
> >  3 files changed, 61 insertions(+) 
> >  
> > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c 
> > index c8d09b1..8191911 100644 
> > --- a/src/libxl/libxl_domain.c 
> > +++ b/src/libxl/libxl_domain.c 
> > @@ -36,6 +36,7 @@ 
> >  #include "virtime.h" 
> >  #include "locking/domain_lock.h" 
> >  #include "xen_common.h" 
> > +#include "network/bridge_driver.h" 
> >   
> >  #define VIR_FROM_THIS VIR_FROM_LIBXL 
> >   
> > @@ -960,6 +961,48 @@ libxlDomainCreateIfaceNames(virDomainDefPtr def,  
> libxl_domain_config *d_config) 
> >      } 
> >  } 
> >   
> > +static int 
> > +libxlNetworkPrepareDevices(virDomainDefPtr def) 
> > +{ 
> > +    int ret = -1; 
> > +    size_t i; 
> > + 
> > +    for (i = 0; i < def->nnets; i++) { 
> > +        virDomainNetDefPtr net = def->nets[i]; 
> > +        int actualType; 
> > + 
> > +        /* If appropriate, grab a physical device from the configured 
> > +         * network's pool of devices, or resolve bridge device name 
> > +         * to the one defined in the network definition. 
> > +         */ 
> > +        if (networkAllocateActualDevice(def, net) < 0) 
> > +            goto cleanup; 
>  
> I noticed that other callers of networkAllocateActualDevice() call 
> networkReleaseActualDevice() in error paths,  
> qemuProcessNetworkPrepareDevices() 
> being a notable exception. Is it necessary to call  
> networkReleaseActualDevice() 
> in 'cleanup' if this function fails?

Not necessary in this function 'cleanup', but needed to call somewhere else,
e.g. this function is called in libxlDomainStart, so whenever libxlDomainStart
fails, need to call networkAllocateActualDevice to release resources. It will
update dev->connections for next time correct handling (if for any vif in
the pool, dev->connections != 0, it's considered as unavailable.)

Places need to call networkReleaseActualDevice(): in libxlDomainCleanup,
when libxlDomainStart fails, in hot-remove device, when hot-add device fails.

Will update and repost.

BTW: in libxlDomainStart, when it fails, why not call libxlDomainCleanup?

- Chunyan
 
>  
> Regards, 
> Jim 
>  
> > + 
> > +        actualType = virDomainNetGetActualType(net); 
> > +        if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV && 
> > +            net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { 
> > +            /* Each type='hostdev' network device must also have a 
> > +             * corresponding entry in the hostdevs array. For netdevs 
> > +             * that are hardcoded as type='hostdev', this is already 
> > +             * done by the parser, but for those allocated from a 
> > +             * network / determined at runtime, we need to do it 
> > +             * separately. 
> > +             */ 
> > +            virDomainHostdevDefPtr hostdev =  
> virDomainNetGetActualHostdev(net); 
> > +            virDomainHostdevSubsysPCIPtr pcisrc =  
> &hostdev->source.subsys.u.pci; 
> > + 
> > +            if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && 
> > +                hostdev->source.subsys.type ==  
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) 
> > +                pcisrc->backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN; 
> > + 
> > +            if (virDomainHostdevInsert(def, hostdev) < 0) 
> > +                goto cleanup; 
> > +        } 
> > +    } 
> > +    ret = 0; 
> > + cleanup: 
> > +    return ret; 
> > +} 
> >   
> >  /* 
> >   * Start a domain through libxenlight. 
> > @@ -1036,6 +1079,9 @@ libxlDomainStart(libxlDriverPrivatePtr driver,  
> virDomainObjPtr vm, 
> >                                      vm, true) < 0) 
> >          goto cleanup; 
> >   
> > +    if (libxlNetworkPrepareDevices(vm->def) < 0) 
> > +        goto cleanup; 
> > + 
> >      if (libxlBuildDomainConfig(driver->reservedGraphicsPorts, vm->def, 
> >                                 cfg->ctx, &d_config) < 0) 
> >          goto cleanup; 
> > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c 
> > index 87ec5a5..a0b157b 100644 
> > --- a/src/libxl/libxl_driver.c 
> > +++ b/src/libxl/libxl_driver.c 
> > @@ -2982,6 +2982,12 @@ libxlDomainAttachHostPCIDevice(libxlDriverPrivatePtr  
> driver, 
> >   
> >      libxl_device_pci_init(&pcidev); 
> >   
> > +    /* For those allocated from a network pool/ determined at runtime,  
> it's 
> > +     * not handled by libxlDomainDeviceDefPostParse as hostdev, we need to 
> > +     * set backend correctly. 
> > +     */ 
> > +    pcisrc->backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN; 
> > + 
> >      if (virDomainHostdevFind(vm->def, hostdev, &found) >= 0) { 
> >          virReportError(VIR_ERR_OPERATION_FAILED, 
> >                         _("target pci device %.4x:%.2x:%.2x.%.1x already  
> exists"), 
> > @@ -3323,6 +3329,12 @@ libxlDomainDetachHostPCIDevice(libxlDriverPrivatePtr  
> driver, 
> >   
> >      libxl_device_pci_init(&pcidev); 
> >   
> > +    /* For those allocated from a network pool/ determined at runtime,  
> it's 
> > +     * not handled by libxlDomainDeviceDefPostParse as hostdev, we need to 
> > +     * set backend correctly. 
> > +     */ 
> > +    pcisrc->backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN; 
> > + 
> >      idx = virDomainHostdevFind(vm->def, hostdev, &detach); 
> >      if (idx < 0) { 
> >          virReportError(VIR_ERR_OPERATION_FAILED, 
> > diff --git a/tests/Makefile.am b/tests/Makefile.am 
> > index 90981dc..b08cc7a 100644 
> > --- a/tests/Makefile.am 
> > +++ b/tests/Makefile.am 
> > @@ -530,6 +530,9 @@ endif ! WITH_XEN 
> >   
> >  if WITH_LIBXL 
> >  libxl_LDADDS = ../src/libvirt_driver_libxl_impl.la 
> > +if WITH_NETWORK 
> > +libxl_LDADDS += ../src/libvirt_driver_network_impl.la 
> > +endif WITH_NETWORK 
> >  libxl_LDADDS += $(LDADDS) 
> >   
> >  xlconfigtest_SOURCES = \ 
>  
>