On Wed, Dec 16, 2020 at 19:37:49 -0600, Ryan Gahagan wrote:
Please add a commit message.
> Signed-off-by: Ryan Gahagan <rgahagan@xxxxxxxxxxxxx>
> ---
> src/libxl/libxl_conf.c | 1 +
> src/libxl/xen_xl.c | 1 +
> src/qemu/qemu_block.c | 3 +++
> src/qemu/qemu_command.c | 1 +
> src/qemu/qemu_domain.c | 10 ++++++++++
> src/qemu/qemu_snapshot.c | 3 +++
> src/util/virstoragefile.c | 6 ++++++
> src/util/virstoragefile.h | 1 +
> 8 files changed, 26 insertions(+)
[...]
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index bfb6e23942..692bc925c6 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4626,6 +4626,14 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src,
> return -1;
> }
>
> + /* NFS protocol may only be used if current QEMU supports blockdev */
> + if (actualType == VIR_STORAGE_TYPE_NETWORK &&
> + src->protocol == VIR_STORAGE_NET_PROTOCL_NFS &&
> + !blockdev) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("'nfs' protocol is not supported with this QEMU binary"));
> + }
This doesn't belong to this patch. This should be part of the patch
implementing the qemu support for it.
In this patch you are adding a new type and we try to minimize and
separate changes.
> +
> return 0;
> }
[..]
> @@ -4627,6 +4629,10 @@ virStorageSourceNetworkDefaultPort(virStorageNetProtocol protocol)
> case VIR_STORAGE_NET_PROTOCOL_VXHS:
> return 9999;
>
> + case VIR_STORAGE_NET_PROTOCOL_NFS:
> + /* return based on exmaple and SUSE support docs */
It's better to add a link rather than a statement that is hard to
verify.
> + return 2049;
> +
> case VIR_STORAGE_NET_PROTOCOL_LAST:
> case VIR_STORAGE_NET_PROTOCOL_NONE:
> return 0;
We'll discuss the lack of tests separately.
The rest of this patch looks good.
