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

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

 



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@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]