9:46 a.m.
This is v2 of:
https://listman.redhat.com/archives/libvir-list/2021-July/msg00129.html
Changes since v1 (suggested by Michal and Jano):
* change of int variable to one-word bool
* isolate changes outside of qemu to separate patch
* rewritten assignment of return value
* slight change of documentation
* extension of use to all devices, not just input
* @created variable is set in one core function instead of setting it in
each device function separately
Kristina Hanicova (3):
qemu: Check for existing file in namespace
virprocess: Return retval of the child on success, not 0
qemu: Do not erase duplicate devices from namespace if error occurs
src/qemu/qemu_domain.c | 4 +--
src/qemu/qemu_hotplug.c | 27 +++++----------
src/qemu/qemu_namespace.c | 71 +++++++++++++++++++++++++--------------
src/qemu/qemu_namespace.h | 18 ++++++----
src/qemu/qemu_process.c | 2 +-
src/util/virprocess.c | 12 ++++---
6 files changed, 75 insertions(+), 59 deletions(-)
--
2.31.1
9:46 a.m.
New subject: [PATCH v2 1/3] qemu: Check for existing file in namespace
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/qemu/qemu_namespace.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index 98495e8ef8..eb048a2faa 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);
+ bool exists = false;
+
+ if (virFileExists(data->file))
+ exists = true;
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 = exists ? 1 : 0;
cleanup:
if (ret < 0 && delDevice) {
if (isDir)
@@ -1069,15 +1073,21 @@ qemuNamespaceMknodHelper(pid_t pid G_GNUC_UNUSED,
qemuNamespaceMknodData *data = opaque;
size_t i;
int ret = -1;
+ bool exists = false;
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;
+
+ if (rc > 0)
+ exists = true;
}
- ret = 0;
+ ret = exists ? 1 : 0;
cleanup:
qemuNamespaceMknodDataClear(data);
return ret;
--
2.31.1
5:05 a.m.
New subject: [PATCH v2 1/3] qemu: Check for existing file in namespace
On 7/14/21 4:46 PM, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/qemu/qemu_namespace.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index 98495e8ef8..eb048a2faa 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);
+ bool exists = false;
+
+ if (virFileExists(data->file))
+ exists = true;
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 = exists ? 1 : 0;
This ternary operator feels redundant. Plain 'ret = exists' does the
same thing.
cleanup:
if (ret < 0 && delDevice) {
if (isDir)
@@ -1069,15 +1073,21 @@ qemuNamespaceMknodHelper(pid_t pid G_GNUC_UNUSED,
qemuNamespaceMknodData *data = opaque;
size_t i;
int ret = -1;
+ bool exists = false;
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;
+
+ if (rc > 0)
+ exists = true;
}
- ret = 0;
+ ret = exists ? 1 : 0;
Same here.
cleanup:
qemuNamespaceMknodDataClear(data);
return ret;
Michal
9:46 a.m.
New subject: [PATCH v2 2/3] virprocess: Return retval of the child on success, not 0
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/util/virprocess.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 01d5d01d02..67dd599b3e 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -1244,7 +1244,7 @@ static int virProcessNamespaceHelper(pid_t pid G_GNUC_UNUSED,
/* Run cb(opaque) in the mount namespace of pid. Return -1 with error
* message raised if we fail to run the child, if the child dies from
* a signal, or if the child has status EXIT_CANCELED; otherwise return
- * the exit status of the child. The callback will be run in a child
+ * value is the retval of the callback. The callback will be run in a child
* process so must be careful to only use async signal safe functions.
*/
int
@@ -1298,7 +1298,9 @@ virProcessRunInForkHelper(int errfd,
virProcessForkCallback cb,
void *opaque)
{
- if (cb(ppid, opaque) < 0) {
+ int ret = 0;
+
+ if ((ret = cb(ppid, opaque)) < 0) {
virErrorPtr err = virGetLastError();
if (err) {
@@ -1323,7 +1325,7 @@ virProcessRunInForkHelper(int errfd,
return -1;
}
- return 0;
+ return ret;
}
@@ -1344,7 +1346,7 @@ virProcessRunInForkHelper(int errfd,
* On return, the returned value is either -1 with error message
* reported if something went bad in the parent, if child has
* died from a signal or if the child returned EXIT_CANCELED.
- * Otherwise the returned value is the exit status of the child.
+ * Otherwise the returned value is the retval of the callback.
*/
int
virProcessRunInFork(virProcessForkCallback cb,
@@ -1377,7 +1379,7 @@ virProcessRunInFork(virProcessForkCallback cb,
ret = virProcessWait(child, &status, false);
if (ret == 0) {
ret = status == EXIT_CANCELED ? -1 : status;
- if (ret) {
+ if (ret < 0) {
if (nread == sizeof(*bin)) {
bin = g_new0(errorDataBin, 1);
memcpy(bin->bindata, buf, sizeof(*bin));
--
2.31.1
9:46 a.m.
New subject: [PATCH v2 3/3] qemu: Do not erase duplicate devices from namespace if error occurs
If the attempt to attach a device failed, we erased the
unattached device from the namespace. This resulted in erasing an
already attached device in case of a duplicate. We need to check
for existing file in the namespace in order to determine erasing
it in case of a failure.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1780508
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/qemu/qemu_domain.c | 4 +--
src/qemu/qemu_hotplug.c | 27 +++++++------------
src/qemu/qemu_namespace.c | 55 +++++++++++++++++++++++----------------
src/qemu/qemu_namespace.h | 18 ++++++++-----
src/qemu/qemu_process.c | 2 +-
5 files changed, 55 insertions(+), 51 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 89e03fdf72..ac1d8ef151 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7934,10 +7934,8 @@ qemuDomainStorageSourceAccessModify(virQEMUDriver *driver,
revoke_nvme = true;
- if (qemuDomainNamespaceSetupDisk(vm, src) < 0)
+ if (qemuDomainNamespaceSetupDisk(vm, src, &revoke_namespace) < 0)
goto revoke;
-
- revoke_namespace = true;
}
if (qemuSecuritySetImageLabel(driver, vm, src, chain, chain_top) < 0)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index cb6a4e4ea5..6743a8a742 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1685,9 +1685,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver,
goto error;
teardownmemlock = true;
- if (qemuDomainNamespaceSetupHostdev(vm, hostdev) < 0)
+ if (qemuDomainNamespaceSetupHostdev(vm, hostdev, &teardowndevice) < 0)
goto error;
- teardowndevice = true;
if (qemuSetupHostdevCgroup(vm, hostdev) < 0)
goto error;
@@ -2219,9 +2218,8 @@ int qemuDomainAttachChrDevice(virQEMUDriver *driver,
if (rc == 1)
need_release = true;
- if (qemuDomainNamespaceSetupChardev(vm, chr) < 0)
+ if (qemuDomainNamespaceSetupChardev(vm, chr, &teardowndevice) < 0)
goto cleanup;
- teardowndevice = true;
if (qemuSecuritySetChardevLabel(driver, vm, chr) < 0)
goto cleanup;
@@ -2332,9 +2330,8 @@ qemuDomainAttachRNGDevice(virQEMUDriver *driver,
if (qemuDomainEnsureVirtioAddress(&releaseaddr, vm, &dev) < 0)
return -1;
- if (qemuDomainNamespaceSetupRNG(vm, rng) < 0)
+ if (qemuDomainNamespaceSetupRNG(vm, rng, &teardowndevice) < 0)
goto cleanup;
- teardowndevice = true;
if (qemuSetupRNGCgroup(vm, rng) < 0)
goto cleanup;
@@ -2476,9 +2473,8 @@ qemuDomainAttachMemory(virQEMUDriver *driver,
if (qemuProcessBuildDestroyMemoryPaths(driver, vm, mem, true) < 0)
goto cleanup;
- if (qemuDomainNamespaceSetupMemory(vm, mem) < 0)
+ if (qemuDomainNamespaceSetupMemory(vm, mem, &teardowndevice) < 0)
goto cleanup;
- teardowndevice = true;
if (qemuSetupMemoryDevicesCgroup(vm, mem) < 0)
goto cleanup;
@@ -2592,9 +2588,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriver *driver,
added = true;
- if (qemuDomainNamespaceSetupHostdev(vm, hostdev) < 0)
+ if (qemuDomainNamespaceSetupHostdev(vm, hostdev, &teardowndevice) < 0)
goto cleanup;
- teardowndevice = true;
if (qemuSetupHostdevCgroup(vm, hostdev) < 0)
goto cleanup;
@@ -2673,9 +2668,8 @@ qemuDomainAttachHostSCSIDevice(virQEMUDriver *driver,
if (qemuHostdevPrepareSCSIDevices(driver, vm->def->name, &hostdev, 1) <
0)
return -1;
- if (qemuDomainNamespaceSetupHostdev(vm, hostdev) < 0)
+ if (qemuDomainNamespaceSetupHostdev(vm, hostdev, &teardowndevice) < 0)
goto cleanup;
- teardowndevice = true;
if (qemuSetupHostdevCgroup(vm, hostdev) < 0)
goto cleanup;
@@ -2767,9 +2761,8 @@ qemuDomainAttachSCSIVHostDevice(virQEMUDriver *driver,
if (qemuHostdevPrepareSCSIVHostDevices(driver, vm->def->name, &hostdev, 1)
< 0)
return -1;
- if (qemuDomainNamespaceSetupHostdev(vm, hostdev) < 0)
+ if (qemuDomainNamespaceSetupHostdev(vm, hostdev, &teardowndevice) < 0)
goto cleanup;
- teardowndevice = true;
if (qemuSetupHostdevCgroup(vm, hostdev) < 0)
goto cleanup;
@@ -2894,9 +2887,8 @@ qemuDomainAttachMediatedDevice(virQEMUDriver *driver,
goto cleanup;
added = true;
- if (qemuDomainNamespaceSetupHostdev(vm, hostdev) < 0)
+ if (qemuDomainNamespaceSetupHostdev(vm, hostdev, &teardowndevice) < 0)
goto cleanup;
- teardowndevice = true;
if (qemuSetupHostdevCgroup(vm, hostdev) < 0)
goto cleanup;
@@ -3231,9 +3223,8 @@ qemuDomainAttachInputDevice(virQEMUDriver *driver,
if (qemuBuildInputDevStr(&devstr, vm->def, input, priv->qemuCaps) < 0)
goto cleanup;
- if (qemuDomainNamespaceSetupInput(vm, input) < 0)
+ if (qemuDomainNamespaceSetupInput(vm, input, &teardowndevice) < 0)
goto cleanup;
- teardowndevice = true;
if (qemuSetupInputCgroup(vm, input) < 0)
goto cleanup;
diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index eb048a2faa..46ee95b8c8 100644
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -610,7 +610,8 @@ qemuDomainSetupLaunchSecurity(virDomainObj *vm,
static int
qemuNamespaceMknodPaths(virDomainObj *vm,
- GSList *paths);
+ GSList *paths,
+ bool *created);
int
@@ -657,7 +658,7 @@ qemuDomainBuildNamespace(virQEMUDriverConfig *cfg,
if (qemuDomainSetupLaunchSecurity(vm, &paths) < 0)
return -1;
- if (qemuNamespaceMknodPaths(vm, paths) < 0)
+ if (qemuNamespaceMknodPaths(vm, paths, NULL) < 0)
return -1;
return 0;
@@ -1235,7 +1236,8 @@ qemuNamespacePrepareOneItem(qemuNamespaceMknodData *data,
static int
qemuNamespaceMknodPaths(virDomainObj *vm,
- GSList *paths)
+ GSList *paths,
+ bool *created)
{
qemuDomainObjPrivate *priv = vm->privateData;
virQEMUDriver *driver = priv->driver;
@@ -1280,15 +1282,13 @@ qemuNamespaceMknodPaths(virDomainObj *vm,
if (qemuSecurityPreFork(driver->securityManager) < 0)
goto cleanup;
- if (virProcessRunInMountNamespace(vm->pid,
- qemuNamespaceMknodHelper,
- &data) < 0) {
- qemuSecurityPostFork(driver->securityManager);
- goto cleanup;
- }
+ ret = virProcessRunInMountNamespace(vm->pid, qemuNamespaceMknodHelper,
+ &data);
+ if (ret == 0 && created != NULL)
+ *created = true;
+
qemuSecurityPostFork(driver->securityManager);
- ret = 0;
cleanup:
for (i = 0; i < data.nitems; i++) {
if (data.items[i].bindmounted &&
@@ -1307,7 +1307,8 @@ qemuNamespaceMknodPaths(virDomainObj *vm,
static int
qemuNamespaceMknodPaths(virDomainObj *vm G_GNUC_UNUSED,
- GSList *paths G_GNUC_UNUSED)
+ GSList *paths G_GNUC_UNUSED,
+ bool *created G_GNUC_UNUSED)
{
virReportSystemError(ENOSYS, "%s",
_("Namespaces are not supported on this platform."));
@@ -1393,7 +1394,8 @@ qemuNamespaceUnlinkPaths(virDomainObj *vm,
int
qemuDomainNamespaceSetupDisk(virDomainObj *vm,
- virStorageSource *src)
+ virStorageSource *src,
+ bool *created)
{
g_autoptr(virGSListString) paths = NULL;
@@ -1403,7 +1405,7 @@ qemuDomainNamespaceSetupDisk(virDomainObj *vm,
if (qemuDomainSetupDisk(src, &paths) < 0)
return -1;
- if (qemuNamespaceMknodPaths(vm, paths) < 0)
+ if (qemuNamespaceMknodPaths(vm, paths, created) < 0)
return -1;
return 0;
@@ -1437,7 +1439,8 @@ qemuDomainNamespaceTeardownDisk(virDomainObj *vm G_GNUC_UNUSED,
*/
int
qemuDomainNamespaceSetupHostdev(virDomainObj *vm,
- virDomainHostdevDef *hostdev)
+ virDomainHostdevDef *hostdev,
+ bool *created)
{
g_autoptr(virGSListString) paths = NULL;
@@ -1450,7 +1453,7 @@ qemuDomainNamespaceSetupHostdev(virDomainObj *vm,
&paths) < 0)
return -1;
- if (qemuNamespaceMknodPaths(vm, paths) < 0)
+ if (qemuNamespaceMknodPaths(vm, paths, created) < 0)
return -1;
return 0;
@@ -1492,7 +1495,8 @@ qemuDomainNamespaceTeardownHostdev(virDomainObj *vm,
int
qemuDomainNamespaceSetupMemory(virDomainObj *vm,
- virDomainMemoryDef *mem)
+ virDomainMemoryDef *mem,
+ bool *created)
{
g_autoptr(virGSListString) paths = NULL;
@@ -1502,7 +1506,7 @@ qemuDomainNamespaceSetupMemory(virDomainObj *vm,
if (qemuDomainSetupMemory(mem, &paths) < 0)
return -1;
- if (qemuNamespaceMknodPaths(vm, paths) < 0)
+ if (qemuNamespaceMknodPaths(vm, paths, created) < 0)
return -1;
return 0;
@@ -1530,7 +1534,8 @@ qemuDomainNamespaceTeardownMemory(virDomainObj *vm,
int
qemuDomainNamespaceSetupChardev(virDomainObj *vm,
- virDomainChrDef *chr)
+ virDomainChrDef *chr,
+ bool *created)
{
g_autoptr(virGSListString) paths = NULL;
@@ -1540,7 +1545,7 @@ qemuDomainNamespaceSetupChardev(virDomainObj *vm,
if (qemuDomainSetupChardev(vm->def, chr, &paths) < 0)
return -1;
- if (qemuNamespaceMknodPaths(vm, paths) < 0)
+ if (qemuNamespaceMknodPaths(vm, paths, created) < 0)
return -1;
return 0;
@@ -1568,7 +1573,8 @@ qemuDomainNamespaceTeardownChardev(virDomainObj *vm,
int
qemuDomainNamespaceSetupRNG(virDomainObj *vm,
- virDomainRNGDef *rng)
+ virDomainRNGDef *rng,
+ bool *created)
{
g_autoptr(virGSListString) paths = NULL;
@@ -1578,7 +1584,7 @@ qemuDomainNamespaceSetupRNG(virDomainObj *vm,
if (qemuDomainSetupRNG(rng, &paths) < 0)
return -1;
- if (qemuNamespaceMknodPaths(vm, paths) < 0)
+ if (qemuNamespaceMknodPaths(vm, paths, created) < 0)
return -1;
return 0;
@@ -1606,9 +1612,11 @@ qemuDomainNamespaceTeardownRNG(virDomainObj *vm,
int
qemuDomainNamespaceSetupInput(virDomainObj *vm,
- virDomainInputDef *input)
+ virDomainInputDef *input,
+ bool *created)
{
g_autoptr(virGSListString) paths = NULL;
+ int ret = 0;
if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
return 0;
@@ -1616,8 +1624,9 @@ qemuDomainNamespaceSetupInput(virDomainObj *vm,
if (qemuDomainSetupInput(input, &paths) < 0)
return -1;
- if (qemuNamespaceMknodPaths(vm, paths) < 0)
+ if ((ret = qemuNamespaceMknodPaths(vm, paths, created)) < 0)
return -1;
+
return 0;
}
diff --git a/src/qemu/qemu_namespace.h b/src/qemu/qemu_namespace.h
index 771d7873ef..020aca13d8 100644
--- a/src/qemu/qemu_namespace.h
+++ b/src/qemu/qemu_namespace.h
@@ -50,37 +50,43 @@ void qemuDomainDestroyNamespace(virQEMUDriver *driver,
bool qemuDomainNamespaceAvailable(qemuDomainNamespace ns);
int qemuDomainNamespaceSetupDisk(virDomainObj *vm,
- virStorageSource *src);
+ virStorageSource *src,
+ bool *created);
int qemuDomainNamespaceTeardownDisk(virDomainObj *vm,
virStorageSource *src);
int qemuDomainNamespaceSetupHostdev(virDomainObj *vm,
- virDomainHostdevDef *hostdev);
+ virDomainHostdevDef *hostdev,
+ bool *created);
int qemuDomainNamespaceTeardownHostdev(virDomainObj *vm,
virDomainHostdevDef *hostdev);
int qemuDomainNamespaceSetupMemory(virDomainObj *vm,
- virDomainMemoryDef *memory);
+ virDomainMemoryDef *memory,
+ bool *created);
int qemuDomainNamespaceTeardownMemory(virDomainObj *vm,
virDomainMemoryDef *memory);
int qemuDomainNamespaceSetupChardev(virDomainObj *vm,
- virDomainChrDef *chr);
+ virDomainChrDef *chr,
+ bool *created);
int qemuDomainNamespaceTeardownChardev(virDomainObj *vm,
virDomainChrDef *chr);
int qemuDomainNamespaceSetupRNG(virDomainObj *vm,
- virDomainRNGDef *rng);
+ virDomainRNGDef *rng,
+ bool *created);
int qemuDomainNamespaceTeardownRNG(virDomainObj *vm,
virDomainRNGDef *rng);
int qemuDomainNamespaceSetupInput(virDomainObj *vm,
- virDomainInputDef *input);
+ virDomainInputDef *input,
+ bool *created);
int qemuDomainNamespaceTeardownInput(virDomainObj *vm,
virDomainInputDef *input);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7487e12640..c972c90801 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8452,7 +8452,7 @@ qemuProcessRefreshLegacyBlockjob(void *payload,
if (disk->mirror->format &&
disk->mirror->format != VIR_STORAGE_FILE_RAW &&
- (qemuDomainNamespaceSetupDisk(vm, disk->mirror) < 0 ||
+ (qemuDomainNamespaceSetupDisk(vm, disk->mirror, NULL) < 0 ||
qemuSetupImageChainCgroup(vm, disk->mirror) < 0 ||
qemuSecuritySetImageLabel(priv->driver, vm, disk->mirror,
true, true) < 0))
--
2.31.1
5:05 a.m.
New subject: [PATCH v2 3/3] qemu: Do not erase duplicate devices from namespace if error occurs
On 7/14/21 4:46 PM, Kristina Hanicova wrote:
If the attempt to attach a device failed, we erased the
unattached device from the namespace. This resulted in erasing an
already attached device in case of a duplicate. We need to check
for existing file in the namespace in order to determine erasing
it in case of a failure.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1780508
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/qemu/qemu_domain.c | 4 +--
src/qemu/qemu_hotplug.c | 27 +++++++------------
src/qemu/qemu_namespace.c | 55 +++++++++++++++++++++++----------------
src/qemu/qemu_namespace.h | 18 ++++++++-----
src/qemu/qemu_process.c | 2 +-
5 files changed, 55 insertions(+), 51 deletions(-)
diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c
index eb048a2faa..46ee95b8c8 100644
--- a/src/qemu/qemu_namespace.c
+++ b/src/qemu/qemu_namespace.c
@@ -1235,7 +1236,8 @@
qemuNamespacePrepareOneItem(qemuNamespaceMknodData *data,
static int
qemuNamespaceMknodPaths(virDomainObj *vm,
- GSList *paths)
+ GSList *paths,
+ bool *created)
{
qemuDomainObjPrivate *priv = vm->privateData;
virQEMUDriver *driver = priv->driver;
@@ -1280,15 +1282,13 @@ qemuNamespaceMknodPaths(virDomainObj *vm,
if (qemuSecurityPreFork(driver->securityManager) < 0)
goto cleanup;
- if (virProcessRunInMountNamespace(vm->pid,
- qemuNamespaceMknodHelper,
- &data) < 0) {
- qemuSecurityPostFork(driver->securityManager);
- goto cleanup;
- }
+ ret = virProcessRunInMountNamespace(vm->pid, qemuNamespaceMknodHelper,
+ &data);
+ if (ret == 0 && created != NULL)
+ *created = true;
+
qemuSecurityPostFork(driver->securityManager);
Here it's better if qemuSecurityPostFork() is called before the if().
The reason we have PreFork() and PostFork() calls is to fight
async-signal unsafe functions; Anyway - PreFork() locks
driver->securityManager() and only PostFork() unlocks it. And it's just
virProcessRunInMountNamespace() that's in critical section, not if().
Micro optimization, I know.
Michal
5:05 a.m.
On 7/14/21 4:46 PM, Kristina Hanicova wrote:
This is v2 of:
https://listman.redhat.com/archives/libvir-list/2021-July/msg00129.html
Changes since v1 (suggested by Michal and Jano):
* change of int variable to one-word bool
* isolate changes outside of qemu to separate patch
* rewritten assignment of return value
* slight change of documentation
* extension of use to all devices, not just input
* @created variable is set in one core function instead of setting it in
each device function separately
Kristina Hanicova (3):
qemu: Check for existing file in namespace
virprocess: Return retval of the child on success, not 0
qemu: Do not erase duplicate devices from namespace if error occurs
src/qemu/qemu_domain.c | 4 +--
src/qemu/qemu_hotplug.c | 27 +++++----------
src/qemu/qemu_namespace.c | 71 +++++++++++++++++++++++++--------------
src/qemu/qemu_namespace.h | 18 ++++++----
src/qemu/qemu_process.c | 2 +-
src/util/virprocess.c | 12 ++++---
6 files changed, 75 insertions(+), 59 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
and pushed.
Michal
1262
days inactive
1263
days old
6 comments
2 participants
participants (2)
-
Kristina Hanicova -
Michal Prívozník