[libvirt] [PATCH v2 01/10] conf: Introduce readonly to hostdev and change helper function

[Daniel P. Berrange]

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 :|