On Wed, 2015-10-21 at 20:40 -0400, Laine Stump wrote:
On 10/21/2015 06:44 PM, John Ferlan wrote:
> Typically when searching I can change the "virDomainFoo" type
> string to
> a "qemuDomainFoo" string and find the qemu API. Here those you'll
> see
> there's a virHostDevPreparePCIDevices(), so in a way I'd expect a
> qemuHostdevPreparePCIDevices
>
> Since is a "qemu_hostdev.{c,h}" type change, then theoretically all
> the
> API's in here should be qemuHostdevFoo, right?
>
>
> Rather than qemuPrepareFoo, qemuUpdateFoo, and
> qemuDomainReAttachFoo
> It'd be qemuHostdevPrepare, qemuHostdevUpdate, and
> qemuHostdevReAttach
>
> and your new API would be qemuHostdevUpdateActiveDevices
I had a similar reply queued up, but got interrupted by dinner:
You've changes all Hostdev into Host, but in all of these cases they
really are <hostdev> devices, so I think the function name is clearer
if
it has Hostdev in it rather than Host.
Also, if we're going to normalize the names, it might be better to
try
and fit in with the naming convention that we've tried to start using
in
other places - virModuleNameModifierAction or
virModuleNameActionModifier (or in the case of functions in the qemu
directory qemuModuleNameModifierAction() or
qemuModuleNameActionModifier()), for example
virNetdevMacVLanCreate().
So instead of qemuPrepareHostdevPCIDevices() it could maybe be
qemuHostdevPCIPrepareDevices(), qemuHostdevUSBPrepareDevices() and
qemuHostdevSCSIPrepareDevices() (or maybe
qemuHostdev*DevicesPrepare(),
depending on how anal you want to be about putting the verb at the
end)
I went with the smallest change possible to achieve some
sort of internal consistency, but I agree that being
consistent with the rest of the code is even better.
John's use case is very compelling IMHO, plus personally
something like
qemuHostdevPreparePCIDevices()
feels way more natural than
qemuHostdevPCIPrepareDevices()
so I'd go with his suggestion if that's okay with you.
Cheers.
--
Andrea Bolognani
Software Engineer - Virtualization Team