On Thu, Mar 11, 2021 at 01:43:05PM +0000, Andrew Cooper wrote: > On 11/03/2021 13:28, Jan Beulich wrote: > > On 11.03.2021 14:00, Andrew Cooper wrote: > >> However, having laid things out in this way today, it occurs to me that > >> we should consider further cleanup as well. > >> > >> I do agree that code wanting to use the libxendevicemodel.h API almost > >> certainly don't want/need the dmop ABI. (i.e. an individual consumer > >> will want one, or the other, but almost certainly not both together). > >> > >> Should libxendevicemodel.h really be including dm_op.h?
FTR, this is xendevicemodel.h. Just saying because it took me a bit to find the header. I'm dense today. > > I was indeed wondering. > > > >> AFAICT, it is > >> only the ioserverid_t typedef which is API shared between the two > >> contexts, and we can trivially typedef that locally. > > Hmm, a local typedef isn't nice - there should be one central point. > > Granted there's no risk for this to change in anywhere halfway > > foreseeable future, but still. Also neither C89 nor C99 allow a > > typedef to be repeated - in those versions we'd then rely on an > > extension. > > I wonder if we're depending on that extension elsewhere. As far as the > stable libraries go, we are dependent on a Linux or BSD environment > currently. > > Alternatively we can drop the typedef and use uint16_t instead without > breaking things in practice. (As long as we make the change in 4.15 and > we lose the wiggle room afforded us by the entire interface being behind > __XEN_TOOLS__ previously). > > Thoughts? I can't think of any ifdefary which would help, and swapping > to uint16_t would reduce the use of an improperly namespaced identifier. I don't see much problem in switching to uint16_t, it's likely what should have been used from the start in order to avoid bits of dm_op.h leaking into xendevicemodel.h. Or alternatively a new type that maps to uint16_t if we think that would be more descriptive from a header PoV: server_t or some such. At the end of day it should be an opaque handler from the caller PoV, or is it expected that the ioserverid_t obtained from xendevicemodel will be used as a parameter to other libraries? If you end up changing the type to uint16_t it might help to expand the parameter name to server_id or some such, as an id parameter with type uint16_t is kind of ambiguous. We already have some comment blocks to describe the purpose of the parameters, so I don't think it's a big deal if you also leave then as 'id'. Thanks, Roger.
