On Mon, 30 Jun 2014 10:36:19 +0100
Will Deacon <will.deacon@xxxxxxx> wrote:
> Hello,
>
> On Mon, Jun 30, 2014 at 10:21:14AM +0100, Cornelia Huck wrote:
> > On Thu, 26 Jun 2014 18:30:16 +0100
> > Will Deacon <will.deacon@xxxxxxx> wrote:
> > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > > index e11d8f170a62..3b368166286f 100644
> > > --- a/include/uapi/linux/kvm.h
> > > +++ b/include/uapi/linux/kvm.h
> > > @@ -949,6 +949,7 @@ struct kvm_device_attr {
> > > #define KVM_DEV_VFIO_GROUP_DEL 2
> > > #define KVM_DEV_TYPE_ARM_VGIC_V2 5
> > > #define KVM_DEV_TYPE_FLIC 6
> > > +#define KVM_DEV_TYPE_MAX 7
> >
> > This means we always need to move this value once we introduce a new
> > kvm device type. Can't you keep it in a dynamic list instead of a
> > table? We just need to do the lookup during device creation anyway.
>
> Well, we do need the fixed IDs in order for userspace to create these
> devices via the ioctl. If it's the fixed size you're worried about, the
> easiest thing is to replace the array with an idr. I actually started off
> with that, but it felt a bit overkill (since we never need dynamic ID
> allocation). I can bring it back if you prefer?
>
> At the end of the day, we can't get around the fact that the IDs need to
> added with some caution (e.g. not assigning an ID twice).
Ah, just to make this clear, I was only worried about the _MAX value;
the other ids obviously need to be known by userspace :)
So what this basically boils down to is an internal implementation
detail: Do we keep a static table that needs to be grown each time we
add a new device type, or do we keep a dynamic list that can have a
variable number of entries? The dynamic list appeals to me, but it is
slightly more complex code, and we probably won't be adding new device
types left and right anyway. So if your idr code is short and sweet,
I'd say go for it, but not if we end up with a mountain of code.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
