[libvirt] [PATCH v1 1/4] conf: add xen type for channels

[Joao Martins]

On 09/19/2016 11:29 PM, Jim Fehlig wrote:
> On 09/16/2016 05:43 PM, Joao Martins wrote:
>> So far only guestfwd and virtio were supported. Add an additional
>> for Xen as libxl channels create Xen console visible to the guest.
>>
>> Signed-off-by: Joao Martins <joao.m.martins at oracle.com>
>> ---
>>  docs/schemas/domaincommon.rng | 11 +++++++++++
>>  src/conf/domain_conf.c        | 18 ++++++++++++++----
>>  src/conf/domain_conf.h        |  1 +
>>  src/qemu/qemu_command.c       |  1 +
>>  4 files changed, 27 insertions(+), 4 deletions(-)
>>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 8aaa67e..5901452 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -3686,6 +3686,16 @@
>>        </optional>
>>      </element>
>>    </define>
>> +  <define name="xenTarget">
>> +    <element name="target">
>> +      <attribute name="type">
>> +        <value>xen</value>
>> +      </attribute>
>> +      <optional>
>> +        <attribute name="name"/>
>> +      </optional>
>> +    </element>
>> +  </define>
>>    <define name="channel">
>>      <element name="channel">
>>        <ref name="qemucdevSrcType"/>
>> @@ -3694,6 +3704,7 @@
>>          <choice>
>>            <ref name="guestfwdTarget"/>
>>            <ref name="virtioTarget"/>
>> +          <ref name="xenTarget"/>
>>          </choice>
>>          <optional>
>>            <ref name="alias"/>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 0828041..196799d 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -434,7 +434,8 @@ VIR_ENUM_IMPL(virDomainChrChannelTarget,
>>                VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST,
>>                "none",
>>                "guestfwd",
>> -              "virtio")
>> +              "virtio",
>> +              "xen")
>>  
>>  VIR_ENUM_IMPL(virDomainChrConsoleTarget,
>>                VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST,
>> @@ -2068,6 +2069,7 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
>>              VIR_FREE(def->target.addr);
>>              break;
>>  
>> +        case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
>>          case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
>>              VIR_FREE(def->target.name);
>>              break;
>> @@ -9877,10 +9879,12 @@ virDomainChrDefParseTargetXML(virDomainChrDefPtr def,
>>              virSocketAddrSetPort(def->target.addr, port);
>>              break;
>>  
>> +        case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
>>          case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
>>              def->target.name = virXMLPropString(cur, "name");
>>  
>> -            if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
>> +            if (def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
>> +                !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
>>                  (stateStr = virXMLPropString(cur, "state"))) {
>>                  int tmp;
> 
> I guess we'll need an answer to your question about the 'state' attribute to
> know if this is needed.
Based in the earlier discussion with Michal I guess this can stay as it is?

>> @@ -10171,7 +10175,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
>>          /* path can be auto generated */
>>          if (!path &&
>>              (!chr_def ||
>> -             chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO)) {
>> +             (chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN &&
>> +              chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))) {
>>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>                             _("Missing source path attribute for char device"));
>>              goto error;
>> @@ -14373,6 +14378,7 @@ virDomainChrEquals(virDomainChrDefPtr src,
>>          if (src->targetType != tgt->targetType)
>>              return false;
>>          switch ((virDomainChrChannelTargetType) src->targetType) {
>> +        case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
>>          case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
>>              return STREQ_NULLABLE(src->target.name, tgt->target.name);
>>              break;
>> @@ -18310,6 +18316,8 @@ virDomainChannelDefCheckABIStability(virDomainChrDefPtr src,
>>      }
>>  
>>      switch (src->targetType) {
>> +
>> +    case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
>>      case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
>>          if (STRNEQ_NULLABLE(src->target.name, dst->target.name)) {
>>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> @@ -21432,11 +21440,13 @@ virDomainChrDefFormat(virBufferPtr buf,
>>              break;
>>          }
>>  
>> +        case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
>>          case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
>>              if (def->target.name)
>>                  virBufferEscapeString(buf, " name='%s'", def->target.name);
>>  
>> -            if (def->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT &&
>> +            if (def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
>> +                def->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT &&
>>                  !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) {
>>                  virBufferAsprintf(buf, " state='%s'",
>>                                    virDomainChrDeviceStateTypeToString(def->state));
> 
> Same here.


> Looks good otherwise.
Cool!

Joao