[RFCv2 37/46] conf: Replace virDomainGraphicsDefParseXMLSpice(hardcoded) with virDomainGraphicsSpiceDefParseXML(generated)

[Shi Lei]

On 2020-12-05 at 02:17, DanielP. Berrangé wrote:
>On Fri, Sep 04, 2020 at 11:35:29AM +0800, Shi Lei wrote:
>> Signed-off-by: Shi Lei <shi_lei at massclouds.com>
>> ---
>>  src/conf/domain_conf.c | 272 ++---------------------------------------
>>  src/conf/domain_conf.h |  37 +++---
>>  2 files changed, 26 insertions(+), 283 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index b3ec111..20d731b 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -877,6 +877,7 @@ VIR_ENUM_IMPL(virDomainGraphicsVNCSharePolicy,
>> 
>>  VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelName,
>>                VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST,
>> +              "none",
>>                "main",
>>                "display",
>>                "inputs",
>> @@ -14431,13 +14432,14 @@ virDomainGraphicsRDPDefParseXMLHook(xmlNodePtr node
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index df84763..f27f429 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -1584,6 +1584,7 @@ struct _virDomainGraphicsAuthDef {  /* genparse, genformat:separate */
>>  };
>> 
>>  typedef enum {
>> +    VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_NONE = 0,
>
>I'm not sure why this extra enum field needs to be added ?
>
>IMHO we don't really want to have such extra values except in
>a few special cases where we need to track some "default"
>explicitly. 

There're two forms of enum in the code.

(1) Start with a 'default' or 'none' item, e.g., virDomainDiskIo

    The virDomainDiskIo has VIR_DOMAIN_DISK_IO_DEFAULT which is ZERO-indexed.

    When parsing it, the code is like:

    if ((tmp = virXMLPropString(cur, "io")) &&
        (def->iomode = virDomainDiskIoTypeFromString(tmp)) <= 0) {
        virReportError(...);
        return -1;
    }

    It checks XXXTypeFromString() ** <= ** 0, because the form <... io='default'>
    is illegal.

(2) Without a 'default' or 'none' value, e.g., virDomainDiskDevice

    When parsing it, the code is like:

     if ((tmp = virXMLPropString(node, "device")) &&
         (def->device = virDomainDiskDeviceTypeFromString(tmp)) < 0) {
         virReportError(...);
        return -1;
    }

    It just checks XXXTypeFromString() ** < ** 0, because the enum's ZERO-Value item
    is valid.


To handle these two situations, I add extra "default" for case #2 and unify the form of enums,
so that it's easier to implement the generator.

Another solution is to add an extra directive to indicate whether a enum has a 'default' item,
so that the generator can decide between '<=' and '<'. Perhaps it is more safe and reliable.


Regards,
Shi Lei

>
>>      VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MAIN,
>>      VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_DISPLAY,
>>      VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_INPUT,
>
>
>Regards,
>Daniel
>--
>|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org -o- https://fstop138.berrange.com :|
>|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>