Re: [PATCH] ch_domain: Add handler for virDomainDeviceDefPostParseCallback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 16, 2021 at 21:16:01 -0700, William Douglas wrote:
> Instead of trying to match devices passed in based on the monitor
> detecting the number of devices that were used in the domain
> definition, use the devicesPostParseCallback to evaluate if
> unsupported devices are used.
> 
> This allows the compiler to detect when new device types are added
> that need to be checked.
> 
> Signed-off-by: William Douglas <william.douglas@xxxxxxxxx>
> ---
>  src/ch/ch_domain.c  | 121 +++++++++++++++++++++++++++++++++++++++++++
>  src/ch/ch_monitor.c | 122 --------------------------------------------
>  2 files changed, 121 insertions(+), 122 deletions(-)
> 
> diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
> index f9a6f3f31d..02ca5ea06c 100644
> --- a/src/ch/ch_domain.c
> +++ b/src/ch/ch_domain.c
> @@ -197,7 +197,128 @@ virCHDomainDefPostParse(virDomainDef *def,
>      return 0;
>  }
>  
> +static int
> +virCHDomainDeviceDefPostParse(virDomainDeviceDef *dev,
> +                              const virDomainDef *def G_GNUC_UNUSED,
> +                              unsigned int parseFlags G_GNUC_UNUSED,
> +                              void *opaque G_GNUC_UNUSED,
> +                              void *parseOpaque G_GNUC_UNUSED)
> +{
> +    int ret = -1;
> +
> +    switch ((virDomainDeviceType)dev->type) {
> +    case VIR_DOMAIN_DEVICE_DISK:
> +        ret = 0;
> +        break;
> +    case VIR_DOMAIN_DEVICE_LEASE:
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("Cloud-Hypervisor doesn't support lease"));
> +        break;
> +    case VIR_DOMAIN_DEVICE_FS:
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("Cloud-Hypervisor doesn't support fs"));
> +        break;
> +    case VIR_DOMAIN_DEVICE_NET:
> +        ret = 0;
> +        break;
> +    case VIR_DOMAIN_DEVICE_INPUT:
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("Cloud-Hypervisor doesn't support input"));
> +        break;
> +    case VIR_DOMAIN_DEVICE_SOUND:
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("Cloud-Hypervisor doesn't support sound"));
> +        break;

To further simplify the code you can also do the following (just an
example):

    case VIR_DOMAIN_DEVICE_VIDEO:
    case VIR_DOMAIN_DEVICE_HOSTDEV:
    case VIR_DOMAIN_DEVICE_WATCHDOG:
    case VIR_DOMAIN_DEVICE_CONTROLLER:
    case VIR_DOMAIN_DEVICE_GRAPHICS:
    case VIR_DOMAIN_DEVICE_HUB:
    case VIR_DOMAIN_DEVICE_REDIRDEV:
    case VIR_DOMAIN_DEVICE_SMARTCARD:
    case VIR_DOMAIN_DEVICE_CHR:
    default:
        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                       _("Cloud-Hypervisor doesn't support '%s' device"),
                       virDomainDeviceTypeToString(dev->type));
        return -1;


Also note that in validation callbacks any invalid configuration should
direclty return -1 to short circuit the execution so that we don't need
to have a 'ret' variable. It makes the code easier to read.




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux