1. Patchew
  2. Libvirt
  3. View series

[PATCH v2] libxl: adjust handling of libxl_device_disk objects

Olaf Hering posted 1 patch 2 years, 11 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210520101720.14305-1-olaf@aepfle.de
src/libxl/libxl_conf.c   | 20 +++++---------------
src/libxl/libxl_driver.c |  6 ++++++
2 files changed, 11 insertions(+), 15 deletions(-)
[PATCH v2] libxl: adjust handling of libxl_device_disk objects
Posted by Olaf Hering 2 years, 11 months ago 
libxl objects are supposed to be initialized and disposed.
Correct the usage of libxl_device_disk objects which are allocated on
the stack. Initialize each one prior usage, and dispose them once done.

Adjust libxlMakeDisk to use an already initialized object, it is owned
by the caller.

Adjust libxlMakeDiskList to initialize the list of objects, before they
are filled by libxlMakeDisk. In case of error, the objects are disposed
by libxl_domain_config_dispose.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 src/libxl/libxl_conf.c   | 20 +++++---------------
 src/libxl/libxl_driver.c |  6 ++++++
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 3149ee3b4a..f29a28a841 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1114,8 +1114,6 @@ libxlMakeDisk(virDomainDiskDef *l_disk, libxl_device_disk *x_disk)
     int format = virDomainDiskGetFormat(l_disk);
     int actual_type = virStorageSourceGetActualType(l_disk->src);
 
