On Thu, Sep 17, 2020 at 09:30:43 -0400, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma <m.mizuma@xxxxxxxxxxxxxx>
>
> Update validation of transient disk option. The option for qemu is supported
> with under condistions.
>
> - qemu has blockdev feature
> - the type is file and the format is qcow2 and raw
> - writable disk
>
> Signed-off-by: Masayoshi Mizuma <m.mizuma@xxxxxxxxxxxxxx>
> ---
> src/qemu/qemu_validate.c | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 070f1c962b..9cf78ca0c9 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -2002,6 +2002,28 @@ qemuValidateDomainDeviceDefDiskSerial(const char *value)
> }
>
>
> +static int
This is declared as returning 'int'
> +qemuValidateDomainDeviceDefDiskTransient(const virDomainDiskDef *disk,
If this function is called qemuValidateDomainDeviceDefDiskTransient it
should also do all of the validation including reporting errors.
> + virQEMUCapsPtr qemuCaps)
> +{
> + if ((!qemuCaps) || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV))
Parentheses are not required around negation
> + return false;
but returns booleans instead
> +
> + if (disk->src->readonly)
> + return false;
> +
> + if ((disk->src->format != VIR_STORAGE_FILE_QCOW2) &&
> + (disk->src->format != VIR_STORAGE_FILE_RAW) &&
> + (disk->src->type != VIR_STORAGE_TYPE_FILE)) {
> + return false;
> + }
> +
> + if (virStorageSourceIsEmpty(disk->src))
> + return false;
> +
> + return true;
> +}
> +
> static int
> qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk,
> virQEMUCapsPtr qemuCaps)
> @@ -2186,7 +2208,8 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk,
> }
> }
>
> - if (disk->transient) {
> + if ((disk->transient) &&
> + !qemuValidateDomainDeviceDefDiskTransient(disk, qemuCaps)) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> _("transient disks not supported yet"));
The error messages this produces are really suboptimal. You don't know
whether your qemu is old, whether incorrect format is used or whether
it's a network disk.
> return -1;
I'll rewrite this patch to produce better error messages and fix the
problems above.
