From 869c76c029390ebb6e2b89b3bb0ccf9ac4610806 Mon Sep 17 00:00:00 2001 From: Jim Fehlig Date: Tue, 23 Feb 2016 16:20:29 -0700 Subject: [PATCH] libxl: use a common virConnect object Depending on the type of NIC or disk, libxlMakeNic and libxlMakeDisk may need a virConnect object, e.g. to obtain a port from the network driver or a secret from the secret driver. Instead of opening a virConnect object in each of these functions, have the callers provide the object. This approach provides benefits over opening individual virConnect objects, e.g. already fixing a virConnect unref bug in libxlMakeNic. The downside is changing several functions to include a virConnectPtr parameter. Signed-off-by: Jim Fehlig --- src/libxl/libxl_conf.c | 71 ++++++++++++++++++++++++++---------------------- src/libxl/libxl_conf.h | 7 +++-- src/libxl/libxl_driver.c | 43 ++++++++++++++++++----------- 3 files changed, 71 insertions(+), 50 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index a109bb5..5d5c85e 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1071,9 +1071,10 @@ libxlMakeNetworkDiskSrcStr(virStorageSourcePtr src, } static int -libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr) +libxlMakeNetworkDiskSrc(virConnectPtr conn, + virStorageSourcePtr src, + char **srcstr) { - virConnectPtr conn = NULL; char *secret = NULL; char *username = NULL; int ret = -1; @@ -1083,9 +1084,6 @@ libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr) const char *protocol = virStorageNetProtocolTypeToString(src->protocol); username = src->auth->username; - if (!(conn = virConnectOpen("xen:///system"))) - goto cleanup; - if (!(secret = libxlGetSecretString(conn, protocol, true, @@ -1101,12 +1099,13 @@ libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr) cleanup: VIR_FREE(secret); - virObjectUnref(conn); return ret; } int -libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) +libxlMakeDisk(virConnectPtr conn, + virDomainDiskDefPtr l_disk, + libxl_device_disk *x_disk) { const char *driver; int format; @@ -1115,7 +1114,7 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) libxl_device_disk_init(x_disk); if (actual_type == VIR_STORAGE_TYPE_NETWORK) { - if (libxlMakeNetworkDiskSrc(l_disk->src, &x_disk->pdev_path) < 0) + if (libxlMakeNetworkDiskSrc(conn, l_disk->src, &x_disk->pdev_path) < 0) return -1; } else { if (VIR_STRDUP(x_disk->pdev_path, virDomainDiskGetSource(l_disk)) < 0) @@ -1252,7 +1251,9 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) } static int -libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config) +libxlMakeDiskList(virConnectPtr conn, + virDomainDefPtr def, + libxl_domain_config *d_config) { virDomainDiskDefPtr *l_disks = def->disks; int ndisks = def->ndisks; @@ -1263,7 +1264,7 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config) return -1; for (i = 0; i < ndisks; i++) { - if (libxlMakeDisk(l_disks[i], &x_disks[i]) < 0) + if (libxlMakeDisk(conn, l_disks[i], &x_disks[i]) < 0) goto error; } @@ -1280,7 +1281,8 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config *d_config) } int -libxlMakeNic(virDomainDefPtr def, +libxlMakeNic(virConnectPtr conn, + virDomainDefPtr def, virDomainNetDefPtr l_nic, libxl_device_nic *x_nic) { @@ -1340,16 +1342,10 @@ libxlMakeNic(virDomainDefPtr def, bool fail = false; char *brname = NULL; virNetworkPtr network; - virConnectPtr conn; - - if (!(conn = virConnectOpen("xen:///system"))) - return -1; if (!(network = - virNetworkLookupByName(conn, l_nic->data.network.name))) { - virObjectUnref(conn); + virNetworkLookupByName(conn, l_nic->data.network.name))) return -1; - } if (l_nic->nips > 0) { x_nic->ip = virSocketAddrFormat(&l_nic->ips[0]->address); @@ -1367,7 +1363,6 @@ libxlMakeNic(virDomainDefPtr def, VIR_FREE(brname); virObjectUnref(network); - virObjectUnref(conn); if (fail) return -1; break; @@ -1442,7 +1437,9 @@ libxlMakeNic(virDomainDefPtr def, } static int -libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) +libxlMakeNicList(virConnectPtr conn, + virDomainDefPtr def, + libxl_domain_config *d_config) { virDomainNetDefPtr *l_nics = def->nets; size_t nnics = def->nnets; @@ -1456,7 +1453,7 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) if (l_nics[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) continue; - if (libxlMakeNic(def, l_nics[i], &x_nics[nvnics])) + if (libxlMakeNic(conn, def, l_nics[i], &x_nics[nvnics])) goto error; /* * The devid (at least right now) will not get initialized by @@ -2118,28 +2115,34 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, libxl_ctx *ctx, libxl_domain_config *d_config) { + virConnectPtr conn = NULL; + int ret = -1; + libxl_domain_config_init(d_config); - if (libxlMakeDomCreateInfo(ctx, def, &d_config->c_info) < 0) + if (!(conn = virConnectOpen("xen:///system"))) return -1; + if (libxlMakeDomCreateInfo(ctx, def, &d_config->c_info) < 0) + goto cleanup; + if (libxlMakeDomBuildInfo(def, ctx, d_config) < 0) - return -1; + goto cleanup; - if (libxlMakeDiskList(def, d_config) < 0) - return -1; + if (libxlMakeDiskList(conn, def, d_config) < 0) + goto cleanup; - if (libxlMakeNicList(def, d_config) < 0) - return -1; + if (libxlMakeNicList(conn, def, d_config) < 0) + goto cleanup; if (libxlMakeVfbList(graphicsports, def, d_config) < 0) - return -1; + goto cleanup; if (libxlMakeBuildInfoVfb(graphicsports, def, d_config) < 0) - return -1; + goto cleanup; if (libxlMakePCIList(def, d_config) < 0) - return -1; + goto cleanup; /* * Now that any potential VFBs are defined, update the build info with @@ -2147,13 +2150,17 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, * so but as it does not right now, better be explicit. */ if (libxlMakeVideo(def, d_config) < 0) - return -1; + goto cleanup; d_config->on_reboot = libxlActionFromVirLifecycle(def->onReboot); d_config->on_poweroff = libxlActionFromVirLifecycle(def->onPoweroff); d_config->on_crash = libxlActionFromVirLifecycleCrash(def->onCrash); - return 0; + ret = 0; + + cleanup: + virObjectUnref(conn); + return ret; } virDomainXMLOptionPtr diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h index 3c0eafb..94fb866 100644 --- a/src/libxl/libxl_conf.h +++ b/src/libxl/libxl_conf.h @@ -192,9 +192,12 @@ int libxlDomainGetEmulatorType(const virDomainDef *def); int -libxlMakeDisk(virDomainDiskDefPtr l_dev, libxl_device_disk *x_dev); +libxlMakeDisk(virConnectPtr conn, + virDomainDiskDefPtr l_dev, + libxl_device_disk *x_dev); int -libxlMakeNic(virDomainDefPtr def, +libxlMakeNic(virConnectPtr conn, + virDomainDefPtr def, virDomainNetDefPtr l_nic, libxl_device_nic *x_nic); int diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 404016e..531e2d6 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2912,7 +2912,9 @@ libxlDomainUndefine(virDomainPtr dom) } static int -libxlDomainChangeEjectableMedia(virDomainObjPtr vm, virDomainDiskDefPtr disk) +libxlDomainChangeEjectableMedia(virConnectPtr conn, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) { libxlDriverConfigPtr cfg = libxlDriverConfigGet(libxl_driver); virDomainDiskDefPtr origdisk = NULL; @@ -2942,7 +2944,7 @@ libxlDomainChangeEjectableMedia(virDomainObjPtr vm, virDomainDiskDefPtr disk) return -1; } - if (libxlMakeDisk(disk, &x_disk) < 0) + if (libxlMakeDisk(conn, disk, &x_disk) < 0) goto cleanup; if ((ret = libxl_cdrom_insert(cfg->ctx, vm->def->id, &x_disk, NULL)) < 0) { @@ -2966,7 +2968,9 @@ libxlDomainChangeEjectableMedia(virDomainObjPtr vm, virDomainDiskDefPtr disk) } static int -libxlDomainAttachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) +libxlDomainAttachDeviceDiskLive(virConnectPtr conn, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev) { libxlDriverConfigPtr cfg = libxlDriverConfigGet(libxl_driver); virDomainDiskDefPtr l_disk = dev->data.disk; @@ -2975,7 +2979,7 @@ libxlDomainAttachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) switch (l_disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: - ret = libxlDomainChangeEjectableMedia(vm, l_disk); + ret = libxlDomainChangeEjectableMedia(conn, vm, l_disk); break; case VIR_DOMAIN_DISK_DEVICE_DISK: if (l_disk->bus == VIR_DOMAIN_DISK_BUS_XEN) { @@ -2994,7 +2998,7 @@ libxlDomainAttachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) goto cleanup; - if (libxlMakeDisk(l_disk, &x_disk) < 0) + if (libxlMakeDisk(conn, l_disk, &x_disk) < 0) goto cleanup; if (virDomainLockDiskAttach(libxl_driver->lockManager, @@ -3119,7 +3123,9 @@ libxlDomainAttachHostDevice(libxlDriverPrivatePtr driver, } static int -libxlDomainDetachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) +libxlDomainDetachDeviceDiskLive(virConnectPtr conn, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev) { libxlDriverConfigPtr cfg = libxlDriverConfigGet(libxl_driver); virDomainDiskDefPtr l_disk = NULL; @@ -3141,7 +3147,7 @@ libxlDomainDetachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) l_disk = vm->def->disks[idx]; - if (libxlMakeDisk(l_disk, &x_disk) < 0) + if (libxlMakeDisk(conn, l_disk, &x_disk) < 0) goto cleanup; if ((ret = libxl_device_disk_remove(cfg->ctx, vm->def->id, @@ -3180,6 +3186,7 @@ libxlDomainDetachDeviceDiskLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) static int libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, + virConnectPtr conn, virDomainObjPtr vm, virDomainNetDefPtr net) { @@ -3221,7 +3228,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, } libxl_device_nic_init(&nic); - if (libxlMakeNic(vm->def, net, &nic) < 0) + if (libxlMakeNic(conn, vm->def, net, &nic) < 0) goto cleanup; if (libxl_device_nic_add(cfg->ctx, vm->def->id, &nic, 0)) { @@ -3243,6 +3250,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, static int libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver, + virConnectPtr conn, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { @@ -3250,13 +3258,13 @@ libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver, switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: - ret = libxlDomainAttachDeviceDiskLive(vm, dev); + ret = libxlDomainAttachDeviceDiskLive(conn, vm, dev); if (!ret) dev->data.disk = NULL; break; case VIR_DOMAIN_DEVICE_NET: - ret = libxlDomainAttachNetDevice(driver, vm, + ret = libxlDomainAttachNetDevice(driver, conn, vm, dev->data.net); if (!ret) dev->data.net = NULL; @@ -3514,6 +3522,7 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver, static int libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver, + virConnectPtr conn, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { @@ -3521,7 +3530,7 @@ libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver, switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: - ret = libxlDomainDetachDeviceDiskLive(vm, dev); + ret = libxlDomainDetachDeviceDiskLive(conn, vm, dev); break; case VIR_DOMAIN_DEVICE_NET: @@ -3595,7 +3604,9 @@ libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) } static int -libxlDomainUpdateDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) +libxlDomainUpdateDeviceLive(virConnectPtr conn, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev) { virDomainDiskDefPtr disk; int ret = -1; @@ -3605,7 +3616,7 @@ libxlDomainUpdateDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr dev) disk = dev->data.disk; switch (disk->device) { case VIR_DOMAIN_DISK_DEVICE_CDROM: - ret = libxlDomainChangeEjectableMedia(vm, disk); + ret = libxlDomainChangeEjectableMedia(conn, vm, disk); if (ret == 0) dev->data.disk = NULL; break; @@ -3733,7 +3744,7 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto endjob; - if (libxlDomainAttachDeviceLive(driver, vm, dev) < 0) + if (libxlDomainAttachDeviceLive(driver, dom->conn, vm, dev) < 0) goto endjob; /* @@ -3841,7 +3852,7 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto endjob; - if (libxlDomainDetachDeviceLive(driver, vm, dev) < 0) + if (libxlDomainDetachDeviceLive(driver, dom->conn, vm, dev) < 0) goto endjob; /* @@ -3948,7 +3959,7 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto cleanup; - if ((ret = libxlDomainUpdateDeviceLive(vm, dev)) < 0) + if ((ret = libxlDomainUpdateDeviceLive(dom->conn, vm, dev)) < 0) goto cleanup; /* -- 2.6.1