Re: [PATCH v6 04/13] vhost: rxtx: prepare work for multiple queue support

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

On Tue, Oct 20, 2015 at 08:57:52AM +0200, Thomas Monjalon wrote:
> 2015-10-09 13:46, Yuanhan Liu:
> > From: Changchun Ouyang <changchun.ouyang@intel.com>
> > 
> > Do not use VIRTIO_RXQ or VIRTIO_TXQ anymore; use the queue_id,
> > instead, which will be set to a proper value for a specific queue
> > when we have multiple queue support enabled.
> > 
> > For now, queue_id is still set with VIRTIO_RXQ or VIRTIO_TXQ,
> > so it should not break anything.
> 
> The title may be more informative.
> Example:
> 	vhost: use queue id instead of constant ring index

Thanks, that's indeed better.

> 
> > -	if (unlikely(queue_id != VIRTIO_RXQ)) {
> > -		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
> > +	if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->virt_qp_nb))) {
> > +		RTE_LOG(ERR, VHOST_DATA,
> > +			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
> > +			__func__, dev->device_fh, queue_id);
> [...]
> > -	if (unlikely(queue_id != VIRTIO_RXQ)) {
> > -		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
> > +	if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->virt_qp_nb))) {
> > +		RTE_LOG(ERR, VHOST_DATA,
> > +			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
> > +			__func__, dev->device_fh, queue_id);
> > +		return 0;
> [...]
> > -	if (unlikely(queue_id != VIRTIO_TXQ)) {
> > -		LOG_DEBUG(VHOST_DATA, "mq isn't supported in this version.\n");
> > +	if (unlikely(!is_valid_virt_queue_idx(queue_id, 1, dev->virt_qp_nb))) {
> > +		RTE_LOG(ERR, VHOST_DATA,
> > +			"%s (%"PRIu64"): virtqueue idx:%d invalid.\n",
> > +			__func__, dev->device_fh, queue_id);
> 
> Why not use the log macros in drivers/net/virtio/virtio_logs.h?

Do you mean I should keep using LOG_DEBUG instead?

> If you don't like the current log format, it would deserve a separate patch
> to change them consistently.

Changchun made the change, and I don't know why.

	--yliu