On 02/09/2018 12:24 PM, Daniel P. Berrangé wrote:
> Rather than expecting callers to pass a virConnectPtr into the
> virDomainDiskTranslateSourcePool() method, just acquire a connection
> to the storage driver when needed.
> 
> Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
> ---
>  src/conf/domain_conf.c  | 10 +++++++---
>  src/conf/domain_conf.h  |  3 +--
>  src/qemu/qemu_conf.c    |  3 +--
>  src/qemu/qemu_conf.h    |  3 +--
>  src/qemu/qemu_driver.c  | 39 ++++++++++++++++-----------------------
>  src/qemu/qemu_hotplug.c |  2 +-
>  src/qemu/qemu_process.c |  4 ++--
>  7 files changed, 29 insertions(+), 35 deletions(-)
> 

qemuxml2argvtest fails with this patch (turned on VIR_TEST_DEBUG=1)


171) QEMU XML-2-ARGV disk-source-pool
... libvirt: XML-RPC error : Failed to connect socket to
'/run/user/1000/libvirt/libvirt-sock': No such file or directory
libvirt: XML-RPC error : Failed to connect socket to
'/run/user/1000/libvirt/libvirt-sock': No such file or directory
FAILED
172) QEMU XML-2-ARGV disk-source-pool-mode
... libvirt: XML-RPC error : Failed to connect socket to
'/run/user/1000/libvirt/libvirt-sock': No such file or directory
libvirt: XML-RPC error : Failed to connect socket to
'/run/user/1000/libvirt/libvirt-sock': No such file or directory
FAILED
...
428) QEMU XML-2-ARGV luks-disks-source
... libvirt: XML-RPC error : Failed to connect socket to
'/run/user/1000/libvirt/libvirt-sock': No such file or directory
libvirt: XML-RPC error : Failed to connect socket to
'/run/user/1000/libvirt/libvirt-sock': No such file or directory
FAILED

Turning on LIBVIRT_DEBUG=1 finds:

2018-02-13 17:16:05.206+0000: 24834: debug : virConnectOpenInternal:1033
: Split "storage:///session" to URI components:
  scheme storage
  server <null>
  user <null>
  port -1
  path /session

It seems the test driver will need some updating...  I wonder if perhaps
these were missed because you had disabled qemuxml2argvtest for a bit,
but now that it's back...

