[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] RFC configurable block formats
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] RFC configurable block formats |
Date: |
Thu, 01 Oct 2009 22:29:28 +0200 |
User-agent: |
Gnus/5.11 (Gnus v5.11) Emacs/22.3 (gnu/linux) |
Anthony Liguori <address@hidden> 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);
- Re: [Qemu-devel] RFC configurable block formats,
Markus Armbruster <=