Re: [kvm-devel] [PATCH 2/6] virtio block driver for QEMU (v2)
Rusty Russell wrote: On Tuesday 13 November 2007 10:25:54 Anthony Liguori wrote: Dor Laor wrote: Anthony Liguori wrote: Dor Laor wrote: In general I think we need to add another feature or even version number ( I know you guys hate it). The reason is - Let's say you dont change functionality but change the irq protocol (for example the isr won't be zeroed on read), then an old guest driver wouldn't know it runs on a new host version and will have its irq line pulled up. So I suggest adding a capability of VIRTIO_ISR_CLEAR_XXX or adding a version number. Comments? I don't think we'll actually change the ISR protocol. I think it's the best that it can actually be. However, if we do need to change the ABI for some reason, I think the right thing to do is just use a new device ID (since it's effectively a new device). Regards, Anthony Liguori Changing the devid is acceptable and much more easier then backward compatibility support, I prefer it too. Note that there are some disadvantages when changing the devid - For example, if you installed an old device drivers in the guest kernel and after a while you bring the guest down, upgrade the kvm host version and bring the guest back up. If it has a new device id (and since the abi is not maintained the driver won't match VENDOR=virtio; DEVID=*), then the guest won't have a driver for the new device. In that case I think a guest agent can switch to fully virtualized device and afterwards install the driver automatically. This is what we do in our production environment. Hrm, I have to think about backwards compatibility at the virtio-pci layer. virtio-pci basically exposes two things, the first is an ABI for doing bidirectional notification and setting driver status. The second is a standard and transparent mechanism for virt_rings. I think that the first is simply enough that we don't need a feature mask or a version number. Maybe perhaps with the status bits, I don't know. For the virt_rings, if we had something more sophisticated than virt_rings down the road, that can be discovered/configured in the per-device configuration area so I don't think we need feature/version info for that. I originally had feature bits on each virtqueue for just such a reason. Used the same ack logic as the normal feature bits (ie. set corresponding bit to indicate guest wants to use the feature). Probably worth engineering this in now. I don't think a per-queue feature bit is worthwhile. What we need is feature bits for the ABI. I'd start with one feature bit for the base device ABI. We can add further bits if we add new descriptor types, new queues, or newer config features The feature bits would be explicitly named and documented, instead of I have a third virtqueue so I can use this new ABI. -- error compiling committee.c: too many arguments to function - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 2/6] virtio block driver for QEMU (v2)
Anthony Liguori wrote: Dor Laor wrote: In general I think we need to add another feature or even version number ( I know you guys hate it). The reason is - Let's say you dont change functionality but change the irq protocol (for example the isr won't be zeroed on read), then an old guest driver wouldn't know it runs on a new host version and will have its irq line pulled up. So I suggest adding a capability of VIRTIO_ISR_CLEAR_XXX or adding a version number. Comments? I don't think we'll actually change the ISR protocol. I think it's the best that it can actually be. However, if we do need to change the ABI for some reason, I think the right thing to do is just use a new device ID (since it's effectively a new device). Regards, Anthony Liguori Changing the devid is acceptable and much more easier then backward compatibility support, I prefer it too. Note that there are some disadvantages when changing the devid - For example, if you installed an old device drivers in the guest kernel and after a while you bring the guest down, upgrade the kvm host version and bring the guest back up. If it has a new device id (and since the abi is not maintained the driver won't match VENDOR=virtio; DEVID=*), then the guest won't have a driver for the new device. In that case I think a guest agent can switch to fully virtualized device and afterwards install the driver automatically. This is what we do in our production environment. Regards, 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/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 2/6] virtio block driver for QEMU (v2)
On Sat, Nov 10, 2007 at 09:20:36PM -0600, Anthony Liguori wrote: This patch implements a very naive virtio block device backend in QEMU. There's a lot of room for future optimization. We need to merge a -disk patch before we can provide a mechanism to expose this to users. The latest generation of -disk patches posted to upstream qemu-devel are looking quite promising with any luck should get accepted for merge in the very near future. Are these block net drivers able todo hot-plug/unplug after a domain has been created ? If so we'd also want to have disk_add/disk_remove and net_add/net_remove monitor commands (cf usb_add/usb_remove) for on-the-fly changes. Regards Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| - 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/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 2/6] virtio block driver for QEMU (v2)
Daniel P. Berrange wrote: On Sat, Nov 10, 2007 at 09:20:36PM -0600, Anthony Liguori wrote: This patch implements a very naive virtio block device backend in QEMU. There's a lot of room for future optimization. We need to merge a -disk patch before we can provide a mechanism to expose this to users. The latest generation of -disk patches posted to upstream qemu-devel are looking quite promising with any luck should get accepted for merge in the very near future. Are these block net drivers able todo hot-plug/unplug after a domain has been created ? If so we'd also want to have disk_add/disk_remove and net_add/net_remove monitor commands (cf usb_add/usb_remove) for on-the-fly changes. Some poor soul would need to add this functionality to kvm's acpi tables. -- error compiling committee.c: too many arguments to function - 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/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 2/6] virtio block driver for QEMU (v2)
Avi Kivity wrote: Daniel P. Berrange wrote: On Sat, Nov 10, 2007 at 09:20:36PM -0600, Anthony Liguori wrote: This patch implements a very naive virtio block device backend in QEMU. There's a lot of room for future optimization. We need to merge a -disk patch before we can provide a mechanism to expose this to users. The latest generation of -disk patches posted to upstream qemu-devel are looking quite promising with any luck should get accepted for merge in the very near future. Are these block net drivers able todo hot-plug/unplug after a domain has been created ? If so we'd also want to have disk_add/disk_remove and net_add/net_remove monitor commands (cf usb_add/usb_remove) for on-the-fly changes. Some poor soul would need to add this functionality to kvm's acpi tables. I don't think there's anything I need to do in virtio-pci to support this. I would just see a -probe() event I imagine. Regards, Anthony Liguori - 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/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 2/6] virtio block driver for QEMU (v2)
Dor Laor wrote: Anthony Liguori wrote: Dor Laor wrote: In general I think we need to add another feature or even version number ( I know you guys hate it). The reason is - Let's say you dont change functionality but change the irq protocol (for example the isr won't be zeroed on read), then an old guest driver wouldn't know it runs on a new host version and will have its irq line pulled up. So I suggest adding a capability of VIRTIO_ISR_CLEAR_XXX or adding a version number. Comments? I don't think we'll actually change the ISR protocol. I think it's the best that it can actually be. However, if we do need to change the ABI for some reason, I think the right thing to do is just use a new device ID (since it's effectively a new device). Regards, Anthony Liguori Changing the devid is acceptable and much more easier then backward compatibility support, I prefer it too. Note that there are some disadvantages when changing the devid - For example, if you installed an old device drivers in the guest kernel and after a while you bring the guest down, upgrade the kvm host version and bring the guest back up. If it has a new device id (and since the abi is not maintained the driver won't match VENDOR=virtio; DEVID=*), then the guest won't have a driver for the new device. In that case I think a guest agent can switch to fully virtualized device and afterwards install the driver automatically. This is what we do in our production environment. Hrm, I have to think about backwards compatibility at the virtio-pci layer. virtio-pci basically exposes two things, the first is an ABI for doing bidirectional notification and setting driver status. The second is a standard and transparent mechanism for virt_rings. I think that the first is simply enough that we don't need a feature mask or a version number. Maybe perhaps with the status bits, I don't know. For the virt_rings, if we had something more sophisticated than virt_rings down the road, that can be discovered/configured in the per-device configuration area so I don't think we need feature/version info for that. Rusty, Avi: any thoughts? I'm a bit on the fence. Regards, Anthony Liguori Regards, 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/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 2/6] virtio block driver for QEMU (v2)
On Tuesday 13 November 2007 10:25:54 Anthony Liguori wrote: Dor Laor wrote: Anthony Liguori wrote: Dor Laor wrote: In general I think we need to add another feature or even version number ( I know you guys hate it). The reason is - Let's say you dont change functionality but change the irq protocol (for example the isr won't be zeroed on read), then an old guest driver wouldn't know it runs on a new host version and will have its irq line pulled up. So I suggest adding a capability of VIRTIO_ISR_CLEAR_XXX or adding a version number. Comments? I don't think we'll actually change the ISR protocol. I think it's the best that it can actually be. However, if we do need to change the ABI for some reason, I think the right thing to do is just use a new device ID (since it's effectively a new device). Regards, Anthony Liguori Changing the devid is acceptable and much more easier then backward compatibility support, I prefer it too. Note that there are some disadvantages when changing the devid - For example, if you installed an old device drivers in the guest kernel and after a while you bring the guest down, upgrade the kvm host version and bring the guest back up. If it has a new device id (and since the abi is not maintained the driver won't match VENDOR=virtio; DEVID=*), then the guest won't have a driver for the new device. In that case I think a guest agent can switch to fully virtualized device and afterwards install the driver automatically. This is what we do in our production environment. Hrm, I have to think about backwards compatibility at the virtio-pci layer. virtio-pci basically exposes two things, the first is an ABI for doing bidirectional notification and setting driver status. The second is a standard and transparent mechanism for virt_rings. I think that the first is simply enough that we don't need a feature mask or a version number. Maybe perhaps with the status bits, I don't know. For the virt_rings, if we had something more sophisticated than virt_rings down the road, that can be discovered/configured in the per-device configuration area so I don't think we need feature/version info for that. I originally had feature bits on each virtqueue for just such a reason. Used the same ack logic as the normal feature bits (ie. set corresponding bit to indicate guest wants to use the feature). Probably worth engineering this in now. Cheers, Rusty. - 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/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 2/6] virtio block driver for QEMU (v2)
Anthony Liguori wrote:
This patch implements a very naive virtio block device backend in QEMU.
There's a lot of room for future optimization. We need to merge a -disk patch
before we can provide a mechanism to expose this to users.
Signed-off-by: Anthony Liguori [EMAIL PROTECTED]
diff --git a/qemu/Makefile.target b/qemu/Makefile.target
index c7686b2..49c0fc7 100644
[snip]
+
+if (1) {
+ BlockDriverState *bs = bdrv_new(vda);
+ if (bdrv_open(bs, /home/anthony/images/linux.img, BDRV_O_SNAPSHOT))
+ exit(1);
Can you add a printf to the exit(1). I had to gdb the code to find why
my qemu is not running no more (in earlier version
I did remember to change the path but not after the new patches.
[snip]
+
+static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
+{
+VirtIOBlock *s = to_virtio_blk(vdev);
+VirtQueueElement elem;
+unsigned int count;
+
+while ((count = virtqueue_pop(vq, elem)) != 0) {
+ struct virtio_blk_inhdr *in;
+ struct virtio_blk_outhdr *out;
+ unsigned int wlen;
+ off_t off;
+ int i;
+
+ out = (void *)elem.out_sg[0].iov_base;
+ in = (void *)elem.in_sg[elem.in_num - 1].iov_base;
+ off = out-sector;
+
+ if (out-type VIRTIO_BLK_T_SCSI_CMD) {
+ wlen = sizeof(*in);
+ in-status = VIRTIO_BLK_S_UNSUPP;
+ } else if (out-type VIRTIO_BLK_T_OUT) {
+ wlen = sizeof(*in);
+
+ for (i = 1; i elem.out_num; i++) {
+ bdrv_write(s-bs, off,
+elem.out_sg[i].iov_base,
+elem.out_sg[i].iov_len / 512);
+ off += elem.out_sg[i].iov_len / 512;
+ }
+
+ in-status = VIRTIO_BLK_S_OK;
+ } else {
+ wlen = sizeof(*in);
+
+ for (i = 0; i elem.in_num - 1; i++) {
+ bdrv_read(s-bs, off,
+ elem.in_sg[i].iov_base,
+ elem.in_sg[i].iov_len / 512);
+ off += elem.in_sg[i].iov_len / 512;
+ wlen += elem.in_sg[i].iov_len;
+ }
+
+ in-status = VIRTIO_BLK_S_OK;
+ }
+
+ virtqueue_push(vq, elem, wlen);
+ virtio_notify(vdev, vq);
+}
You can move the notify out of the while loop. This way you save irqs.
+}
+
+static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
+{
+VirtIOBlock *s = to_virtio_blk(vdev);
+int64_t capacity;
+uint32_t v;
+
+bdrv_get_geometry(s-bs, capacity);
+memcpy(config + VIRTIO_CONFIG_BLK_F_CAPACITY, capacity,
sizeof(capacity));
+
+v = VIRTQUEUE_MAX_SIZE - 2;
+memcpy(config + VIRTIO_CONFIG_BLK_F_SEG_MAX, v, sizeof(v));
+}
+
+static uint32_t virtio_blk_get_features(VirtIODevice *vdev)
+{
+return (1 VIRTIO_BLK_F_SEG_MAX);
In general I think we need to add another feature or even version number
( I know you guys hate it).
The reason is - Let's say you dont change functionality but change the
irq protocol (for example the isr won't be zeroed on read), then an old
guest driver wouldn't know it runs on a new host version and will have
its irq line pulled up.
So I suggest adding a capability of VIRTIO_ISR_CLEAR_XXX or adding a
version number.
Comments?
+}
+
+VirtIODevice *virtio_blk_init(PCIBus *bus, uint16_t vendor, uint16_t device,
+ BlockDriverState *bs)
+{
+VirtIOBlock *s;
+
+s = (VirtIOBlock *)virtio_init_pci(bus, virtio-blk, vendor, device,
+vendor, VIRTIO_ID_BLOCK,
+16, sizeof(VirtIOBlock));
+
+s-vdev.update_config = virtio_blk_update_config;
+s-vdev.get_features = virtio_blk_get_features;
+s-bs = bs;
+
+virtio_add_queue(s-vdev, virtio_blk_handle_output);
+
+return s-vdev;
+}
diff --git a/qemu/vl.h b/qemu/vl.h
index fafcf09..249ede2 100644
--- a/qemu/vl.h
+++ b/qemu/vl.h
@@ -1396,6 +1396,9 @@ void vmchannel_init(CharDriverState *hd, uint32_t
deviceid, uint32_t index);
typedef struct VirtIODevice VirtIODevice;
+VirtIODevice *virtio_blk_init(PCIBus *bus, uint16_t vendor, uint16_t device,
+ BlockDriverState *bs);
+
/* buf = NULL means polling */
typedef int ADBDeviceRequest(ADBDevice *d, uint8_t *buf_out,
const uint8_t *buf, int len);
-
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/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 2/6] virtio block driver for QEMU (v2)
Dor Laor wrote:
+
+if (1) {
+BlockDriverState *bs = bdrv_new(vda);
+if (bdrv_open(bs, /home/anthony/images/linux.img,
BDRV_O_SNAPSHOT))
+exit(1);
Can you add a printf to the exit(1). I had to gdb the code to find why
my qemu is not running no more (in earlier version
I did remember to change the path but not after the new patches.
Heh, sorry about that.
[snip]
+virtqueue_push(vq, elem, wlen);
+virtio_notify(vdev, vq);
+}
You can move the notify out of the while loop. This way you save irqs.
I don't think it does actually. This raises the line multiple times,
but if the line is already raised, it's effectively a nop. Now, with
KVM's in-kernel APIC, it ends up generating more syscalls than is
necessary so it's worth changing.
+}
+
+static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t
*config)
+{
+VirtIOBlock *s = to_virtio_blk(vdev);
+int64_t capacity;
+uint32_t v;
+
+bdrv_get_geometry(s-bs, capacity);
+memcpy(config + VIRTIO_CONFIG_BLK_F_CAPACITY, capacity,
sizeof(capacity));
+
+v = VIRTQUEUE_MAX_SIZE - 2;
+memcpy(config + VIRTIO_CONFIG_BLK_F_SEG_MAX, v, sizeof(v));
+}
+
+static uint32_t virtio_blk_get_features(VirtIODevice *vdev)
+{
+return (1 VIRTIO_BLK_F_SEG_MAX);
In general I think we need to add another feature or even version
number ( I know you guys hate it).
The reason is - Let's say you dont change functionality but change the
irq protocol (for example the isr won't be zeroed on read), then an old
guest driver wouldn't know it runs on a new host version and will have
its irq line pulled up.
So I suggest adding a capability of VIRTIO_ISR_CLEAR_XXX or adding a
version number.
Comments?
I don't think we'll actually change the ISR protocol. I think it's the
best that it can actually be. However, if we do need to change the ABI
for some reason, I think the right thing to do is just use a new device
ID (since it's effectively a new device).
Regards,
Anthony Liguori
+}
+
+VirtIODevice *virtio_blk_init(PCIBus *bus, uint16_t vendor, uint16_t
device,
+ BlockDriverState *bs)
+{
+VirtIOBlock *s;
+
+s = (VirtIOBlock *)virtio_init_pci(bus, virtio-blk, vendor,
device,
+ vendor, VIRTIO_ID_BLOCK,
+ 16, sizeof(VirtIOBlock));
+
+s-vdev.update_config = virtio_blk_update_config;
+s-vdev.get_features = virtio_blk_get_features;
+s-bs = bs;
+
+virtio_add_queue(s-vdev, virtio_blk_handle_output);
+
+return s-vdev;
+}
diff --git a/qemu/vl.h b/qemu/vl.h
index fafcf09..249ede2 100644
--- a/qemu/vl.h
+++ b/qemu/vl.h
@@ -1396,6 +1396,9 @@ void vmchannel_init(CharDriverState *hd,
uint32_t deviceid, uint32_t index);
typedef struct VirtIODevice VirtIODevice;
+VirtIODevice *virtio_blk_init(PCIBus *bus, uint16_t vendor, uint16_t
device,
+ BlockDriverState *bs);
+
/* buf = NULL means polling */
typedef int ADBDeviceRequest(ADBDevice *d, uint8_t *buf_out,
const uint8_t *buf, int len);
-
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/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
