On Fri, Jan 20, 2023 at 16:03:04 -0600, Jonathon Jongsma wrote:
> Add some helper functions to build a virCommand object and run the
> nbdkit process for a given virStorageSource.
>
> Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> ---
> src/qemu/qemu_nbdkit.c | 255 +++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_nbdkit.h | 10 ++
> 2 files changed, 265 insertions(+)
[...]
> @@ -667,6 +668,167 @@ qemuNbdkitInitStorageSource(qemuNbdkitCaps *caps,
> }
>
>
> +static int
> +qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc,
> + virCommand *cmd)
> +{
> + g_autoptr(virURI) uri = qemuBlockStorageSourceGetURI(proc->source);
> + g_autofree char *uristring = virURIFormat(uri);
> +
> + /* nbdkit plugin name */
> + virCommandAddArg(cmd, "curl");
> + if (proc->source->protocol == VIR_STORAGE_NET_PROTOCOL_HTTP) {
> + /* allow http to be upgraded to https via e.g. redirect */
> + g_autofree char* protocols = g_strdup_printf("%s,%s",
> + virStorageNetProtocolTypeToString(proc->source->protocol),
> + virStorageNetProtocolTypeToString(VIR_STORAGE_NET_PROTOCOL_HTTPS));
IMO you can spell out "http,https" manually here rahter than attempting
to construct it from the convertors when the result is always the same.
> + virCommandAddArgPair(cmd, "protocols", protocols);
> + } else {
> + virCommandAddArgPair(cmd, "protocols",
> + virStorageNetProtocolTypeToString(proc->source->protocol));
> + }
> + virCommandAddArgPair(cmd, "url", uristring);
> +
> + if (proc->source->auth) {
> + g_autoptr(virConnect) conn = virGetConnectSecret();
> + g_autofree uint8_t *secret = NULL;
> + size_t secretlen = 0;
> + g_autofree char *password = NULL;
> + int secrettype;
> + virStorageAuthDef *authdef = proc->source->auth;
> +
> + virCommandAddArgPair(cmd, "user",
> + proc->source->auth->username);
> +
> + if ((secrettype = virSecretUsageTypeFromString(proc->source->auth->secrettype)) < 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("invalid secret type %s"),
> + proc->source->auth->secrettype);
> + return -1;
> + }
> +
> + if (virSecretGetSecretString(conn,
> + &authdef->seclookupdef,
> + secrettype,
> + &secret,
> + &secretlen) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("failed to get auth secret for storage"));
> + return -1;
> + }
> +
> + /* ensure that the secret is a NULL-terminated string */
> + password = g_strndup((char*)secret, secretlen);
Both 'secret' and 'password' should be disposed of using
virSecureErase(String).
> +
> + /* for now, just report an error rather than passing the password in
> + * cleartext on the commandline */
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Password not yet supported for nbdkit sources"));
> + return -1;
> + }
> +
> + if (proc->source->ncookies > 0) {
> + /* for now, just report an error rather than passing cookies in
> + * cleartext on the commandline */
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Cookies not yet supported for nbdkit sources"));
> + return -1;
> + }
> +
> + if (proc->source->sslverify == VIR_TRISTATE_BOOL_NO) {
> + virCommandAddArgPair(cmd, "sslverify", "false");
[...]
> @@ -675,3 +837,96 @@ qemuNbdkitProcessFree(qemuNbdkitProcess *proc)
> g_clear_object(&proc->caps);
> g_free(proc);
> }
> +
> +
> +int
> +qemuNbdkitProcessStart(qemuNbdkitProcess *proc,
> + virDomainObj *vm,
> + virQEMUDriver *driver)
> +{
> + g_autoptr(virCommand) cmd = NULL;
> + int rc;
> + int exitstatus = 0;
> + int cmdret = 0;
> + VIR_AUTOCLOSE errfd = -1;
> + virTimeBackOffVar timebackoff;
> + bool socketCreated = false;
> +
> + if (!(cmd = qemuNbdkitProcessBuildCommand(proc)))
> + return -1;
> +
> + VIR_DEBUG("starting nbdkit process for %s", proc->source->nodestorage);
> + virCommandSetErrorFD(cmd, &errfd);
Using virCommandSetErrorBuffer will probably be cleaner given the
reading needed in the error fd.
> + virCommandSetPidFile(cmd, proc->pidfile);
Once you de-clutter the formatting of the allowed protocols:
Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
