On Mon, Jun 30, 2014 at 11:25:01AM +0100, Cornelia Huck wrote:
> On Mon, 30 Jun 2014 10:36:19 +0100
> Will Deacon <will.deacon@xxxxxxx> wrote:
> > 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 :)
Agreed.
> 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.
Okey doke, I'll take a look for v2 (I need to remove a stray semi-colon from
the patch anyway).
Thanks for the feedback,
Will
--
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
