Re: [RFC] virtio-blk PCI backend

[Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>]

Dor Laor wrote:
> Anthony Liguori wrote:
>> This still needs quite a lot of work but I wanted to post it for 
>> reference.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>> diff --git a/qemu/Makefile.target b/qemu/Makefile.target
>>   
> ...
> Why change Rusty's codding standard? It will be harder to track changes.

Because Linux kernel coding standards aren't QEMU coding standards.  
Besides, this is supposed to be an ABI so it shouldn't be changing all 
that much :-)

I posted the QEMU bits as soon as I got it working.  I still have a lot 
to do in it however I have addressed some of the things you brought up 
already.

>> +    case VIRTIO_PCI_QUEUE_PFN:
>> +    pa = (ram_addr_t)val << TARGET_PAGE_BITS;
>> +    vdev->vq[vdev->queue_sel].pfn = val;
>>   
> Some validity checks are missing, you assume you have the queue_sel.

Yes, the code is carefully written so that queue_sel is always valid.

>> +    if (pa < (ram_size - TARGET_PAGE_SIZE))
>> +        vring_init(&vdev->vq[vdev->queue_sel], phys_ram_base + pa);
>> +    break;
>> +    case VIRTIO_PCI_QUEUE_SEL:
>> +    if (val < VIRTIO_PCI_QUEUE_MAX)
>> +        vdev->queue_sel = val;
>> +    break;
>> +    case VIRTIO_PCI_QUEUE_NOTIFY:
>> +    if (val < VIRTIO_PCI_QUEUE_MAX)
>> +        virtio_ring_kick(vdev, &vdev->vq[val]);
>> +    break;
>> +    case VIRTIO_PCI_STATUS:
>> +    vdev->status = val & 0xFF;
>>   
> we should keep another internal status and it will track the 
> initialization of all the above fields (
> pfn, queue_sel,..) the device will be active once all of them were 
> initialized by the guest

Hrm, I don't follow.  The only thing that has to be written to by the 
guest is the PFN which also has the effect of activating the queue.

>> +    break;
>> +    default:
>> +    if (addr >= VIRTIO_PCI_CONFIG && vdev->set_config)
>> +        vdev->set_config(vdev->opaque, addr - VIRTIO_PCI_CONFIG, val);
>> +    break;
>> +    }
>> +}
>> +
>>   
>
> What about having block/net/9p.. in separate files? It will grow over 
> time.

Yup, already have that in my own queue.

The latest version queue for the kernel side is at 
http://hg.codemonkey.ws/virtio-pci  (based on the master branch of 
Rusty's virtio tree).  The latest queue for the QEMU side is at 
http://hg.codemonkey.ws/qemu-virtio.  I have a functioning block and 9p 
transport.  I'll continue cleaning up tomorrow and will hopefully post 
another set of patches early next week.

Unfortunately, I uncovered a bug in the in-kernel APIC code today so you 
need to run with -no-kvm-irqchip if you want to use multiple virtio 
devices at once :-/

Regards,

Anthony Liguori

>> +#include <linux/virtio_blk.h>
>> +#include <stdbool.h>
>> +
>> +#define BLK_MAX_QUEUE_SIZE    127
>> +
>> +static bool virtio_blk_handle_request(BlockDriverState *bs,
>> +                      VirtIODevice *vdev, VirtQueue *vq)
>> +{
>> +    struct iovec iov[vq->vring.num];
>> +    unsigned int head, out_num, in_num, wlen;
>> +    struct virtio_blk_inhdr *in;
>> +    struct virtio_blk_outhdr *out;
>> +
> Great job, Dor.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/