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@redhat.com> --- 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@redhat.com>