John

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 4f50547580..613e34f8c4 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -29068,9 +29068,9 @@ 
> virDomainDiskTranslateSourcePoolAuth(virDomainDiskDefPtr def,
>  
>  
>  int
> -virDomainDiskTranslateSourcePool(virConnectPtr conn,
> -                                 virDomainDiskDefPtr def)
> +virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def)
>  {
> +    virConnectPtr conn = NULL;
>      virStoragePoolDefPtr pooldef = NULL;
>      virStoragePoolPtr pool = NULL;
>      virStorageVolPtr vol = NULL;
> @@ -29084,9 +29084,12 @@ virDomainDiskTranslateSourcePool(virConnectPtr conn,
>      if (!def->src->srcpool)
>          return 0;
>  
> -    if (!(pool = virStoragePoolLookupByName(conn, def->src->srcpool->pool)))
> +    if (!(conn = virGetConnectStorage()))
>          return -1;
>  
> +    if (!(pool = virStoragePoolLookupByName(conn, def->src->srcpool->pool)))
> +        goto cleanup;
> +
>      if (virStoragePoolIsActive(pool) != 1) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                         _("storage pool '%s' containing volume '%s' "
> @@ -29230,6 +29233,7 @@ virDomainDiskTranslateSourcePool(virConnectPtr conn,
>  
>      ret = 0;
>   cleanup:
> +    virObjectUnref(conn);
>      virObjectUnref(pool);
>      virObjectUnref(vol);
>      VIR_FREE(poolxml);
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 7b450ce8f1..8be08bc9b3 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -3514,8 +3514,7 @@ virDomainNetResolveActualType(virDomainNetDefPtr iface)
>      ATTRIBUTE_NONNULL(1);
>  
>  
> -int virDomainDiskTranslateSourcePool(virConnectPtr conn,
> -                                     virDomainDiskDefPtr def);
> +int virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def);
>  
>  
>  #endif /* __DOMAIN_CONF_H */
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 2fa96431fa..b1ee36efea 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1635,8 +1635,7 @@ int qemuDriverAllocateID(virQEMUDriverPtr driver)
>  
>  
>  int
> -qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn ATTRIBUTE_UNUSED,
> -                                    virDomainSnapshotDiskDefPtr def)
> +qemuTranslateSnapshotDiskSourcePool(virDomainSnapshotDiskDefPtr def)
>  {
>      if (def->src->type != VIR_STORAGE_TYPE_VOLUME)
>          return 0;
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index 3f38a76c26..947e52dfe2 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -352,8 +352,7 @@ int qemuSetUnprivSGIO(virDomainDeviceDefPtr dev);
>  int qemuDriverAllocateID(virQEMUDriverPtr driver);
>  virDomainXMLOptionPtr virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver);
>  
> -int qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn,
> -                                        virDomainSnapshotDiskDefPtr def);
> +int qemuTranslateSnapshotDiskSourcePool(virDomainSnapshotDiskDefPtr def);
>  
>  char * qemuGetBaseHugepagePath(virHugeTLBFSPtr hugepage);
>  char * qemuGetDomainHugepagePath(const virDomainDef *def,
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 0aa0f05d3c..6eec6d282c 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7871,8 +7871,7 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
>  }
>  
>  static int
> -qemuDomainChangeDiskLive(virConnectPtr conn,
> -                         virDomainObjPtr vm,
> +qemuDomainChangeDiskLive(virDomainObjPtr vm,
>                           virDomainDeviceDefPtr dev,
>                           virQEMUDriverPtr driver,
>                           bool force)
> @@ -7881,7 +7880,7 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
>      virDomainDiskDefPtr orig_disk = NULL;
>      int ret = -1;
>  
> -    if (virDomainDiskTranslateSourcePool(conn, disk) < 0)
> +    if (virDomainDiskTranslateSourcePool(disk) < 0)
>          goto cleanup;
>  
>      if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
> @@ -7932,8 +7931,7 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
>  }
>  
>  static int
> -qemuDomainUpdateDeviceLive(virConnectPtr conn,
> -                           virDomainObjPtr vm,
> +qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
>                             virDomainDeviceDefPtr dev,
>                             virDomainPtr dom,
>                             bool force)
> @@ -7944,7 +7942,7 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn,
>      switch ((virDomainDeviceType) dev->type) {
>      case VIR_DOMAIN_DEVICE_DISK:
>          qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL);
> -        ret = qemuDomainChangeDiskLive(conn, vm, dev, driver, force);
> +        ret = qemuDomainChangeDiskLive(vm, dev, driver, force);
>          break;
>      case VIR_DOMAIN_DEVICE_GRAPHICS:
>          ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics);
> @@ -7986,7 +7984,6 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn,
>  static int
>  qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
>                               virDomainDeviceDefPtr dev,
> -                             virConnectPtr conn,
>                               virCapsPtr caps,
>                               unsigned int parse_flags,
>                               virDomainXMLOptionPtr xmlopt)
> @@ -8008,7 +8005,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
>                             _("target %s already exists"), disk->dst);
>              return -1;
>          }
> -        if (virDomainDiskTranslateSourcePool(conn, disk) < 0)
> +        if (virDomainDiskTranslateSourcePool(disk) < 0)
>              return -1;
>          if (qemuCheckDiskConfig(disk, NULL) < 0)
>              return -1;
> @@ -8494,7 +8491,7 @@ qemuDomainAttachDeviceLiveAndConfig(virConnectPtr conn,
>  
>          if (virDomainDefCompatibleDevice(vmdef, dev) < 0)
>              goto cleanup;
> -        if ((ret = qemuDomainAttachDeviceConfig(vmdef, dev, conn, caps,
> +        if ((ret = qemuDomainAttachDeviceConfig(vmdef, dev, caps,
>                                                  parse_flags,
>                                                  driver->xmlopt)) < 0)
>              goto cleanup;
> @@ -8654,7 +8651,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
>          if (virDomainDefCompatibleDevice(vm->def, dev_copy) < 0)
>              goto endjob;
>  
> -        if ((ret = qemuDomainUpdateDeviceLive(dom->conn, vm, dev_copy, dom, 
> force)) < 0)
> +        if ((ret = qemuDomainUpdateDeviceLive(vm, dev_copy, dom, force)) < 0)
>              goto endjob;
>          /*
>           * update domain status forcibly because the domain status may be
> @@ -14229,8 +14226,7 @@ 
> qemuDomainSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDefPtr 
> snapdisk
>  
>  
>  static int
> -qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn,
> -                                      virDomainDiskDefPtr disk,
> +qemuDomainSnapshotPrepareDiskExternal(virDomainDiskDefPtr disk,
>                                        virDomainSnapshotDiskDefPtr snapdisk,
>                                        bool active,
>                                        bool reuse)
> @@ -14238,11 +14234,11 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr 
> conn,
>      int ret = -1;
>      struct stat st;
>  
> -    if (qemuTranslateSnapshotDiskSourcePool(conn, snapdisk) < 0)
> +    if (qemuTranslateSnapshotDiskSourcePool(snapdisk) < 0)
>          return -1;
>  
>      if (!active) {
> -        if (virDomainDiskTranslateSourcePool(conn, disk) < 0)
> +        if (virDomainDiskTranslateSourcePool(disk) < 0)
>              return -1;
>  
>          if (qemuDomainSnapshotPrepareDiskExternalInactive(snapdisk, disk) < 
> 0)
> @@ -14284,8 +14280,7 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr 
> conn,
>  
>  
>  static int
> -qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn,
> -                                      virDomainDiskDefPtr disk,
> +qemuDomainSnapshotPrepareDiskInternal(virDomainDiskDefPtr disk,
>                                        bool active)
>  {
>      int actualType;
> @@ -14294,7 +14289,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr 
> conn,
>      if (active)
>          return 0;
>  
> -    if (virDomainDiskTranslateSourcePool(conn, disk) < 0)
> +    if (virDomainDiskTranslateSourcePool(disk) < 0)
>          return -1;
>  
>      actualType = virStorageSourceGetActualType(disk->src);
> @@ -14343,8 +14338,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr 
> conn,
>  
>  
>  static int
> -qemuDomainSnapshotPrepare(virConnectPtr conn,
> -                          virDomainObjPtr vm,
> +qemuDomainSnapshotPrepare(virDomainObjPtr vm,
>                            virDomainSnapshotDefPtr def,
>                            unsigned int *flags)
>  {
> @@ -14385,7 +14379,7 @@ qemuDomainSnapshotPrepare(virConnectPtr conn,
>                  goto cleanup;
>              }
>  
> -            if (qemuDomainSnapshotPrepareDiskInternal(conn, dom_disk,
> +            if (qemuDomainSnapshotPrepareDiskInternal(dom_disk,
>                                                        active) < 0)
>                  goto cleanup;
>  
> @@ -14414,7 +14408,7 @@ qemuDomainSnapshotPrepare(virConnectPtr conn,
>                  goto cleanup;
>              }
>  
> -            if (qemuDomainSnapshotPrepareDiskExternal(conn, dom_disk, disk,
> +            if (qemuDomainSnapshotPrepareDiskExternal(dom_disk, disk,
>                                                        active, reuse) < 0)
>                  goto cleanup;
>  
> @@ -15062,7 +15056,6 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
>                              const char *xmlDesc,
>                              unsigned int flags)
>  {
> -    virConnectPtr conn = domain->conn;
>      virQEMUDriverPtr driver = domain->conn->privateData;
>      virDomainObjPtr vm = NULL;
>      char *xml = NULL;
> @@ -15241,7 +15234,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
>          }
>          if (virDomainSnapshotAlignDisks(def, align_location,
>                                          align_match) < 0 ||
> -            qemuDomainSnapshotPrepare(conn, vm, def, &flags) < 0)
> +            qemuDomainSnapshotPrepare(vm, def, &flags) < 0)
>              goto endjob;
>      }
>  
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index c7bf25eeef..a2268be576 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -713,7 +713,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
>          goto cleanup;
>      }
>  
> -    if (virDomainDiskTranslateSourcePool(conn, disk) < 0)
> +    if (virDomainDiskTranslateSourcePool(disk) < 0)
>          goto cleanup;
>  
>      if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index d0a25cecb9..586d11bba3 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5603,7 +5603,7 @@ qemuProcessPrepareDomainStorage(virConnectPtr conn,
>          size_t idx = i - 1;
>          virDomainDiskDefPtr disk = vm->def->disks[idx];
>  
> -        if (virDomainDiskTranslateSourcePool(conn, disk) < 0) {
> +        if (virDomainDiskTranslateSourcePool(disk) < 0) {
>              if (qemuDomainCheckDiskStartupPolicy(driver, vm, idx, cold_boot) 
> < 0)
>                  return -1;
>  
> @@ -7362,7 +7362,7 @@ qemuProcessReconnect(void *opaque)
>          virDomainDiskDefPtr disk = obj->def->disks[i];
>          virDomainDeviceDef dev;
>  
> -        if (virDomainDiskTranslateSourcePool(conn, disk) < 0)
> +        if (virDomainDiskTranslateSourcePool(disk) < 0)
>              goto error;
>  
>          /* backing chains need to be refreshed only if they could change */
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to