New vendor branch for FreeBSD on Hyper-V. Please review.
Alexander Motin
mav at FreeBSD.org
Wed Mar 27 07:17:09 UTC 2013
Hi.
As ATA driver maintainer I've took a look on that part of the code. I
have some comments about what I see. Please keep me CC'd, as I am
normally not on this list.
1) You are using machine dependent function do_cpuid() in machine
independent ATA code. It will just not build on any architecture except
amd64 and i386. You should periodically run `make universe` to be sure
that you code does not breaks the build for some rare archs. As minimum
that code should be wrapped into respective #ifdef's, but I would prefer
it to be moved out somewhere to MD code (like MD code that does general
VM detection now), as this doesn't really belong to the ATA driver.
2) You are disabling "ad" ATA disk driver. Here I have two objections:
a) "ad" driver is obsoleted and not used since FreeBSD 9.0, when
CAM-based ATA stack replaces it completely, wrapping only controller
driver parts of old ata(4) into CAM SIM, which is now only one of four
ATA/SATA SIMs in the system. Your change is effectively null now.
b) I believe you are using wrong approach here. I believe instead of
completely disabling disk driver you should disable only specific
Hyper-V emulated ATA controller device instance. I've never used
Hyper-V, but I guess it should support PCI device pass-through. Just
imagine what happen if somebody wish to pass-through ATA/SATA
controller? You will disable it also.
3) Looking on what I suppose is "Fast IDE" driver in the file
hv_storvsc_drv_freebsd.c, it seems more like cut-down SAS, then IDE. It
doesn't support ATA commands, it reports itself to CAM as SAS. The only
differences I see are disabling LUNs and limiting queue to 1 slot. Just
a smile: it is neither really "Fast" nor at all "IDE". :)
4) In you SAS driver in storvsc_action() you have hardcoded
CTS_SCSI_FLAGS_TAG_ENB flag. While I guess it should not cause any
problems, it wont allow user to control it if he decide to experiment
with disabling it.
--
Alexander Motin
More information about the freebsd-virtualization
mailing list