Re: [Qemu-devel] [PATCH 00/16] QEMU vhost-scsi support

[Paolo Bonzini]

Il 19/04/2012 04:38, address@hidden ha scritto:
> From: Zhi Yong Wu <address@hidden>
> 
> The patchset was developed originally by Stefan about one year ago. I
> now rebase it to latest qemu.git/master and fixed some issues to make
> it work against tcm_vhost and virtio_scsi driver. But there are still
> some issues to fix. Let us make more effort later.

Zhi Yong, I'll warn you in advance that this message is going to be a
bit harsh.

You need to understand that once people have patches that work, that can
be only a minor part of the work for getting them ready for submission.
 This is especially true of experienced people like Stefan.  It is quite
common to work briefly on something to build a proof of concept, and
then pick up the work again months later.

Taking patches from other people and completing them can be much harder
than working on new code, because you need to get up to speed with
incomplete code and that is difficult.  You need to understand where the
work was left and what comes next.  You also need to ask the right
person to the guy who did the work, Stefan in this case.  Anything else
will lead to frustration for you and everyone involved, and delay the
project indefinitely.

To put it very frankly I don't know where to start listing mistakes in
this series.  For example:

1) The series is missing any information on how you configure and use
vhost-scsi.  Or on how you build it for that matter.

2) The series uses vhost-scsi unconditionally, as far as I can tell.  So
it breaks command-lines that build scsi-{disk,block} devices for use
with a virtio-scsi-pci device.

Even if it actually worked, you need to mention what happens if you
configure SCSI devices on the same virtio-scsi driver that uses vhost.

3) The cover letter is missing any performance numbers or examples of
why vhost-scsi is preferable to scsi-{disk,block}.  I know some of them,
but it needs to be documented elsewhere.

Documentation is also needed on how/whether migration works with
vhost-scsi, and between the QEMU and kernel targets (I know the answer
to the latter is "it doesn't"; I don't know about the former.

4) The series is not properly split, for example a patch like 4/16 needs
to be squashed above.

5) The series is not up-to-date, for example CONFIG_VIRTIO_SCSI does not
exist anymore.

6) You sent this series without even an RFC tag in the subject.

7) You sent this series, but you didn't even reply on target-devel to my
review of the kernel-side changes.

8) Last but not least, I count two pending patch series from you (NetDev
QOM conversion and network hubs) that you dropped on the list without
hardly following up to comments that were made.  So this gives me very
little incentive to look at your series.

To put it very frankly, I seriously think that you need either more
discipline, or very good mentoring to contribute complex patch series to
QEMU.  The list can offer mentoring, but you must expect criticism like
this one (believe me, this is nothing compared to what you get on the
linux kernel mailing list).

I strongly suggest that you work on one series at a time, for example
the network hub conversion seems to be the most promising.

Paolo