On 4/19/22 9:05 AM, Daniel P. Berrangé wrote:
On Thu, Apr 14, 2022 at 05:02:46PM -0500, Jonathon Jongsma wrote:
As mentioned in https://bugzilla.redhat.com/show_bug.cgi?id=2016527, RHEL is
planning to remove dependencies on the qemu-block-curl and qemu-block-ssh
plugins from the main qemu package. This creates issues for libvirt for
supporting network disk sources. So I've been looking into using nbdkit from
libvirt to proxy these network disks to qemu as NBD disks.
The basic idea is that libvirt will spin up an nbdkit instance for e.g. an
https network disk source, and will provide the resulting unix socket to
qemu as an nbd disk. This allows libvirt to continue supporting http/ftp/ssh
disk sources regardless of whether the qemu block plugins are installed or
not.
However, there are a couple of issues and feature gaps that I've run into
that I'd like to discuss.
1. secrets
There is some code in libvirt[1] which seems to expect that it is possible
for http(s) disk sources to have a username and password specified. However,
I can't find any valid xml schema for specifying an http username and
password, and my reading of the code suggests that there shouldn't be any
way for these to be set for http(s)/ftp(s) [disk sources either since auth is
only supported for ISCSI and RBD protocols [2]. Am I missing something?
[1] https://gitlab.com/libvirt/libvirt/-/blob/6be7beb3bdb9ad611a5598dad7edfbd2a836fd2e/src/qemu/qemu_block.c#L749
[2] https://gitlab.com/libvirt/libvirt/-/blob/6be7beb3bdb9ad611a5598dad7edfbd2a836fd2e/src/qemu/qemu_domain.c#L1235
I'm unclear if this is a regression, or a long term bug. Clearly the
intention of the code is to support auth, so if it doesn't work right
now we need to fix that, regardless of adding the nbdkit support.
I do notice we don't have any XML in tests/qemuxml2argvdata/ that
exercises auth with https, which is likely why we have this bug
lurking.
Well, As far as I can tell, there is no valid XML for exercising http
auth. The schema for http(s) sources does not include any <auth> element
[1]. And the schema for the <auth> element [2] requires a <secret>
element with a required 'type' attribute, and the only choices for
'type' are 'ceph' or 'iscsi', neither of which apply to http authentication.
So it looks to me like http auth was never supported, rather than a
regression. It also seems that it would require some additional schema
changes to support this.
[1]
https://gitlab.com/libvirt/libvirt/-/blob/136b821f183deb0b58c571211f6917985bed3308/src/conf/schemas/domaincommon.rng#L1974
[2]https://gitlab.com/libvirt/libvirt/-/blob/136b821f183deb0b58c571211f6917985bed3308/src/conf/schemas/domaincommon.rng#L6969
If it *is* possible for the username/password to be set for these disks,
then we have the issue that these sensitive pieces of data have so far been
passed as encrypted data to qemu using qemu secrets. But if nbdkit is
handling the http requests, we need to pass this data to nbdkit rather than
qemu and so we can no longer use qemu secrets. The same issue applies to
http cookies, which could potentially include security-sensitive data such
as login credentials, etc.
Fortunately, nbdkit provides a method for reading cookies and passwords from
a file, which should be secure if the file has permissions set properly. So
I'm currently planning to write a file containing the cookies and pass them
to nbdkit by specifying the filename. But I'm still confused about the
username/password possibility.
2. readahead
The libvirt xml format allows to specify a readahead size for disks that are
handled by the qemu-block-curl plugin. Unfortunately, nbdkit doesn't
currently support any readahead configuration. In nbdkit, readahead is
handled by an nbkit "filter" that takes no configuration options (`nbdkit
--filter=readahead ...`). In theory, this filter tries to adaptively read
ahead. But when I discussed it with Rich, he suggested that he had stopped
using it in virt-v2v because it was causing more trouble than it was worth.
He also suggested that this readahead filter might need a complete rewrite,
and presumably the rewrite could include the ability to configure a
readahead buffer size. But I'm not sure what the timeframe might be for
that.
In terms of libvirt upstream policy, there is no requirement for a given
hypervisor implementation to support all the features possible in the XML.
If readahead isn't supported then just report VIR_ERR_CONFIG_UNSUPPORTED
if the user provides it in the XML.
If there is a choice of backends though, we should prefer the impl that
avoids regressions.
IOW, if both qemu-block-curl and nbdkit are installed, we must prefer
qemu-block-curl.
It is upto RHEL downstream if they are happy to ship only nbdkit given
the feature differences.
3. blockdev-create
There is support in libvirt[3] for creating ssh network disks by sending a
'blockdev-create' command to qemu. If qemu is no longer handling ssh network
disk sources directly, this feature becomes significantly more complicated.
I don't yet know enough about this part of the libvirt code to know what
further complications might pop up here. From my reading of the code, this
is mostly triggered by things like `virsh blockcopy` `virsh backup-begin`,
etc. But nbdkit cannot currently do this. Rich pointed me to a recent
commit[4] where he added disk creation to the nbdkit vddk plugin, and
suggested that something similar could be added for the nbdkit ssh plugin.
[3] https://gitlab.com/libvirt/libvirt/-/blob/6be7beb3bdb9ad611a5598dad7edfbd2a836fd2e/src/qemu/qemu_block.c#L2639
[4] https://gitlab.com/nbdkit/nbdkit/-/commit/a39d5773afc3ebab7e5768118a2bccb89a654585
It seems to me that it's essential that we resolve #3 before we can move
forward with nbdkit support in libvirt. (Although I admit that I have no
idea how common it is for people to use ssh disks so I suppose there's a
slim possibility that we could just disable the 'create disk' feature for
ssh disks without any practical loss of functionality?) But it's less
obvious to me whether we could move ahead despite missing readahead size
configuration, etc.
Again, from an upstream POV #3 isn't a blocker, as long as we prefer
qemu-block-curl when available. It is upto RHEL to decide whether
#3 is a blocker for their downstream dropping of qemu-block-curl.
And now I notice that we do not actually have support for 'ssh' network
disks in our xml schema either [3]. The VIR_STORAGE_NET_PROTOCOL_SSH
enum value was originally added with a comment that it allows using the
ssh protocol in backing chains. I've also seen some commits indicating
that some of the support for ssh disk sources may be related to
libguestfs usage in some way. cc'ing Rich and Peter in case they can add
any historical context here.
[3]
https://gitlab.com/libvirt/libvirt/-/blob/136b821f183deb0b58c571211f6917985bed3308/src/conf/schemas/domaincommon.rng#L2150
