On 03/07/19 13:46, Michal Privoznik wrote: > On 3/7/19 12:55 PM, Daniel P. Berrangé wrote: >> On Thu, Mar 07, 2019 at 10:29:19AM +0100, Michal Privoznik wrote: >>> Implementation for yet another part of firmware description >>> specification. This one covers selecting which files to parse. >>> >>> There are three locations from which description files can be >>> loaded. In order of preference, from most generic to most >>> specific these are: >>> >>> /usr/share/qemu/firmware >>> /etc/qemu/firmware >>> $XDG_CONFIG_HOME/qemu/firmware >>> >>> If a file is found in two or more locations then the most specific >>> one is used. Moreover, if file is empty then it means it is >>> overriding some generic description and disabling it. >>> >>> Again, this is described in more details and with nice examples >>> in firmware.json specification (qemu commit 3a0adfc9bf). >>> >>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> >>> --- >>> src/qemu/qemu_firmware.c | 134 +++++++++++++++++++++++++++++++++++++++ >>> src/qemu/qemu_firmware.h | 3 + >>> 2 files changed, 137 insertions(+) >>> >>> diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c >>> index 8f718ee2a6..a818f60c91 100644 >>> --- a/src/qemu/qemu_firmware.c >>> +++ b/src/qemu/qemu_firmware.c >>> @@ -21,9 +21,11 @@ >>> #include <config.h> >>> #include "qemu_firmware.h" >>> +#include "configmake.h" >>> #include "qemu_capabilities.h" >>> #include "virarch.h" >>> #include "virfile.h" >>> +#include "virhash.h" >>> #include "virjson.h" >>> #include "virlog.h" >>> #include "virstring.h" >>> @@ -899,3 +901,135 @@ qemuFirmwareFormat(qemuFirmwarePtr fw) >>> return virJSONValueToString(doc, true); >>> } >>> + >>> + >>> +static int >>> +qemuFirmwareBuildFileList(virHashTablePtr files, const char *dir) >>> +{ >>> + DIR *dirp; >>> + struct dirent *ent = NULL; >>> + int rc; >>> + int ret = -1; >>> + >>> + if ((rc = virDirOpenIfExists(&dirp, dir)) < 0) >>> + return -1; >>> + >>> + if (rc == 0) >>> + return 0; >>> + >>> + while ((rc = virDirRead(dirp, &ent, dir)) > 0) { >>> + VIR_AUTOFREE(char *) filename = NULL; >>> + VIR_AUTOFREE(char *) path = NULL; >>> + >>> + if (ent->d_type != DT_REG && ent->d_type != DT_LNK) >>> + continue; >>> + >>> + if (STRPREFIX(ent->d_name, ".")) >>> + continue; >>> + >>> + if (VIR_STRDUP(filename, ent->d_name) < 0) >>> + goto cleanup; >>> + >>> + if (virAsprintf(&path, "%s/%s", dir, filename) < 0) >>> + goto cleanup; >>> + >>> + if (virHashUpdateEntry(files, filename, path) < 0) >>> + goto cleanup; >>> + >>> + path = NULL; >>> + } >>> + >>> + if (rc < 0) >>> + goto cleanup; >>> + >>> + ret = 0; >>> + cleanup: >>> + virDirClose(&dirp); >>> + return ret; >>> +} >>> + >>> +static int >>> +qemuFirmwareFilesSorter(const virHashKeyValuePair *a, >>> + const virHashKeyValuePair *b) >>> +{ >>> + return strcmp(a->key, b->key); >>> +} >>> + >>> +#define QEMU_FIRMWARE_SYSTEM_LOCATION PREFIX "/share/qemu/firmware" >>> +#define QEMU_FIRMWARE_ETC_LOCATION SYSCONFDIR "/qemu/firmware" >>> + >>> +int >>> +qemuFirmwareFetchConfigs(char ***firmwares) >>> +{ >>> + VIR_AUTOPTR(virHashTable) files = NULL; >>> + VIR_AUTOFREE(char *) homeConfig = NULL; >>> + VIR_AUTOFREE(char *) xdgConfig = NULL; >>> + VIR_AUTOFREE(virHashKeyValuePairPtr) pairs = NULL; >>> + virHashKeyValuePairPtr tmp = NULL; >>> + >>> + *firmwares = NULL; >>> + >>> + if (VIR_STRDUP(xdgConfig, virGetEnvBlockSUID("XDG_CONFIG_HOME")) >>> < 0) >>> + return -1; >>> + >>> + if (!xdgConfig) { >>> + VIR_AUTOFREE(char *) home = virGetUserDirectory(); >>> + >>> + if (!home) >>> + return -1; >>> + >>> + if (virAsprintf(&xdgConfig, "%s/.config", home) < 0) >>> + return -1; >>> + } >>> + >>> + if (virAsprintf(&homeConfig, "%s/qemu/firmware", xdgConfig) < 0) >>> + return -1; >>> + >>> + if (!(files = virHashCreate(10, virHashValueFree))) >>> + return -1; >>> + >>> + if (qemuFirmwareBuildFileList(files, >>> QEMU_FIRMWARE_SYSTEM_LOCATION) < 0) >>> + return -1; >>> + >>> + if (qemuFirmwareBuildFileList(files, QEMU_FIRMWARE_ETC_LOCATION) >>> < 0) >>> + return -1; >>> + >>> + if (qemuFirmwareBuildFileList(files, homeConfig) < 0) >>> + return -1; >> >> I wonder if it really makes sense to read /root/ for this. Normally we >> would >> only look at the home config for unprivileged daemon, eg we don't look in >> /root for finding PKI x509 certs IIRC. > > Fair enough. Root is able to put configs into /etc/qemu/firmware > anyways. Will fix this locally for now. Please carry forward my R-b, given up-thread, when you implement the tweak suggested by Dan. Thanks Laszlo -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list