[libvirt] [PATCH 1/2] Introduce virDomainYesNo enum type

Eric Blake eblake at redhat.com
Mon Jul 14 16:38:01 UTC 2014


On 07/14/2014 08:56 AM, Daniel P. Berrange wrote:
> On Mon, Jul 14, 2014 at 04:47:16PM +0200, Ján Tomko wrote:
>> @@ -10901,9 +10884,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt,
>>                                   "for useserial"));
>>                  goto cleanup;
>>              }
>> -            def->os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_YES;
>> +            def->os.bios.useserial = VIR_DOMAIN_YES_NO_ENABLED;
>>          } else {
>> -            def->os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_NO;
>> +            def->os.bios.useserial = VIR_DOMAIN_YES_NO_DISABLED;
>>          }
>>          VIR_FREE(tmp);
>>      }
>>
>> @@ -16943,10 +16926,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
>>                                virDomainGraphicsSpiceMouseModeTypeToString(def->data.spice.mousemode));
>>          if (def->data.spice.copypaste)
>>              virBufferAsprintf(buf, "<clipboard copypaste='%s'/>\n",
>> -                              virDomainGraphicsSpiceClipboardCopypasteTypeToString(def->data.spice.copypaste));
>> +                              virDomainYesNoTypeToString(def->data.spice.copypaste));
>>          if (def->data.spice.filetransfer)
>>              virBufferAsprintf(buf, "<filetransfer enable='%s'/>\n",
>> -                              virDomainGraphicsSpiceAgentFileTransferTypeToString(def->data.spice.filetransfer));
>> +                              virDomainYesNoTypeToString(def->data.spice.filetransfer));
>>      }
> 
> I'm not really a fan of this cleanup, as IMHO the result is less clear &
> harder to follow than the original code.

How so? The original code was very repetitive, with multiple enums (all
with long names) copying the same few enum elements.  We're not painting
ourselves into a corner - if any of the replaced enums ever grows a
third value (such as "on", "hybrid", "off"), then we just break that one
enum back into a named list rather than using the generic on/off enum.
I'm actually in favor of this cleanup.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140714/df144c23/attachment-0001.sig>


More information about the libvir-list mailing list