From patchwork Thu Oct 1 20:29:28 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Markus Armbruster X-Patchwork-Id: 34772 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 71FB4B7BBD for ; Fri, 2 Oct 2009 07:11:37 +1000 (EST) Received: from localhost ([127.0.0.1]:42762 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MtSw1-0006tu-8J for incoming@patchwork.ozlabs.org; Thu, 01 Oct 2009 17:11:33 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MtSvO-0006r7-H5 for qemu-devel@nongnu.org; Thu, 01 Oct 2009 17:10:54 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MtSvJ-0006kN-QA for qemu-devel@nongnu.org; Thu, 01 Oct 2009 17:10:53 -0400 Received: from [199.232.76.173] (port=33203 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MtSvJ-0006k8-IY for qemu-devel@nongnu.org; Thu, 01 Oct 2009 17:10:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38535) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MtSvI-00015p-Ot for qemu-devel@nongnu.org; Thu, 01 Oct 2009 17:10:49 -0400 Received: from int-mx04.intmail.prod.int.phx2.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.17]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n91LAj37029397; Thu, 1 Oct 2009 17:10:45 -0400 Received: from pike.pond.sub.org (vpn-10-53.str.redhat.com [10.32.10.53]) by int-mx04.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id n91LAii6016270; Thu, 1 Oct 2009 17:10:45 -0400 Received: by pike.pond.sub.org (Postfix, from userid 1000) id 8EC4110097; Thu, 1 Oct 2009 22:29:28 +0200 (CEST) To: Anthony Liguori Subject: Re: [Qemu-devel] RFC configurable block formats References: <873a64dyit.fsf@pike.pond.sub.org> <4AC3E97C.7070005@codemonkey.ws> From: Markus Armbruster Date: Thu, 01 Oct 2009 22:29:28 +0200 In-Reply-To: <4AC3E97C.7070005@codemonkey.ws> (Anthony Liguori's message of "Wed\, 30 Sep 2009 18\:27\:56 -0500") Message-ID: <87ab0azwlz.fsf@pike.pond.sub.org> User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.3 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.67 on 10.5.11.17 X-detected-operating-system: by monty-python.gnu.org: Genre and OS details not recognized. Cc: qemu-devel@nongnu.org X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Anthony Liguori writes: > Markus Armbruster wrote: >> We have code for a quite a few block formats. A quick grep shows bochs, >> cloop, cow, dmg, ftp, ftps, host_cdrom, host_device, host_floppy, http, >> https, nbd, parallels, qcow, qcow2, raw, tftp, vdi, vmdk, vpc, vvfat. >> Only formats ftp, ftps, http, https, tftp are optional (see configure >> --enable-curl). >> >> While I trust that all of these formats are useful at least for some >> people in some circumstances, some of them are of a kind that friends >> don't let friends use in production. >> >> In short, I'd like to be able to configure block formats. Simple >> enough, eh? Except there's a catch: I'd like to be able to include more >> formats in tools like qemu-img than in qemu proper. That lets me >> restrict qemu proper, where a faulty block driver has the greatest >> potential for mischief, to the formats I trust and need, and still keep >> useful capability for other formats in qemu-img. >> >> Thus, I'd like to be able to configure a block driver off for >> everything, or on for utility programs and off for qemu proper, or on >> for everything. >> >> A naive implementation of this idea simply links only those block >> drivers into a program that have been configured for it. Unfortunately, >> this leads to an unwanted difference in behavior between the different >> programs when the format is probed. >> >> Probing gives every block driver a chance to "score" the image, and >> picks the one with the highest score (I'm simplifying, but it'll do to >> illustrate the problem). If two programs have different sets of >> drivers, probing may yield different results. I don't like that. >> >> Say I configure crusty old qcow (note lack of 2) for utility programs >> only. Then I don't want qemu to silently treat qcow images as raw, I >> want it to tell me it can't do qcow. To be precise: >> >> If a format is configured off, no program shall recognize it. >> >> Else all programs shall recognize it, but >> >> if it is configured on for utility programs, off for qemu proper, >> then recognizing it in qemu proper shall be an error. >> >> If you agree this is useful, I'd be willing to code it. >> > > I'd rather see something like a driver white list/black list for qemu > proper. Actually, that's what I had in mind for implementation. > The list would be used to exclude block formats and could be > extended to support read-only formats vs. read-write formats. For > instance, --enable-block-formats='qcow2 raw'. It avoids polluting the > block interface with knowledge of the distinction between a "utility" > program and qemu proper. I agree it's better to keep it out of the BlockDriver interface. Here's a patch for illustration. It's only lightly tested, lacks the configure part, and I still need to check all users of bdrv_open2() are happy. Are we on the same page? diff --git a/block.c b/block.c index 33f3d65..fef686f 100644 --- a/block.c +++ b/block.c @@ -61,6 +61,9 @@ BlockDriverState *bdrv_first; static BlockDriver *first_drv; +/* If non-zero, use only whitelisted block drivers */ +static int use_bdrv_whitelist; + int path_is_absolute(const char *path) { const char *p; @@ -171,6 +174,28 @@ BlockDriver *bdrv_find_format(const char *format_name) return NULL; } +static int bdrv_is_whitelisted(BlockDriver *drv) +{ + static const char *whitelist[] = { + /* TODO this needs to come from configure */ + "raw", "qcow2", "host_device", NULL + }; + const char **p; + + for (p = whitelist; *p; p++) { + if (!strcmp(drv->format_name, *p)) { + return 1; + } + } + return 0; +} + +BlockDriver *bdrv_find_whitelisted_format(const char *format_name) +{ + BlockDriver *drv = bdrv_find_format(format_name); + return drv && bdrv_is_whitelisted(drv) ? drv : NULL; +} + int bdrv_create(BlockDriver *drv, const char* filename, QEMUOptionParameter *options) { @@ -427,7 +452,10 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags, (flags & (BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO)); else open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT); - ret = drv->bdrv_open(bs, filename, open_flags); + if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) + ret = -ENOTSUP; + else + ret = drv->bdrv_open(bs, filename, open_flags); if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE)) { ret = drv->bdrv_open(bs, filename, open_flags & ~BDRV_O_RDWR); bs->read_only = 1; @@ -1739,6 +1767,12 @@ void bdrv_init(void) module_call_init(MODULE_INIT_BLOCK); } +void bdrv_init_with_whitelist(void) +{ + use_bdrv_whitelist = 1; + bdrv_init(); +} + void *qemu_aio_get(AIOPool *pool, BlockDriverState *bs, BlockDriverCompletionFunc *cb, void *opaque) { diff --git a/block.h b/block.h index a966afb..91c7daf 100644 --- a/block.h +++ b/block.h @@ -45,7 +45,9 @@ void bdrv_info(Monitor *mon); void bdrv_info_stats(Monitor *mon); void bdrv_init(void); +void bdrv_init_with_whitelist(void); BlockDriver *bdrv_find_format(const char *format_name); +BlockDriver *bdrv_find_whitelisted_format(const char *format_name); int bdrv_create(BlockDriver *drv, const char* filename, QEMUOptionParameter *options); int bdrv_create2(BlockDriver *drv, diff --git a/monitor.c b/monitor.c index f105a2e..124123b 100644 --- a/monitor.c +++ b/monitor.c @@ -482,7 +482,7 @@ static void do_change_block(Monitor *mon, const char *device, return; } if (fmt) { - drv = bdrv_find_format(fmt); + drv = bdrv_find_whitelisted_format(fmt); if (!drv) { monitor_printf(mon, "invalid format %s\n", fmt); return; diff --git a/vl.c b/vl.c index 7bfd415..580e4a5 100644 --- a/vl.c +++ b/vl.c @@ -2062,7 +2062,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, fprintf(stderr, "\n"); return NULL; } - drv = bdrv_find_format(buf); + drv = bdrv_find_whitelisted_format(buf); if (!drv) { fprintf(stderr, "qemu: '%s' invalid format\n", buf); return NULL; @@ -5597,7 +5597,7 @@ int main(int argc, char **argv, char **envp) /* init the dynamic translator */ cpu_exec_init_all(tb_size * 1024 * 1024); - bdrv_init(); + bdrv_init_with_whitelist(); /* we always create the cdrom drive, even if no disk is there */ drive_add(NULL, CDROM_ALIAS);