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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>

and move on with your life :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux