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@xxxxxxxxxx>
>
> and move on with your life :)
Thanks,
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
