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

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@suse.com> --- src/libxl/libxl_conf.c | 5 +++-- src/libxl/libxl_domain.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- src/libxl/libxl_driver.c | 12 ++++++++++++ tests/Makefile.am | 3 +++ 4 files changed, 65 insertions(+), 3 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 6320421..f50c68a 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1260,7 +1260,8 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) return -1; for (i = 0; i < nnics; i++) { - if (l_nics[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) + if (l_nics[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV || + l_nics[i]->data.network.actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) continue; if (libxlMakeNic(def, l_nics[i], &x_nics[nvnics])) @@ -1278,7 +1279,7 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) VIR_SHRINK_N(x_nics, nnics, nnics - nvnics); d_config->nics = x_nics; - d_config->num_nics = nnics; + d_config->num_nics = nvnics; return 0; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index cf5c9f6..9bf7a5a 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -35,6 +35,7 @@ #include "virstring.h" #include "virtime.h" #include "locking/domain_lock.h" +#include "network/bridge_driver.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -314,7 +315,7 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, virDomainHostdevSubsysPCIPtr pcisrc; if (dev->type == VIR_DOMAIN_DEVICE_NET) - hostdev = &(dev->data.net)->data.hostdev.def; + hostdev = &dev->data.net->data.hostdev.def; else hostdev = dev->data.hostdev; pcisrc = &hostdev->source.subsys.u.pci; @@ -916,6 +917,48 @@ libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback) libxl_event_free(ctx, ev); } +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; +} /* * Start a domain through libxenlight. @@ -992,6 +1035,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 4a9134e..6aca042 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3047,6 +3047,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"), @@ -3388,6 +3394,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 ee16666..6a9fb19 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -526,6 +526,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 = \ -- 1.8.5.6

On 01/25/2016 07:24 PM, 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@suse.com> --- src/libxl/libxl_conf.c | 5 +++-- src/libxl/libxl_domain.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- src/libxl/libxl_driver.c | 12 ++++++++++++ tests/Makefile.am | 3 +++ 4 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 6320421..f50c68a 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1260,7 +1260,8 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) return -1;
for (i = 0; i < nnics; i++) { - if (l_nics[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) + if (l_nics[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV || + l_nics[i]->data.network.actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV)
Elsewhere we use the virDomainNetGetActualType() accessor function.
continue;
if (libxlMakeNic(def, l_nics[i], &x_nics[nvnics])) @@ -1278,7 +1279,7 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config)
VIR_SHRINK_N(x_nics, nnics, nnics - nvnics); d_config->nics = x_nics; - d_config->num_nics = nnics; + d_config->num_nics = nvnics;
This looks like a bug fix. If so, it should be in a separate patch.
return 0;
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index cf5c9f6..9bf7a5a 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -35,6 +35,7 @@ #include "virstring.h" #include "virtime.h" #include "locking/domain_lock.h" +#include "network/bridge_driver.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -314,7 +315,7 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, virDomainHostdevSubsysPCIPtr pcisrc;
if (dev->type == VIR_DOMAIN_DEVICE_NET) - hostdev = &(dev->data.net)->data.hostdev.def; + hostdev = &dev->data.net->data.hostdev.def;
This looks like a bug fix too.
else hostdev = dev->data.hostdev; pcisrc = &hostdev->source.subsys.u.pci; @@ -916,6 +917,48 @@ libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback) libxl_event_free(ctx, ev); }
+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; +
I noticed a similar function in the qemu driver uses virDomainHostdevFind() to see if the hostdev is already in use. Is that needed here too?
+ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
Does this need to include checking if backend is already set (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) ? I suppose it should always be VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN.
+ pcisrc->backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN; + + if (virDomainHostdevInsert(def, hostdev) < 0) + goto cleanup; + } + } + ret = 0; + cleanup: + return ret; +}
Does anything done by this function need to be undone when the domain is shutdown? Regards, Jim

Jim Fehlig <jfehlig@suse.com> 2016-2-20 上午 7:30 >>> On 01/25/2016 07:24 PM, 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@suse.com> --- src/libxl/libxl_conf.c | 5 +++-- src/libxl/libxl_domain.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- src/libxl/libxl_driver.c | 12 ++++++++++++ tests/Makefile.am | 3 +++ 4 files changed, 65 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 6320421..f50c68a 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1260,7 +1260,8 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) return -1;
for (i = 0; i < nnics; i++) { - if (l_nics[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) + if (l_nics[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV || + l_nics[i]->data.network.actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV)
Elsewhere we use the virDomainNetGetActualType() accessor function. Ah, yes, I'll update.
continue;
if (libxlMakeNic(def, l_nics[i], &x_nics[nvnics])) @@ -1278,7 +1279,7 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config)
VIR_SHRINK_N(x_nics, nnics, nnics - nvnics); d_config->nics = x_nics; - d_config->num_nics = nnics; + d_config->num_nics = nvnics;
This looks like a bug fix. If so, it should be in a separate patch. Yes. Will put in a separate patch.
return 0;
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index cf5c9f6..9bf7a5a 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -35,6 +35,7 @@ #include "virstring.h" #include "virtime.h" #include "locking/domain_lock.h" +#include "network/bridge_driver.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL
@@ -314,7 +315,7 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, virDomainHostdevSubsysPCIPtr pcisrc;
if (dev->type == VIR_DOMAIN_DEVICE_NET) - hostdev = &(dev->data.net)->data.hostdev.def; + hostdev = &dev->data.net->data.hostdev.def;
This looks like a bug fix too. Yes. With '()' has no harm, but it doesn't need.
else hostdev = dev->data.hostdev; pcisrc = &hostdev->source.subsys.u.pci; @@ -916,6 +917,48 @@ libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback) libxl_event_free(ctx, ev); }
+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; +
I n > see if the hostdev is already in use. Is that needed here too? Later when doing HostdevAttach, it will check. So I think it's not necessary.
+ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
Does this need to include checking if backend is already set (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) ? I suppose it should always be VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN. There are two cases, one is specified as <interface type='hostdev'>, for this case, it will be handled during DeviceDefParse phase, it's already set as BACKEND_XEN; another case is from a pool of SRIOV vfs, it won't be handled during DeviceDefParse as a hostdev, so the backend type is still BACKEND_DEFAULT, for this case, we need to set it to be BACKEND_XEN.
+ pcisrc->backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN; + + if (virDomainHostdevInsert(def, hostdev) < 0) + goto cleanup; + } + } + ret = 0; + cleanup: + return ret; +}
Does anything done by this function need to be undone when the domain is shutdown? No, when domain is shutdown, all is handled by hostdev process.
Chunyan
Regards, Jim
participants (3)
-
Chun Yan Liu
-
Chunyan Liu
-
Jim Fehlig