[libvirt] [PATCHv2 3/7] Introduce qemuDomainNamespaceMknodPaths

Ján Tomko posted 7 patches 7 years, 5 months ago
[libvirt] [PATCHv2 3/7] Introduce qemuDomainNamespaceMknodPaths
Posted by Ján Tomko 7 years, 5 months ago
Separate the logic of creating devices from their gathering.

Use this new function in qemuDomainNamespaceSetupHostdev and
qemuDomainNamespaceSetupDisk.
---
 src/qemu/qemu_domain.c | 74 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 44 insertions(+), 30 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d7f4b3fd7..f1c664248 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9925,16 +9925,18 @@ qemuDomainDetachDeviceUnlink(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
 }
 
 
-int
-qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver,
-                             virDomainObjPtr vm,
-                             virStorageSourcePtr src)
+static int
+qemuDomainNamespaceMknodPaths(virDomainObjPtr vm,
+                              const char **paths,
+                              size_t npaths)
 {
-    virQEMUDriverConfigPtr cfg = NULL;
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    virQEMUDriverPtr driver = priv->driver;
+    virQEMUDriverConfigPtr cfg;
     char **devMountsPath = NULL;
     size_t ndevMountsPath = 0;
-    virStorageSourcePtr next;
     int ret = -1;
+    size_t i;
 
     if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
         return 0;
@@ -9945,6 +9947,35 @@ qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver,
                                      &ndevMountsPath) < 0)
         goto cleanup;
 
+    for (i = 0; i < npaths; i++) {
+        if (qemuDomainAttachDeviceMknod(driver,
+                                        vm,
+                                        paths[i],
+                                        devMountsPath, ndevMountsPath) < 0)
+            goto cleanup;
+    }
+
+    ret = 0;
+ cleanup:
+    virStringListFreeCount(devMountsPath, ndevMountsPath);
+    virObjectUnref(cfg);
+    return ret;
+}
+
+
+int
+qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
+                             virDomainObjPtr vm,
+                             virStorageSourcePtr src)
+{
+    virStorageSourcePtr next;
+    char **paths = NULL;
+    size_t npaths;
+    int ret = -1;
+
+    if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
+        return 0;
+
     for (next = src; virStorageSourceIsBacking(next); next = next->backingStore) {
         if (virStorageSourceIsEmpty(next) ||
             !virStorageSourceIsLocalStorage(next)) {
@@ -9952,17 +9983,16 @@ qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver,
             continue;
         }
 
-        if (qemuDomainAttachDeviceMknod(driver,
-                                        vm,
-                                        next->path,
-                                        devMountsPath, ndevMountsPath) < 0)
+        if (VIR_APPEND_ELEMENT_COPY(paths, npaths, next->path) < 0)
             goto cleanup;
     }
 
+    if (qemuDomainNamespaceMknodPaths(vm, (const char **)paths, npaths) < 0)
+        return -1;
+
     ret = 0;
  cleanup:
-    virStringListFreeCount(devMountsPath, ndevMountsPath);
-    virObjectUnref(cfg);
+    VIR_FREE(paths);
     return ret;
 }
 
@@ -9983,13 +10013,10 @@ qemuDomainNamespaceTeardownDisk(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
 
 
 int
-qemuDomainNamespaceSetupHostdev(virQEMUDriverPtr driver,
+qemuDomainNamespaceSetupHostdev(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
                                 virDomainObjPtr vm,
                                 virDomainHostdevDefPtr hostdev)
 {
-    virQEMUDriverConfigPtr cfg = NULL;
-    char **devMountsPath = NULL;
-    size_t ndevMountsPath = 0;
     int ret = -1;
     char **paths = NULL;
     size_t i, npaths = 0;
@@ -10000,27 +10027,14 @@ qemuDomainNamespaceSetupHostdev(virQEMUDriverPtr driver,
     if (qemuDomainGetHostdevPath(NULL, hostdev, false, &npaths, &paths, NULL) < 0)
         goto cleanup;
 
-    cfg = virQEMUDriverGetConfig(driver);
-    if (qemuDomainGetPreservedMounts(cfg, vm,
-                                     &devMountsPath, NULL,
-                                     &ndevMountsPath) < 0)
+    if (qemuDomainNamespaceMknodPaths(vm, (const char **)paths, npaths) < 0)
         goto cleanup;
 
-    for (i = 0; i < npaths; i++) {
-        if (qemuDomainAttachDeviceMknod(driver,
-                                        vm,
-                                        paths[i],
-                                        devMountsPath, ndevMountsPath) < 0)
-        goto cleanup;
-    }
-
     ret = 0;
  cleanup:
     for (i = 0; i < npaths; i++)
         VIR_FREE(paths[i]);
     VIR_FREE(paths);
-    virStringListFreeCount(devMountsPath, ndevMountsPath);
-    virObjectUnref(cfg);
     return ret;
 }
 
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/7] Introduce qemuDomainNamespaceMknodPaths
Posted by John Ferlan 7 years, 5 months ago

On 12/01/2017 11:50 AM, Ján Tomko wrote:
> Separate the logic of creating devices from their gathering.
> 
> Use this new function in qemuDomainNamespaceSetupHostdev and
> qemuDomainNamespaceSetupDisk.
> ---
>  src/qemu/qemu_domain.c | 74 ++++++++++++++++++++++++++++++--------------------
>  1 file changed, 44 insertions(+), 30 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index d7f4b3fd7..f1c664248 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -9925,16 +9925,18 @@ qemuDomainDetachDeviceUnlink(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
>  }
>  
>  

[...]

> +
> +
> +int
> +qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
> +                             virDomainObjPtr vm,
> +                             virStorageSourcePtr src)
> +{
> +    virStorageSourcePtr next;
> +    char **paths = NULL;
> +    size_t npaths;
> +    int ret = -1;
> +
> +    if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
> +        return 0;
> +
>      for (next = src; virStorageSourceIsBacking(next); next = next->backingStore) {
>          if (virStorageSourceIsEmpty(next) ||
>              !virStorageSourceIsLocalStorage(next)) {
> @@ -9952,17 +9983,16 @@ qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver,
>              continue;
>          }
>  
> -        if (qemuDomainAttachDeviceMknod(driver,
> -                                        vm,
> -                                        next->path,
> -                                        devMountsPath, ndevMountsPath) < 0)
> +        if (VIR_APPEND_ELEMENT_COPY(paths, npaths, next->path) < 0)

Coverity has noted to me this morning that @npaths is uninitialized...

>              goto cleanup;
>      }
>  
> +    if (qemuDomainNamespaceMknodPaths(vm, (const char **)paths, npaths) < 0)
> +        return -1;
> +
>      ret = 0;
>   cleanup:
> -    virStringListFreeCount(devMountsPath, ndevMountsPath);
> -    virObjectUnref(cfg);
> +    VIR_FREE(paths);

And while not noted by Coverity, shouldn't this be a :

virStringListFree(paths);

John


>      return ret;
>  }
>  
> @@ -9983,13 +10013,10 @@ qemuDomainNamespaceTeardownDisk(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
>  
>  
[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/7] Introduce qemuDomainNamespaceMknodPaths
Posted by Ján Tomko 7 years, 5 months ago
On Thu, Dec 07, 2017 at 07:51:40AM -0500, John Ferlan wrote:
>
>
>On 12/01/2017 11:50 AM, Ján Tomko wrote:
>> Separate the logic of creating devices from their gathering.
>>
>> Use this new function in qemuDomainNamespaceSetupHostdev and
>> qemuDomainNamespaceSetupDisk.
>> ---
>>  src/qemu/qemu_domain.c | 74 ++++++++++++++++++++++++++++++--------------------
>>  1 file changed, 44 insertions(+), 30 deletions(-)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index d7f4b3fd7..f1c664248 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -9925,16 +9925,18 @@ qemuDomainDetachDeviceUnlink(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
>>  }
>>
>>
>
>[...]
>
>> +
>> +
>> +int
>> +qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
>> +                             virDomainObjPtr vm,
>> +                             virStorageSourcePtr src)
>> +{
>> +    virStorageSourcePtr next;
>> +    char **paths = NULL;
>> +    size_t npaths;
>> +    int ret = -1;
>> +
>> +    if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
>> +        return 0;
>> +
>>      for (next = src; virStorageSourceIsBacking(next); next = next->backingStore) {
>>          if (virStorageSourceIsEmpty(next) ||
>>              !virStorageSourceIsLocalStorage(next)) {
>> @@ -9952,17 +9983,16 @@ qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver,
>>              continue;
>>          }
>>
>> -        if (qemuDomainAttachDeviceMknod(driver,
>> -                                        vm,
>> -                                        next->path,
>> -                                        devMountsPath, ndevMountsPath) < 0)
>> +        if (VIR_APPEND_ELEMENT_COPY(paths, npaths, next->path) < 0)
>
>Coverity has noted to me this morning that @npaths is uninitialized...
>

Thanks, fixed.

>>              goto cleanup;
>>      }
>>
>> +    if (qemuDomainNamespaceMknodPaths(vm, (const char **)paths, npaths) < 0)
>> +        return -1;
>> +
>>      ret = 0;
>>   cleanup:
>> -    virStringListFreeCount(devMountsPath, ndevMountsPath);
>> -    virObjectUnref(cfg);
>> +    VIR_FREE(paths);
>
>And while not noted by Coverity, shouldn't this be a :
>
>virStringListFree(paths);
>

No, the paths are owned by the respective StorageSources.

Jan

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