Re: [PATCH 04/11] nodedev: add an active config to mdev

>>> +/** >>> + * virNodeDeviceGetXMLDescFlags: >>> + * >>> + * Flags used to provide the state of the returned node device >>> configuration. >>> + * >>> + * Since: 10.1.0 >>> + */ >>> +typedef enum { >>> +    VIR_NODE_DEVICE_GET_XML_DESC_INACTIVE            = 1 << 0, /* >>> dump inactive device configuration (Since: 10.1.0) */ >>> +} virNodeDeviceGetXMLDescFlags; >> >> In all of the other similar cases, this type is named >> vir$(OBJECT)XMLFlags and the flag itself is named >> VIR_$(OBJECT)_XML_INACTIVE. So for consistency, I'd remove the 'get' >> and the 'desc' from the names. >> > > I disagree as all other methods that make use of flags base the flags > names on the method name. Here are the examples: > > virNodeDeviceCreateXML > virNodeDeviceCreateXMLFlags > vir Node Device Create XML > VIR_NODE_DEVICE_CREATE_XML_* > > virNodeDeviceDefineXML > virNodeDeviceDefineXMLFlags > vir Node Device Define XML > VIR_NODE_DEVICE_DEFINE_XML_* > > virConnectListAllNodeDevices > virConnectListAllNodeDeviceFlags > vir Connect List Node Device [All is removed] > VIR_CONNECT_LIST_NODE_DEVICES_* > > These are the reasons I chose for consistency: > virNodeDeviceGetXMLDesc > virNodeDeviceGetXMLDescFlags > vir Node Device Get XML Desc > VIR_NODE_DEVICE_GET_XML_DESC_INACTIVE That's true in general, however for the *GetXMLDesc() functions, this pattern doesn't hold: domain:  - virDomainGetXMLDesc()  - virDomainXMLFlags    - VIR_DOMAIN_XML_INACTIVE storage:  - virStoragePoolGetXMLDesc()  - virStorageXMLFlags    - VIR_STORAGE_XML_INACTIVE network:  - virNetworkGetXMLDesc()  - virNetworkXMLFlags    - VIR_NETWORK_XML_INACTIVE interface:  - virInterfaceGetXMLDesc()  - virInterfaceXMLFlags    - VIR_INTERFACE_XML_INACTIVE There are no types following the *GetXMLDescFlags pattern: $ git grep XMLDescFlags include/ $ I don't feel personally strongly about this point because there are consistency arguments for both approaches. But I thought I'd mention it. Jonathon