[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/6] conf: relocate virDomainDeviceDef and virDomainHostdevDef



On 02/20/2012 10:10 AM, Laine Stump wrote:
> This patch is only code movement + adding some forward definitions of
> typedefs.
> 
> virDomainHostdevDef (not just a pointer to it, but an actual object)
> will be needed in virDomainNetDef and virDomainActualNetDef, so it
> must be relocated earlier in the file.
> 
> Likewise, virDomainDeviceDef will be needed in virDomainHostdevDef, so
> it must be moved up even earlier. This, in turn, creates a forward
> reference problem, but fortunately only with pointers to other device
> types, so their typedefs can be moved up in the file, eliminating the
> problem.
> 
> Also, a DEVICE_TYPE_NONE is added, to indicate that a
> virDomainDeviceDef doesn't point to a valid device.

On the qemu-devel list, I keep hearing the refrain:

'Any time you write a sentence beginning with "Also" in your commit
message, that's a sign that you probably should have split it into two
patches.'

In this particular case, I'm not going to demand a change, but it's food
for thought.

> +
> +/* Flags for the 'type' field in next struct */

Should we specifically call out virDomainDeviceDef, rather than the
vague "next struct"?  But this is faithful code motion, for now.

> +enum virDomainDeviceType {

Back to the question of whether to use enum typedefs.  No need to change
now, though, as this was just code motion and matches the style in use
throughout this particular file.

ACK.

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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]