From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 1/3] virtio: find_vqs/del_vqs virtio operations Date: Wed, 13 May 2009 10:26:45 +0300 Message-ID: <4A0A7635.6090107@redhat.com> References: <200905130000.03032.rusty@rustcorp.com.au> <20090512153330.GB26883@redhat.com> <200905131047.09416.rusty@rustcorp.com.au> <20090513071834.GC31139@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Rusty Russell , Christian Borntraeger , virtualization@lists.linux-foundation.org, Anthony Liguori , kvm@vger.kernel.org, Carsten Otte To: "Michael S. Tsirkin" Return-path: Received: from mx2.redhat.com ([66.187.237.31]:40761 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750890AbZEMH2C (ORCPT ); Wed, 13 May 2009 03:28:02 -0400 In-Reply-To: <20090513071834.GC31139@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Michael S. Tsirkin wrote: > On Wed, May 13, 2009 at 10:47:08AM +0930, Rusty Russell wrote: > >> On Wed, 13 May 2009 01:03:30 am Michael S. Tsirkin wrote: >> >>> On Wed, May 13, 2009 at 12:00:02AM +0930, Rusty Russell wrote >>> >>>> and perhaps consider >>>> varargs for the callbacks (or would that be too horrible at the >>>> implementation end?) >>>> >>>> Thanks, >>>> Rusty. >>>> >>> Ugh ... I think it will be. And AFAIK gcc generates a lot of code >>> for varargs - not something we want to do in each interrupt handler. >>> >> Err, no I mean for find_vqs: eg. >> (block device) >> err = vdev->config->find_vqs(vdev, 1, &vblk->vq, blk_done); >> >> (net device) >> err = vdev->config->find_vqs(vdev, 3, vqs, skb_recv_done, skb_xmit_done, NULL); >> >> A bit neater for for the single-queue case. >> >> Cheers, >> Rusty. >> > > Oh. I see. But it becomes messy now that we also need to pass in the > names, and we lose type safety. > Let's just add a helper function for the single vq case? > > static inline struct virtqueue *virtio_find_vq(struct virtio_devide *vdev, > vq_callback_t *c, const char *n) > { > vq_callback_t *callbacks[] = { c }; > const char *names[] = { n }; > struct virtqueue *vq; > int err = vdev->config->find_vqs(vdev, 1, &vq, callbacks, names); > if (err < 0) > return ERR_PTR(err); > return vq; > } > Much saner. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic.