On Mon, Apr 08, 2013 at 11:13:51AM -0400, Laine Stump wrote:
> On 04/05/2013 05:10 AM, Han Cheng wrote:
> > On 04/02/2013 10:15 AM, Hu Tao wrote:
> >> On Mon, Apr 01, 2013 at 08:00:53PM +0800, Han Cheng wrote:
> >>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> >>> index edddf25..f8e3973 100644
> >>> --- a/src/conf/domain_conf.h
> >>> +++ b/src/conf/domain_conf.h
> >>> @@ -439,6 +439,8 @@ struct _virDomainHostdevDef {
> >>> } source;
> >>> virDomainHostdevOrigStates origstates;
> >>> virDomainDeviceInfoPtr info; /* Guest address */
> >>> + /* readonly is only used for scsi hostdev */
> >>> + unsigned int readonly;
> >>
> >> bool readonly;
> >>
> >>> };
> >>>
> >>> /* Two types of disk backends */
> >
> > How about:
> > struct _virDomainHostdevDef {
> > ...
> > unsigned int managed : 1;
> > unsigned int missing : 1;
> > unsigned int readonly : 1; /* readonly is only used for scsi
> > hostdev */
> > ...
> > };
>
> Actually the use of bitfields for booleans has kind of fallen out of
> favor in libvirt, and those left in the code seem to be there just
> because nobody has thought it was important enough to change it. If you
> want consistency, I think it would be more appropriate to make a patch
> that changed managed and missing from bitfields into bool, then add
> readonly as a bool too.
Agreed, using the 'bool' type is better than bitfields, since it gives
us consistency with method params / return types where we required use
of bool too. The extra memory consumption isn't worth worrying about
given the context of what we're doing here.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list