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

[lhuang]

On 01/22/2015 03:37 PM, Peter Krempa wrote:
> On Thu, Jan 22, 2015 at 10:28:19 +0800, Luyao Huang wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1164627
>>
>> Add a func just check the base target type which
>> qemu support. And this check will help to avoid add a qemu
>> unsupport target type chr device to a running guest(hotplug)
>> or to the guest inactive XML (coldplug, will effect next boot)
> You can get exactly the same behavior adding the device by specifying a
> new XML containing the device (via virsh edit for example).
> Unfortunately the unsupported devices can't be checked in the post parse
> callback as it would make existing VMs with broken config disappear.
> Additionally even if you forbid known-bad targets you still even with
> this code can get a failure by adding a SCLP console on a x86 machine:
>
>      <console type='pty'>
>        <target type='sclp' port='0'/>
>      </console>
>
> Starting such VM gets you:
> error: unsupported configuration: sclp console requires QEMU to support
> s390-sclp
>
> This kind of failure depends on the actual qemu process running the VM
> and can be checked only when the VM is starting.

Yes, this check cannot avoid attach a unsupported chr device in all 
scene (too old qemu or a chr device not support for x86_64), because we 
cannot do a check to qemu caps in this place. As you said we should 
check if actual qemu support it when start the vm.
>> And this check only check the target type, and other things
>> have been checked in virDomainChrDefParseXML.
>>
>> Signed-off-by: Luyao Huang <lhuang at redhat.com>
>> ---
>>   src/libvirt_private.syms |  2 ++
>>   src/qemu/qemu_hotplug.c  | 67 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 69 insertions(+)
>>
> Said this. I don't think it's worth adding the check to the coldplug
> path.

Eww, I thought about this before and i think maybe we can have more 
check to the Chr device when we do coldplug than we define it (add new 
check in parse will make guest which have broken settings disappear when 
update libvirt), although this check cannot cover all the sence.

And thanks a lot for your opinion and review!
> Peter

Luyao