From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1M8sTi-0003KD-CT for qemu-devel@nongnu.org; Tue, 26 May 2009 04:57:46 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1M8sTd-0003J0-L1 for qemu-devel@nongnu.org; Tue, 26 May 2009 04:57:45 -0400 Received: from [199.232.76.173] (port=56416 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1M8sTd-0003Ix-C8 for qemu-devel@nongnu.org; Tue, 26 May 2009 04:57:41 -0400 Received: from verein.lst.de ([213.95.11.210]:33327) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_3DES_EDE_CBC_SHA1:24) (Exim 4.60) (envelope-from ) id 1M8sTc-0002rM-Ik for qemu-devel@nongnu.org; Tue, 26 May 2009 04:57:41 -0400 Date: Tue, 26 May 2009 10:57:38 +0200 From: Christoph Hellwig Subject: Re: [Qemu-devel] [PATCH 3/4] raw-posix: split hdev drivers Message-ID: <20090526085737.GA6335@lst.de> References: <20090525075949.GC2936@lst.de> <4A1A7C1A.9050406@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4A1A7C1A.9050406@redhat.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: Christoph Hellwig , qemu-devel@nongnu.org On Mon, May 25, 2009 at 02:08:10PM +0300, Avi Kivity wrote: > Christoph Hellwig wrote: > >Instead of declaring one BlockDriver for all host devices declared one > >for each type: a generic one for normal disk devices, a Linux floppy > >driver and a CDROM driver for Linux and FreeBSD. This gets rid of a lot > >of messy ifdefs and switching based on the type in the various removal > >device methods. > > > >block.c grows a new method to find the correct host device driver based > >on OS-sepcific criteria. I would love to move this into some OS-dependant > >file but I don't think we have a place where it fits nicely yet. > > > > > > Add a ->probe_host_device() which accepts the filename (or maybe an fd) > as a parameter. First pass does a ->probe_host_device() for all drivers > that support it, second pass tries ->probe(). Here's a patch to implement that incrementally. I wonder why we have the is_windows_drive check in find_protocol - shouldn't that be handled entirely in find_hdev_driver? Signed-off-by: Christoph Hellwig Index: qemu/block/raw-posix.c =================================================================== --- qemu.orig/block/raw-posix.c 2009-05-26 10:42:04.372841227 +0200 +++ qemu/block/raw-posix.c 2009-05-26 10:44:15.290813943 +0200 @@ -946,6 +946,22 @@ kern_return_t GetBSDPath( io_iterator_t #endif +static int hdev_probe_device(const char *filename) +{ + struct stat st; + + /* allow a dedicated CD-ROM driver to match with a higher priority */ + if (strstart(filename, "/dev/cdrom", NULL)) + return 50; + + if (stat(filename, &st) >= 0 && + (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode))) { + return 100; + } + + return 0; +} + static int hdev_open(BlockDriverState *bs, const char *filename, int flags) { BDRVRawState *s = bs->opaque; @@ -1138,6 +1154,7 @@ static int hdev_create(const char *filen static BlockDriver bdrv_host_device = { .format_name = "host_device", .instance_size = sizeof(BDRVRawState), + .bdrv_probe_device = hdev_probe_device, .bdrv_open = hdev_open, .bdrv_close = raw_close, .bdrv_create = hdev_create, @@ -1187,6 +1204,14 @@ static int floppy_open(BlockDriverState return 0; } +static int floppy_probe_device(const char *filename) +{ + if (strstart(filename, "/dev/fd", NULL)) + return 100; + return 0; +} + + static int floppy_is_inserted(BlockDriverState *bs) { return fd_open(bs) >= 0; @@ -1232,6 +1257,7 @@ static int floppy_eject(BlockDriverState static BlockDriver bdrv_host_floppy = { .format_name = "host_floppy", .instance_size = sizeof(BDRVRawState), + .bdrv_probe_device = floppy_probe_device, .bdrv_open = floppy_open, .bdrv_close = raw_close, .bdrv_create = hdev_create, @@ -1265,6 +1291,13 @@ static int cdrom_open(BlockDriverState * return raw_open_common(bs, filename, flags); } +static int cdrom_probe_device(const char *filename) +{ + if (strstart(filename, "/dev/cd", NULL)) + return 100; + return 0; +} + static int cdrom_is_inserted(BlockDriverState *bs) { BDRVRawState *s = bs->opaque; @@ -1309,6 +1342,7 @@ static int cdrom_set_locked(BlockDriverS static BlockDriver bdrv_host_cdrom = { .format_name = "host_cdrom", .instance_size = sizeof(BDRVRawState), + .bdrv_probe_device = cdrom_probe_device, .bdrv_open = cdrom_open, .bdrv_close = raw_close, .bdrv_create = hdev_create, @@ -1355,6 +1389,14 @@ static int cdrom_open(BlockDriverState * return 0; } +static int cdrom_probe_device(const char *filename) +{ + if (strstart(filename, "/dev/cd", NULL) || + strstart(filename, "/dev/acd", NULL)) + return 100; + return 0; +} + static int cdrom_reopen(BlockDriverState *bs) { BDRVRawState *s = bs->opaque; @@ -1425,6 +1467,7 @@ static int cdrom_set_locked(BlockDriverS static BlockDriver bdrv_host_cdrom = { .format_name = "host_cdrom", .instance_size = sizeof(BDRVRawState), + .bdrv_probe_device = cdrom_probe_device, .bdrv_open = cdrom_open, .bdrv_close = raw_close, .bdrv_create = hdev_create, Index: qemu/block_int.h =================================================================== --- qemu.orig/block_int.h 2009-05-26 10:42:04.391815291 +0200 +++ qemu/block_int.h 2009-05-26 10:51:38.200838865 +0200 @@ -48,6 +48,7 @@ struct BlockDriver { const char *format_name; int instance_size; int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename); + int (*bdrv_probe_device)(const char *filename); int (*bdrv_open)(BlockDriverState *bs, const char *filename, int flags); int (*bdrv_read)(BlockDriverState *bs, int64_t sector_num, uint8_t *buf, int nb_sectors); @@ -186,4 +187,8 @@ void *qemu_blockalign(BlockDriverState * extern BlockDriverState *bdrv_first; +#ifdef _WIN32 +int is_windows_drive(const char *filename); +#endif + #endif /* BLOCK_INT_H */ Index: qemu/block.c =================================================================== --- qemu.orig/block.c 2009-05-26 10:42:04.400815321 +0200 +++ qemu/block.c 2009-05-26 10:51:20.142813836 +0200 @@ -260,7 +260,7 @@ static int is_windows_drive_prefix(const filename[1] == ':'); } -static int is_windows_drive(const char *filename) +int is_windows_drive(const char *filename) { if (is_windows_drive_prefix(filename) && filename[2] == '\0') @@ -304,43 +304,23 @@ static BlockDriver *find_protocol(const * Detect host devices. By convention, /dev/cdrom[N] is always * recognized as a host CDROM. */ -#ifdef _WIN32 static BlockDriver *find_hdev_driver(const char *filename) { - if (strstart(filename, "/dev/cdrom", NULL)) - return bdrv_find_format("host_device"); - if (is_windows_drive(filename)) - return bdrv_find_format("host_device"); - return NULL; -} -#else -static BlockDriver *find_hdev_driver(const char *filename) -{ - struct stat st; + int score_max = 0, score; + BlockDriver *drv = NULL, *d; -#ifdef __linux__ - if (strstart(filename, "/dev/fd", NULL)) - return bdrv_find_format("host_floppy"); - if (strstart(filename, "/dev/cd", NULL)) - return bdrv_find_format("host_cdrom"); -#elif defined(__FreeBSD__) - if (strstart(filename, "/dev/cd", NULL) || - strstart(filename, "/dev/acd", NULL)) { - return bdrv_find_format("host_cdrom"); - } -#else - if (strstart(filename, "/dev/cdrom", NULL)) - return bdrv_find_format("host_device"); -#endif - - if (stat(filename, &st) >= 0 && - (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode))) { - return bdrv_find_format("host_device"); + for (d = first_drv; d; d = d->next) { + if (d->bdrv_probe_device) { + score = d->bdrv_probe_device(filename); + if (score > score_max) { + score_max = score; + drv = d; + } + } } - return NULL; + return drv; } -#endif static BlockDriver *find_image_format(const char *filename) { Index: qemu/block/raw-win32.c =================================================================== --- qemu.orig/block/raw-win32.c 2009-05-26 10:42:04.383816228 +0200 +++ qemu/block/raw-win32.c 2009-05-26 10:50:57.934985432 +0200 @@ -302,6 +302,15 @@ static int find_device_type(BlockDriverS } } +static int hdev_probe_device(const char *filename) +{ + if (strstart(filename, "/dev/cdrom", NULL)) + return 100; + if (is_windows_drive(filename)) + return 100; + return 0; +} + static int hdev_open(BlockDriverState *bs, const char *filename, int flags) { BDRVRawState *s = bs->opaque; @@ -387,6 +396,7 @@ static int raw_set_locked(BlockDriverSta static BlockDriver bdrv_host_device = { .format_name = "host_device", .instance_size = sizeof(BDRVRawState), + .bdrv_probe_device = hdev_probe_device, .bdrv_open = hdev_open, .bdrv_close = raw_close, .bdrv_flush = raw_flush,