5:28 a.m.
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(a)redhat.com>
>> ---
>> 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