-    libxl_device_disk_init(x_disk);
-
     if (actual_type == VIR_STORAGE_TYPE_NETWORK) {
         if (STRNEQ_NULLABLE(driver, "qemu")) {
             virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -1265,26 +1263,18 @@ libxlMakeDiskList(virDomainDef *def, libxl_domain_config *d_config)
 {
     virDomainDiskDef **l_disks = def->disks;
     int ndisks = def->ndisks;
-    libxl_device_disk *x_disks;
     size_t i;
 
-    x_disks = g_new0(libxl_device_disk, ndisks);
+    d_config->disks = g_new0(libxl_device_disk, ndisks);
+    d_config->num_disks = ndisks;
 
     for (i = 0; i < ndisks; i++) {
-        if (libxlMakeDisk(l_disks[i], &x_disks[i]) < 0)
-            goto error;
+        libxl_device_disk_init(&d_config->disks[i]);
+        if (libxlMakeDisk(l_disks[i], &d_config->disks[i]) < 0)
+            return -1;
     }
 
-    d_config->disks = x_disks;
-    d_config->num_disks = ndisks;
-
     return 0;
-
- error:
-    for (i = 0; i < ndisks; i++)
-        libxl_device_disk_dispose(&x_disks[i]);
-    VIR_FREE(x_disks);
-    return -1;
 }
 
 /*
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index d54cd41785..2b844bb3b5 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2978,6 +2978,7 @@ libxlDomainChangeEjectableMedia(virDomainObj *vm, virDomainDiskDef *disk)
     size_t i;
     int ret = -1;
 
+    libxl_device_disk_init(&x_disk);
     for (i = 0; i < vm->def->ndisks; i++) {
         if (vm->def->disks[i]->bus == disk->bus &&
             STREQ(vm->def->disks[i]->dst, disk->dst)) {
@@ -3018,6 +3019,7 @@ libxlDomainChangeEjectableMedia(virDomainObj *vm, virDomainDiskDef *disk)
     ret = 0;
 
  cleanup:
+    libxl_device_disk_dispose(&x_disk);
     virObjectUnref(cfg);
     return ret;
 }
@@ -3030,6 +3032,7 @@ libxlDomainAttachDeviceDiskLive(virDomainObj *vm, virDomainDeviceDef *dev)
     libxl_device_disk x_disk;
     int ret = -1;
 
+    libxl_device_disk_init(&x_disk);
     switch (l_disk->device)  {
         case VIR_DOMAIN_DISK_DEVICE_CDROM:
             ret = libxlDomainChangeEjectableMedia(vm, l_disk);
@@ -3091,6 +3094,7 @@ libxlDomainAttachDeviceDiskLive(virDomainObj *vm, virDomainDeviceDef *dev)
     }
 
  cleanup:
+    libxl_device_disk_dispose(&x_disk);
     virObjectUnref(cfg);
     return ret;
 }
@@ -3329,6 +3333,7 @@ libxlDomainDetachDeviceDiskLive(virDomainObj *vm, virDomainDeviceDef *dev)
     int idx;
     int ret = -1;
 
+    libxl_device_disk_init(&x_disk);
     switch (dev->data.disk->device)  {
         case VIR_DOMAIN_DISK_DEVICE_DISK:
             if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_XEN) {
@@ -3380,6 +3385,7 @@ libxlDomainDetachDeviceDiskLive(virDomainObj *vm, virDomainDeviceDef *dev)
     }
 
  cleanup:
+    libxl_device_disk_dispose(&x_disk);
     virObjectUnref(cfg);
     return ret;
 }
Re: [PATCH v2] libxl: adjust handling of libxl_device_disk objects
Posted by Jim Fehlig 2 years, 11 months ago 
On 5/20/21 4:17 AM, Olaf Hering wrote:
> libxl objects are supposed to be initialized and disposed.
> Correct the usage of libxl_device_disk objects which are allocated on
> the stack. Initialize each one prior usage, and dispose them once done.
> 
> Adjust libxlMakeDisk to use an already initialized object, it is owned
> by the caller.
> 
> Adjust libxlMakeDiskList to initialize the list of objects, before they
> are filled by libxlMakeDisk. In case of error, the objects are disposed
> by libxl_domain_config_dispose.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>   src/libxl/libxl_conf.c   | 20 +++++---------------
>   src/libxl/libxl_driver.c |  6 ++++++
>   2 files changed, 11 insertions(+), 15 deletions(-)

Reviewed-by: Jim Fehlig <jfehlig@suse.com

> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 3149ee3b4a..f29a28a841 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1114,8 +1114,6 @@ libxlMakeDisk(virDomainDiskDef *l_disk, libxl_device_disk *x_disk)
>       int format = virDomainDiskGetFormat(l_disk);
>       int actual_type = virStorageSourceGetActualType(l_disk->src);
>   
> -    libxl_device_disk_init(x_disk);
> -
>       if (actual_type == VIR_STORAGE_TYPE_NETWORK) {
>           if (STRNEQ_NULLABLE(driver, "qemu")) {
>               virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> @@ -1265,26 +1263,18 @@ libxlMakeDiskList(virDomainDef *def, libxl_domain_config *d_config)
>   {
>       virDomainDiskDef **l_disks = def->disks;
>       int ndisks = def->ndisks;
> -    libxl_device_disk *x_disks;
>       size_t i;
>   
> -    x_disks = g_new0(libxl_device_disk, ndisks);
> +    d_config->disks = g_new0(libxl_device_disk, ndisks);
> +    d_config->num_disks = ndisks;
>   
>       for (i = 0; i < ndisks; i++) {
> -        if (libxlMakeDisk(l_disks[i], &x_disks[i]) < 0)
> -            goto error;
> +        libxl_device_disk_init(&d_config->disks[i]);
> +        if (libxlMakeDisk(l_disks[i], &d_config->disks[i]) < 0)
> +            return -1;
>       }
>   
> -    d_config->disks = x_disks;
> -    d_config->num_disks = ndisks;
> -
>       return 0;
> -
> - error:
> -    for (i = 0; i < ndisks; i++)
> -        libxl_device_disk_dispose(&x_disks[i]);
> -    VIR_FREE(x_disks);
> -    return -1;
>   }
>   
>   /*
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index d54cd41785..2b844bb3b5 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -2978,6 +2978,7 @@ libxlDomainChangeEjectableMedia(virDomainObj *vm, virDomainDiskDef *disk)
>       size_t i;
>       int ret = -1;
>   
> +    libxl_device_disk_init(&x_disk);
>       for (i = 0; i < vm->def->ndisks; i++) {
>           if (vm->def->disks[i]->bus == disk->bus &&
>               STREQ(vm->def->disks[i]->dst, disk->dst)) {
> @@ -3018,6 +3019,7 @@ libxlDomainChangeEjectableMedia(virDomainObj *vm, virDomainDiskDef *disk)
>       ret = 0;
>   
>    cleanup:
> +    libxl_device_disk_dispose(&x_disk);
>       virObjectUnref(cfg);
>       return ret;
>   }
> @@ -3030,6 +3032,7 @@ libxlDomainAttachDeviceDiskLive(virDomainObj *vm, virDomainDeviceDef *dev)
>       libxl_device_disk x_disk;
>       int ret = -1;
>   
> +    libxl_device_disk_init(&x_disk);
>       switch (l_disk->device)  {
>           case VIR_DOMAIN_DISK_DEVICE_CDROM:
>               ret = libxlDomainChangeEjectableMedia(vm, l_disk);
> @@ -3091,6 +3094,7 @@ libxlDomainAttachDeviceDiskLive(virDomainObj *vm, virDomainDeviceDef *dev)
>       }
>   
>    cleanup:
> +    libxl_device_disk_dispose(&x_disk);
>       virObjectUnref(cfg);
>       return ret;
>   }
> @@ -3329,6 +3333,7 @@ libxlDomainDetachDeviceDiskLive(virDomainObj *vm, virDomainDeviceDef *dev)
>       int idx;
>       int ret = -1;
>   
> +    libxl_device_disk_init(&x_disk);
>       switch (dev->data.disk->device)  {
>           case VIR_DOMAIN_DISK_DEVICE_DISK:
>               if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_XEN) {
> @@ -3380,6 +3385,7 @@ libxlDomainDetachDeviceDiskLive(virDomainObj *vm, virDomainDeviceDef *dev)
>       }
>   
>    cleanup:
> +    libxl_device_disk_dispose(&x_disk);
>       virObjectUnref(cfg);
>       return ret;
>   }
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.