On 01/21/2015 04:10 AM, lhuang wrote:
>
> On 01/21/2015 03:00 AM, John Ferlan wrote:
>>
>> 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?
>
> Right
>> This will also cause an error earlier that patch 1/2 as
>> qemuDomainChrInsert is called in the path before qemuDomainAttachDeviceLive
>>
>>
>
> Yes and if this patch have been pushed then the patch 1/2 will be a patch for
> improving exist code.
ChrInsert is called after qemuBuildConsoleChrDeviceStr in AttachDevice. We
should error out earlier and include the first patch too.
>>> Signed-off-by: Luyao Huang <lhuang@xxxxxxxxxx>
>>> ---
>>> 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:
The point of casting it to virDomainChrSerialTargetType is to catch all the
unhandled values and it only works if there's no default: clause.
Also I don't think we need the ret variable at all. Just return 0 at the end
of the function, or -1 if we reached an unsupported combination.
>>
>>
>> I think perhaps this one is better than 1/2, but will see if my
>> responses result in other opinions...
Even if this one fixes the bug, 1/2 is nice to have.
>
> Thanks for pointing out and i forgot cc Jan and Pavel when sent this patch,
> maybe they have some other opinions.
>
No need to cc me, I am subscribed to the list. :)
Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list