On 03/30/2016 03:38 AM, Chun Yan Liu wrote:
>
>>>> 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.
Yes, I'm seeing the same thing. Looks like a bug in libxl or qemu.
>> 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);
> + }
Yikes! I should have looked at the code closer before making those changes -
lesson learned. I sent a small series to fix it
https://www.redhat.com/archives/libvir-list/2016-March/msg01470.html
>> 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.
Yep, another bug in xen-unstable libxl. With the small fixes in the series I
mentioned above, this patch looks and tests good :-). Since it has been around
for quite some time, I've asked for permission to commit it even though we are
in 1.3.3 freeze.
Regards,
Jim
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list