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

[Michal Prívozník]

On 7/1/19 4:47 PM, Andrea Bolognani wrote:
> 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?

Sounds good. But I'll save it for a follow up patch or something. I
needed this patch because of my NVMe work. Therefore I chose to push
this one.

> 
> 
> 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 :)

Thanks,
Michal