On Sat, Sep 07, 2024 at 17:15:31 +0300, Nikolai Barybin via Devel wrote:
> Signed-off-by: Nikolai Barybin <[email protected]>
> ---
> src/qemu/qemu_cgroup.c | 4 ++++
> src/qemu/qemu_namespace.c | 5 +++++
> 2 files changed, 9 insertions(+)
Firstly, that applies everywhere, you must not use the 'path' member of
virStorageSource as a local file path unless you check that it's a local
source by calling virStorageSourceIsLocalStorage. Network storage
backends (e.g. NBD) use that field for the relative path/export name
so it's not a local path then.
>
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index 23b7e6b4e8..a6d3c16e50 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -223,6 +223,10 @@ qemuSetupImageCgroupInternal(virDomainObj *vm,
> qemuSetupImagePathCgroup(vm, QEMU_DEVICE_MAPPER_CONTROL_PATH, false)
> < 0)
> return -1;
>
> + if (src->dataFileStore &&
> + qemuSetupImagePathCgroup(vm, src->dataFileStore->path, readonly) < 0)
> + return -1;
> +
I don't think this is correct. You should call
'qemuSetupImageCgroupInternal' on the data file virStorageSource struct
instead, rather than setup it partially inside.
> return qemuSetupImagePathCgroup(vm, path, readonly);
> }
>
> diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
> index bbe3d5a1f7..afeec63a4f 100644
> --- a/src/qemu/qemu_namespace.c
> +++ b/src/qemu/qemu_namespace.c
> @@ -290,6 +290,11 @@ qemuDomainSetupDisk(virStorageSource *src,
>
> if (targetPaths)
> *paths = g_slist_concat(g_slist_reverse(targetPaths),
> *paths);
> +
> + if (next->dataFileStore) {
> + g_autofree char *dataFilePath =
> g_strdup(next->dataFileStore->path);
> + *paths = g_slist_prepend(*paths,
> g_steal_pointer(&dataFilePath));
> + }
And this isn't correct either because this code will be skipped if the
QCOW2 bit (the metadata file) is not stored locally.
> }
>
> *paths = g_slist_prepend(*paths, g_steal_pointer(&tmpPath));
> --
> 2.43.5
>
