On 12/9/22 4:09 AM, Peter Krempa wrote:
On Thu, Oct 20, 2022 at 16:59:00 -0500, 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 | 251 +++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_nbdkit.h | 10 ++
2 files changed, 261 insertions(+)
...
+static int
+qemuNbdkitProcessBuildCommandSSH(qemuNbdkitProcess *proc,
+ virCommand *cmd)
+{
+ const char *user = NULL;
+ virStorageNetHostDef *host = &proc->source->hosts[0];
+ g_autofree char *portstr = g_strdup_printf("%u", host->port);
+
+ /* nbdkit plugin name */
+ virCommandAddArg(cmd, "ssh");
+
+ virCommandAddArgPair(cmd, "host", host->name);
+ virCommandAddArgPair(cmd, "port", portstr);
+ virCommandAddArgPair(cmd, "path", proc->source->path);
+
+ if (proc->source->auth)
+ user = proc->source->auth->username;
+ else if (proc->source->ssh_user)
+ user = proc->source->ssh_user;
So there's a bit problem with this, which also explains my reluctance to
simply supporting the SSH protocol in the current state without properly
designing other required parameters for SSH authentication. And even
then it will most likely break "historical compatibility".
So historically we didn't implement ssh protocol at all. Users (notably
libguestfs) worked this around by adding a wrapper QCOW2 image and
setting up the backing store string such that qemu would access the ssh
image.
When blockdev support was added I needed a way to port the existing
functionality specifically for libguestfs. This made me add the ssh
protocol and forwart the minimum set of properties to the image (thus
the 'ssh_user' field.
But there's another thing that was always configured out-of-band (via
environment variables) and that is the ssh agent socket path or ssh key
path.
These are not present in the virStorageSource because they are not even
configured for the blockdev 'ssh' protocol driver directly.
Now with switching to nbdkit this means agent/sshkey authentication will
necessarily break for such usage as we simply don't have the
information.
So what to do about it?
We can either break the old way completely, which I guess would be
mostly okay since libguestfs most likely now uses nbdkit anyways, and
then design it properly. Obviously since the qemu ssh blockdev backend
driver still configures stuff using environment variables thus the new
configuration will be available only when nbdkit is in use.
Other possibility, if we want to keep old functionality working as it
was, is to avoid the use of nbdkit.
Either way the implementation of SSH as done in this series simply just
breaks SSH usage completely, by not being able to support the old hacky
way of using agent/sshkey, not implementing any new support and not even
implementing password authentication.
What now? I don't really care too deeply about the hacky impl we have
now. If libguestfs is no longer using I reckon we can simply break it
and not deal with the fallout.
I'm not sure how others care about that though.
Obviously another option (besides doing a proper desing on
authentication config) is to not touch anything ssh-related for now.
Adding Rich to cc in case he has anything to add to this discussion.
The problem, as I understand it, is the desire to remove the qemu block
plugins from distributions. If that happens, then avoiding the use of
nbdkit within libvirt won't solve anything -- the ssh use case will
still break due to the absence of these qemu plugins.
I'm afraid I don't have enough background information about when ssh is
used and how common it is to make these decisions on my own. But I agree
that the current implementation in this patch series is not complete
enough to be useful.
Jonathon
