[libvirt] [PATCH] qemu: De-duplicate some path definitions

Andrea Bolognani abologna at redhat.com
Mon Jul 1 14:47:59 UTC 2019


On Tue, 2019-06-25 at 13:38 +0200, Michal Privoznik wrote:
> There are some paths (e.g. /dev/vfio/vfio or /dev/mapper/control)
> which are defined in qemu_domain.c and then in qemu_cgroup.c
> again. This is suboptimal. Lets move paths into qemu_domain.h and

s/Lets/Let's/

[...]
> -#define PROC_MOUNTS "/proc/mounts"

$ git grep '"/proc/mounts"' | grep -vE '^(po|docs)/'
src/lxc/lxc_container.c:    if (virFileGetMountReverseSubtree("/proc/mounts", prefix,
src/lxc/lxc_container.c:    if (!(procmnt = setmntent("/proc/mounts", "r"))) {
src/qemu/qemu_domain.h:#define QEMU_PROC_MOUNTS "/proc/mounts"
src/util/vircgroup.c:    mounts = fopen("/proc/mounts", "r");
src/util/vircgroupv1.c:    if (!(mounts = fopen("/proc/mounts", "r")))
src/util/vircgroupv2.c:    if (!(mounts = fopen("/proc/mounts", "r")))
src/util/virfile.c:    f = setmntent("/proc/mounts", "r");
src/util/virfile.c:# define PROC_MOUNTS "/proc/mounts"
tests/vircgroupmock.c:    if (STREQ(path, "/proc/mounts")) {
tests/vircgroupmock.c:    } else if (STREQ(path, "/proc/mounts")) {

> -#define DEVPREFIX "/dev/"

Too many instances to include here! But quite a few are spurious.

> -#define DEV_VFIO "/dev/vfio/vfio"

$ git grep '"/dev/vfio/vfio"' | grep -vE '^(po|docs)/'
src/qemu/qemu_domain.h:#define QEMU_DEV_VFIO "/dev/vfio/vfio"
src/security/virt-aa-helper.c:        virBufferAddLit(&buf, "  \"/dev/vfio/vfio\" rw,\n");

> -#define DEVICE_MAPPER_CONTROL_PATH "/dev/mapper/control"

This one's good :)

> -#define DEV_SEV "/dev/sev"

$ git grep '"/dev/sev"' | grep -vE '^(po|docs)/'
src/qemu/qemu_cgroup.c:    ret = virCgroupAllowDevicePath(priv->cgroup, "/dev/sev",
src/qemu/qemu_cgroup.c:    virDomainAuditCgroupPath(vm, priv->cgroup, "allow", "/dev/sev",
src/qemu/qemu_domain.h:#define QEMU_DEV_SEV "/dev/sev"
src/security/security_dac.c:#define DEV_SEV "/dev/sev"


So most of the above are used at least once outside of the QEMU
drivers... Should we have a virpath.h to collect all these defines
under a single roof? And possibly a syntax-check rule to ensure
additional uses of the bare string don't slip back in over time?


Anyway, if you feel like going further down the rabbit hole be my
guest, and I'll happily review the resulting patches. If not, you
can just fix the commit message, pick up this

  Reviewed-by: Andrea Bolognani <abologna at redhat.com>

and move on with your life :)

-- 
Andrea Bolognani / Red Hat / Virtualization




More information about the libvir-list mailing list