Re: [PATCH v2 1/4] KVM: device: add simple registration mechanism for kvm_device_ops

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

 



On Wed, 2 Jul 2014 10:32:41 +0100
Will Deacon <will.deacon@xxxxxxx> wrote:

> On Wed, Jul 02, 2014 at 10:17:20AM +0100, Cornelia Huck wrote:
> > On Tue,  1 Jul 2014 15:45:15 +0100
> > Will Deacon <will.deacon@xxxxxxx> wrote:

> > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > > index e11d8f170a62..6875cc225dff 100644
> > > --- a/include/uapi/linux/kvm.h
> > > +++ b/include/uapi/linux/kvm.h
> > > @@ -940,15 +940,25 @@ struct kvm_device_attr {
> > >  	__u64	addr;		/* userspace address of attr data */
> > >  };
> > > 
> > > -#define KVM_DEV_TYPE_FSL_MPIC_20	1
> > > -#define KVM_DEV_TYPE_FSL_MPIC_42	2
> > > -#define KVM_DEV_TYPE_XICS		3
> > > -#define KVM_DEV_TYPE_VFIO		4
> > >  #define  KVM_DEV_VFIO_GROUP			1
> > >  #define   KVM_DEV_VFIO_GROUP_ADD			1
> > >  #define   KVM_DEV_VFIO_GROUP_DEL			2
> > > -#define KVM_DEV_TYPE_ARM_VGIC_V2	5
> > > -#define KVM_DEV_TYPE_FLIC		6
> > > +
> > > +enum kvm_device_type {
> > > +	KVM_DEV_TYPE_FSL_MPIC_20	= 1,
> > > +#define KVM_DEV_TYPE_FSL_MPIC_20	KVM_DEV_TYPE_FSL_MPIC_20
> > > +	KVM_DEV_TYPE_FSL_MPIC_42,
> > > +#define KVM_DEV_TYPE_FSL_MPIC_42	KVM_DEV_TYPE_FSL_MPIC_42
> > > +	KVM_DEV_TYPE_XICS,
> > > +#define KVM_DEV_TYPE_XICS		KVM_DEV_TYPE_XICS
> > > +	KVM_DEV_TYPE_VFIO,
> > > +#define KVM_DEV_TYPE_VFIO		KVM_DEV_TYPE_VFIO
> > > +	KVM_DEV_TYPE_ARM_VGIC_V2,
> > > +#define KVM_DEV_TYPE_ARM_VGIC_V2	KVM_DEV_TYPE_ARM_VGIC_V2
> > > +	KVM_DEV_TYPE_FLIC,
> > > +#define KVM_DEV_TYPE_FLIC		KVM_DEV_TYPE_FLIC
> > > +	KVM_DEV_TYPE_MAX,
> > 
> > Do you want to add the individual values to the enum? A removal of a
> > type would be an API break (so we should be safe against renumbering),
> > but it's sometimes helpful if one can get the numeric value at a glance.
> 
> Could do, but then I think the advantage of the enum is questionable over
> the #defines, since you could just as easily have two entries in the enum
> with the same ID value as forgetting to update a KVM_DEV_TYPE_MAX #define
> (which was the reason for the enum in the first place).
> 
> So I'd be inclined to keep the patch as-is, unless you have really strong
> objections?

I don't really have a strong opinion on that, so

Acked-by: Cornelia Huck <cornelia.huck@xxxxxxxxxx>

_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm




[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux