[libvirt] [PATCH 3/5] domain conf: char: Add an explicit targetType field

[Cole Robinson]

On 07/15/2010 05:31 AM, Daniel P. Berrange wrote:
> On Wed, Jul 14, 2010 at 03:44:54PM -0400, Cole Robinson wrote:
>> targetType only tracks the actual <target> format we are parsing.
>> TYPE_DEFAULT is the typical serial/parallel format, NONE is for the
>> <monitor> device which prints nothing.
>>
>> Signed-off-by: Cole Robinson <crobinso at redhat.com>
>> ---
>>  src/conf/domain_conf.c |  108 ++++++++++++++++++++++++-----------------------
>>  src/conf/domain_conf.h |   19 ++++++--
>>  src/qemu/qemu_conf.c   |    6 +-
>>  3 files changed, 72 insertions(+), 61 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index c9140fe..e4d52ff 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -161,14 +161,18 @@ VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST,
>>                "internal",
>>                "direct")
>>  
>> +VIR_ENUM_IMPL(virDomainChrTarget, VIR_DOMAIN_CHR_TARGET_TYPE_LAST,
>> +              "none",
>> +              "default",
>> +              "guestfwd",
>> +              "virtio")
>> +
>>  VIR_ENUM_IMPL(virDomainChrDevice, VIR_DOMAIN_CHR_DEVICE_TYPE_LAST,
>> -              "null",
>>                "monitor",
>>                "parallel",
>>                "serial",
>>                "console",
>> -              "guestfwd",
>> -              "virtio")
>> +              "channel")
> 
> This is breaking backwards compatability AFAICT.
> 
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index b4e756a..75dc29a 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -312,15 +312,22 @@ struct _virDomainNetDef {
>>  };
>>  
>>  enum virDomainChrDeviceType {
>> -    VIR_DOMAIN_CHR_DEVICE_TYPE_NULL = 0,
>> -    VIR_DOMAIN_CHR_DEVICE_TYPE_MONITOR,
>> +    VIR_DOMAIN_CHR_DEVICE_TYPE_MONITOR = 0,
>>      VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL,
>>      VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL,
>>      VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE,
>> -    VIR_DOMAIN_CHR_DEVICE_TYPE_GUESTFWD,
>> -    VIR_DOMAIN_CHR_DEVICE_TYPE_VIRTIO,
>> +    VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL,
>>  
>> -    VIR_DOMAIN_CHR_DEVICE_TYPE_LAST
>> +    VIR_DOMAIN_CHR_DEVICE_TYPE_LAST,
>> +};
> 
> This is breaking backwards compatability with existing XML configs
> surely.
>

Hmm, not sure if it is. Let me try to clarify a bit.

The "null" here being removed above is actually a string value with no
meaning in the XML. There is no <null type='pty'>... character device,
and there is no <serial><target type='null'></serial> device. "null"
here is just a place holder to enable having a deviceType value of
VIR_DOMAIN_CHR_DEVICE_TYPE_NULL, which was used internally to work
around the issues of combining device type
serial/parallel/console/monitor/channel and target type guestfwd/virtio

Since this patch attempts to separate those differences into two fields,
we can drop the "null" piece: it was never actually being read from XML.

Unfortunately the targetType field in this patch suffers from the same
problem: there are 2 fields "none" and "default" which are never written
into the XML and only used for internal implementation. That could be
unwound in another patch, but I think it would require always generating
an explicit <target type=''> in the XML.

Thanks,
Cole