[PATCH v1 10/34] qemuDomainAttachDeviceMknodRecursive: Isolate bind mounted devices condition

[Ján Tomko]

On a Wednesday in 2020, Michal Privoznik wrote:
>When attaching a device into a domain, the corresponding /dev
>node might need to be created in the domain's namespace. For some
>types of files we call mknod(), for symlinks we call symlink(),
>but for others - which exist in the host namespace - we need to
>so called 'bind mount' them (which is a way of passing a
>file/directory between mount namespaces). There is this condition
>in qemuDomainAttachDeviceMknodRecursive() which decides whether a
>bind mount will be used, move it into a separate function so that
>it can be reused later.
>
>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> src/qemu/qemu_domain_namespace.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
>diff --git a/src/qemu/qemu_domain_namespace.c b/src/qemu/qemu_domain_namespace.c
>index 40c4fb36cb..0c40118938 100644
>--- a/src/qemu/qemu_domain_namespace.c
>+++ b/src/qemu/qemu_domain_namespace.c
>@@ -1253,6 +1253,17 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid G_GNUC_UNUSED,
> }
>
>
>+static bool
>+qemuDomainMknodItemIsBindMounted(mode_t st_mode)

To me, IsBindMounted reads like it's done already, but we yet have to do
that.

Consider:
qemuNamespaceMknodItemNeedsBindMount

>+{
>+    /* A block device S_ISBLK() or a chardev S_ISCHR() is intentionally not
>+     * handled.  We want to mknod() it instead of passing in through bind
>+     * mounting. */
>+    return S_ISREG(st_mode) || S_ISFIFO(st_mode) ||
>+           S_ISSOCK(st_mode) || S_ISDIR(st_mode);
>+}
>+
>+
> static int
> qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr driver,
>                                      virDomainObjPtr vm,
>@@ -1267,7 +1278,6 @@ qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr driver,
>     g_autofree char *target = NULL;
>     bool isLink;
>     bool isReg;
>-    bool isDir;
>
>     if (!ttl) {
>         virReportSystemError(ELOOP,
>@@ -1289,10 +1299,9 @@ qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr driver,
>     }
>
>     isLink = S_ISLNK(data.sb.st_mode);
>-    isReg = S_ISREG(data.sb.st_mode) || S_ISFIFO(data.sb.st_mode) || S_ISSOCK(data.sb.st_mode);
>-    isDir = S_ISDIR(data.sb.st_mode);
>+    isReg = qemuDomainMknodItemIsBindMounted(data.sb.st_mode);

note that the variable is still misleadingly called isReg.

>
>-    if ((isReg || isDir) && STRPREFIX(file, QEMU_DEVPREFIX)) {
>+    if (isReg && STRPREFIX(file, QEMU_DEVPREFIX)) {
>         cfg = virQEMUDriverGetConfig(driver);
>         if (!(target = qemuDomainGetPreservedMountPath(cfg, vm, file)))

Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200724/70bafd25/attachment-0001.sig>