Re: [libvirt] [PATCH v2 09/15] qemu_firmware: Introduce qemuFirmwareFetchConfigs

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(a)redhat.com&gt; > --- > 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.