[libvirt] [PATCH 2/2] qemu: add a check when cold-plug a Chr device

[John Ferlan]

On 12/09/2014 01:48 AM, Luyao Huang wrote:
> Add a func just check the base target type which
> qemu support. But i still doubt this will be useful
> , we already have a check when we try to start the
> vm. And this check only check the target type, and
> the other things will be done in virDomainChrDefParseXML.
> 

The commit message needs some massaging.

Essentially, this patch fixes the "condition" where if a guest has been
started and someone uses attach-device with (or without) the --config
option, then these checks will avoid the "next" guest being modified,
correct?

This will also cause an error earlier that patch 1/2 as
qemuDomainChrInsert is called in the path before qemuDomainAttachDeviceLive



> Signed-off-by: Luyao Huang <lhuang at redhat.com>
> ---
>  src/qemu/qemu_hotplug.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index b9a0cee..fe91ec7 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1384,10 +1384,74 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
>  
>  }
>  
> +static int
> +qemuDomainChrCheckDefSupport(virDomainChrDefPtr chr)
> +{
> +    int ret = -1;
> +
> +    switch (chr->deviceType) {
> +    case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
> +        switch ((virDomainChrSerialTargetType) chr->targetType) {
> +        case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA:
> +        case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB:
> +            ret = 0;
> +            break;
> +
> +        default:

Typically in switches listing other options rather than default:

        case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST:

> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("unsupported serial target type %s for qemu"),
> +                           NULLSTR(virDomainChrSerialTargetTypeToString(chr->targetType)));
> +            break;
> +        }
> +        break;
> +
> +    case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL:
> +        ret = 0;
> +        break;
> +
> +    case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL:
> +        switch ((virDomainChrChannelTargetType) chr->targetType) {
> +        case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
> +        case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
> +            ret = 0;
> +            break;
> +
> +        default:

Same, but:

        case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE:
        case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST:

> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("unsupported channel target type %s for qemu"),
> +                           NULLSTR(virDomainChrChannelTargetTypeToString(chr->targetType)));
> +            break;
> +        }
> +        break;
> +
> +    case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
> +        switch ((virDomainChrConsoleTargetType) chr->targetType) {
> +        case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL:
> +        case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
> +        case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP:
> +        case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM:
> +            ret = 0;
> +            break;
> +
> +        default:

Same, but:

        case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE:
        case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN:
        case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML:
        case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC:
        case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ:
        case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST:


I think perhaps this one is better than 1/2, but will see if my
responses result in other opinions...

John

> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("unsupported console target type %s for qemu"),
> +                           NULLSTR(virDomainChrConsoleTargetTypeToString(chr->targetType)));
> +            break;
> +        }
> +        break;
> +    }
> +
> +    return ret;
> +}
> +
>  int
>  qemuDomainChrInsert(virDomainDefPtr vmdef,
>                      virDomainChrDefPtr chr)
>  {
> +    if (qemuDomainChrCheckDefSupport(chr) < 0)
> +        return -1;
> +
>      if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
>          chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) {
>          virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
>