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

Re: [libvirt] [PATCH V4 4/6] libxl: support creating domain with VF assignment from a pool




>>> On 3/29/2016 at 08:05 AM, in message <56F9C6D0 5000702 suse com>, Jim Fehlig
<jfehlig suse com> wrote: 
> On 03/21/2016 02:11 AM, Chunyan Liu wrote: 
> > Add codes to support creating domain with network defition of assigning 
> > SRIOV VF from a pool. 
> > 
> > Signed-off-by: Chunyan Liu <cyliu suse com> 
> > --- 
> >  src/libxl/libxl_domain.c | 50  
> ++++++++++++++++++++++++++++++++++++++++++++++++ 
> >  tests/Makefile.am        |  3 +++ 
> >  2 files changed, 53 insertions(+) 
> > 
> > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c 
> > index c8d09b1..d11bf3a 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 
> >   
> > @@ -764,6 +765,10 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, 
> >              if (net->ifname && 
> >                  STRPREFIX(net->ifname, LIBXL_GENERATED_PREFIX_XEN)) 
> >                  VIR_FREE(net->ifname); 
> > + 
> > +            /* cleanup actual device */ 
> > +            virDomainNetRemoveHostdev(vm->def, net); 
> > +            networkReleaseActualDevice(vm->def, net); 
> >          } 
> >      } 
> >   
> > @@ -960,6 +965,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; 
> > + 
> > +        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; 
>  
> Nothing to cleanup here. I think we should just return -1 on failure paths  
> and 0 
> on success. 
>  
> > +} 
> >   
> >  /* 
> >   * Start a domain through libxenlight. 
> > @@ -1036,6 +1083,9 @@ libxlDomainStart(libxlDriverPrivatePtr driver,  
> virDomainObjPtr vm, 
> >                                      vm, true) < 0) 
> >          goto cleanup; 
> >   
> > +    if (libxlNetworkPrepareDevices(vm->def) < 0) 
> > +        goto cleanup; 
>  
> I accessed a machine to test this series and have found a few problems. 
>  
> 1. When attaching an SR-IOV vf from a pool, the attach appears successful. 
>    I see that xen's pciback driver is bound to the device in the host. I 
>    didn't see the device in the guest (could be a guest problem), 

For pv guest, from testing, it is shown in guest. For HVM, seems
xl pci-attach has the same result.

> nor in 
>    dumpxml or /var/run/libvirt/libxl/<dom-name>.xml.

Ah, there is a mistake during a review change to original patch 1/6.
Didn't notice that earlier.

In libxlDomainAttachNetDevice:
The if (!ret) is needed, maybe cleanup is not proper.
Since for actual type is hostdev case, after a successful
libxlDomainAttachHostDevice, we need to
update vm->def->nets too.

  cleanup:
     libxl_device_nic_dispose(&nic);
- out:
-    if (!ret)
+    if (!ret) {
         vm->def->nets[vm->def->nnets++] = net;
+    } else {
+        virDomainNetRemoveHostdev(vm->def, net);
+        networkReleaseActualDevice(vm->def, net);
+    }

Similar for libxDomainDetachNetDevice cleanup:
Original is still needed.

  cleanup:
     libxl_device_nic_dispose(&nic);
-    if (!ret)
+    if (!ret) {
+        networkReleaseActualDevice(vm->def, detach);
         virDomainNetRemove(vm->def, detachidx);
+    }

> Not surprisingly, 
>    this causes problems with device-detach 
>  
>    # virsh detach-device dom sriov-pool-vif.xml 
>    error: Failed to detach device from sriov-pool-vif.xml 
>    error: operation failed: no device matching mac address 00:16:3e:7a:35:df  
> found 
>  
> 2. After starting a domain containing an interface from sr-iov vf pool, the 
>    interface xml is changed from type='network' to type='hostdev'. E.g. 
>    before creating domain 
>  
>     <interface type='network'> 
>       <source network='passthrough'/> 
>       <mac address='00:16:3e:7a:35:df'/> 
>     </interface> 
>  
>    after creating domain 
>  
>     <interface type='hostdev' managed='yes'> 
>       <mac address='00:16:3e:7a:35:df'/> 
>       <driver name='xen'/> 
>       <source> 
>         <address type='pci' domain='0x0000' bus='0x0a' slot='0x11'  
> function='0x1'/> 
>       </source> 
>     </interface> 
>  
>    Maybe this is intended behavior, but it seems odd. 

This is intended.

>  
> 3. I started a domain containing an interface from sr-iov vf pool. Looks 
>    good. I then tried 'virsh detach-device ...', which never returned 
>    until keepalive timeout. The device was removed from the guest, but 
>    libvirtd got hung up in the process. Oddly, I wasn't able to interrupt 
>    libvirtd with gdb to see what its threads were doing. 

This is libxl problem. With xl pci-detach, it has the same issue.

>  
> Do you have access to an appropriate machine to test these scenarios, to see  
> if 
> the issues are reproducible or just a problem with my configuration? 
>  
> Regards, 
> Jim 
>  



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