Re: [PATCH 2/6] virtio block driver for QEMU (v2)

Rusty Russell
2007-11-13T00:28:44Z

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.