>>> On 3/29/2016 at 08:05 AM, in message <56F9C6D0.5000702@xxxxxxxx>, Jim Fehlig
<jfehlig@xxxxxxxx> 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@xxxxxxxx>
> > ---
> > 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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list