Re: [PATCH 1/2] qemu: Check for existing file in namespace

On 7/7/21 12:46 PM, Kristina Hanicova wrote: > Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com&gt; The commit message is a bit sparse. Can you describe the intent? > --- > src/qemu/qemu_namespace.c | 24 ++++++++++++++---------- > src/util/virprocess.c | 6 ++++-- > 2 files changed, 18 insertions(+), 12 deletions(-) > > diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c > index 98495e8ef8..154968acbd 100644 > --- a/src/qemu/qemu_namespace.c > +++ b/src/qemu/qemu_namespace.c > @@ -929,6 +929,10 @@ qemuNamespaceMknodOne(qemuNamespaceMknodItem *data) > bool isDev = S_ISCHR(data->sb.st_mode) || S_ISBLK(data->sb.st_mode); > bool isReg = S_ISREG(data->sb.st_mode) || S_ISFIFO(data->sb.st_mode) || S_ISSOCK(data->sb.st_mode); > bool isDir = S_ISDIR(data->sb.st_mode); > + int file_exists = 0; Oh, the rest of the function uses camelCase ;-) However, what if this was a bool ..

> + > + if (virFileExists(data->file)) > + file_exists = 1; > > if (virFileMakeParentPath(data->file) < 0) { > virReportSystemError(errno, > @@ -1039,7 +1043,7 @@ qemuNamespaceMknodOne(qemuNamespaceMknodItem *data) > virFileMoveMount(data->target, data->file) < 0) > goto cleanup; > > - ret = 0; > + ret = file_exists; > cleanup: > if (ret < 0 && delDevice) { > if (isDir) > @@ -1069,15 +1073,19 @@ qemuNamespaceMknodHelper(pid_t pid G_GNUC_UNUSED, > qemuNamespaceMknodData *data = opaque; > size_t i; > int ret = -1; > + int file_existed = 0; > > qemuSecurityPostFork(data->driver->securityManager); > > for (i = 0; i < data->nitems; i++) { > - if (qemuNamespaceMknodOne(&data->items[i]) < 0) > + int rc = 0; > + > + if ((rc = qemuNamespaceMknodOne(&data->items[i])) < 0) > goto cleanup;

> + file_existed = file_existed || rc;

> } > > - ret = 0; > + ret = file_existed; And then this was: ret = file_existed ? 1 : 0; IMO it's easier to track return value.

> cleanup: > qemuNamespaceMknodDataClear(data); > return ret;